[Lens] Enabling Random Sampling

[dej611]

Summary

This PR is a design implementation to improve the Random sampling feedback for the user.
Some work has been done to extract locally the SamplingSlider component, that will be eventually moved into a separate package outside of Lens.

In terms of design, to start, the Layer setting now inherits the same Data/Appearance design from the dimension editor:

Next, on the dataView picker of the layer panel the random sampling information is shown when enabled:

when transitioning to Maximum/Minimum operation during editing the sampling is disabled and a info toast is shown:

The toast will show only when transitioning to such operations. If the user goes from Maximum/Minimum to other supported operations then the toast flag is reset, therefore going again into Maximum will trigger a new toast.
Transitioning from a quick function Maximum into a formula max(...) will not trigger a toast as the flag is persisted within the same "editing session".

If the user configured a random sampling setting but then picked a Maximum operation the Layer setting becomes disabled:

and last the embeddable view with the new visualization modifiers view on the bottom-left:

Previous PoC design

This PR works as a PoC for random sampling with the current state of the design. The UI is still a bit rough and not final.

when transitioning to Maximum/Minimum operation:

The toast will show only when transitioning to such operations. If the user goes from Maximum/Minimum to other supported operations then the toast flag is reset, therefore going again into Maximum will trigger a new toast.
Transitioning from a quick function Maximum into a formula max(...) will not trigger a toast as the flag is persisted within the same "editing session".

If the user configured a random sampling setting but then picked a Maximum operation the Layer setting becomes disabled:

At dashboard level the random sampling is notified via an icon on the bottom left:

Hovering the icon will show a detailed tooltip:

At the dashboard level a new i icon is displayed when a random sampling feature is enabled in the panel and a popup with more details is shown on hover:

Checklist

Delete any items that are not applicable to this PR.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
• Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
• This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[elasticmachine]

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

[dej611]

Note the current design does not reflect the new "palette" design on the badge popover as it requires a deep change of the architecture of both the Embeddable code and how Lens derives dimension colors.
Layers with colors can be a follow up PR built on top of this.

[drewdaemon]

First off—this looks great from a UX perspective.

However, UserMessage was intended to be the one message interface to rule them all. Why are we adding FeatureBadge instead of simply extending UserMessage with another severity rating?

[dej611]

However, UserMessage was intended to be the one message interface to rule them all. Why are we adding FeatureBadge instead of simply extending UserMessage with another severity rating?

To me they are serving two distinct purposes.

[dej611]

Thanks @MichaelMarcialis for your feedback. I've tried to address it as much as possible and commented inline for each point.
The current PR is already complex and I prefer to not introduce "out of scope" (where the scope is the "random sampling") changes, so I would propose to additional follow up tasks on top of this PR.

• On annotation layers, the option to ignore global filters is still contained in the layer actions menu. Can this be moved to the new layer settings flyout, as indicated in the designs? Or is that something that will happen in a follow-up PR?
• The indicator on the data view selector of layers that indicates whether that layer is ignoring global filters that was shown in the designs appears to be missing. Is this something that will be added as part of a future PR?

I prefer to push these additional task in a ignore global filters follow up PR.

• Will the design suggestion for adding visibility controls at the layer level (on both the layer panel and layer settings) be an addition for a follow-up PR? I noticed it was omitted in this PR.

Also in this case filter visibility would be best to be pursued in a dedicated layer visibility follow up PR.

• In the layer settings flyout, there is an extra border appearing between the "Layer settings" heading and the "Data" heading, causing it to look like a single 2px border. Would it be possible to correct this so only a single 1px border is showing?

Fixed in f1ca9b1

• I'm not seeing the toast appear at all when applying a minimum or maximum quick function to a layer that has reduced sampling. Is this the bug you mentioned about the toasts crashing? If so, would it be possible to address?
• The toast in your screenshot appears to be using the color="primary" prop to give it a blue top border. Can we remove that to better match the designs?

Fixed in f1ca9b1
Also extended the check for referenced operations like Differences/Moving average/etc... when the user picks Minimum/Maximum as sub-functions.

• When a layer is using reduced sampling, the minimum and maximum quick functions for that layer should receive one of the yellow incompatible dots with accompanying tooltip explaining that sampling will be forced to 100%. This appears to be missing from the PR. Would it be possible to add?

Fixed in f1ca9b1
Also Counter rate uses Max under the hood and has been added to the warning operations list. When in formula it has a separate validation, so no conflict there with this new check.

• When the sampling slider is disabled due to a minimum or maximum quick function being applied, the disabled slider should be shown at 100%, making it clear to the user that it is being forced to sample the full dataset. Possible to change this to better match the designs?

Fixed in f1ca9b1

• Rather than using the iInCircle icon, can we use this SVG (provide) as a custom icon? I'll make a PR to get the icon added to EUI in the meantime, and we can swap to the EUI version in a future update.

If that is ok I can do a follow up after this PR merged with the provided icon.

• Bonus nitpick: can we make the hover/focus state underline for the delete/clear layer actions be red to match the text color? Happy to make a separate issue if preferable.

Will create a separate issue about this as well.

• It looks like the embedded/panel filters are not being included in the modifier popover, as indicated in the design. They still show in the top-right of the dashboard panels. Possible to add to modifier popover? Or will that be a separate effort?

I think also this needs to be done in a move filters into visualization modifiers follow up PR, .

• Can we increase the warning/error icon size to 16px (in the conditional error/warning button) to match the wrench icon size (and all other icon button sizes in the panel)? Currently, I believe it's showing as 14px.

Fixed in f1ca9b1

• Per the design notes, could we also remove the warning/error button border radius override styles and instead just use the default 4px border radius for all button corners?

Fixed in f1ca9b1
I've re-itroduced the default borders, but the final result looks a bit odd to me:

@stratoula: There is a toast that is supposed to be showing when users select a minimum or maximum quick function on a layer that has reduced sampling. Currently, it doesn't appear to be working. There should also be an incompatibility dot and tooltip on the minimum and maximum quick functions when reduced sampling is in effect. That is missing as well. I've noted both issues for @dej611 in my review.

Fixed in f1ca9b1

I may be under-thinking it, but it seems fairly obvious to me that reduced sampling on a layer wouldn't affect static dimension values. I'm personally inclined to not disable the slider in those situations, and I'm on the fence as to whether it really needs any explanation in the settings flyout. If @KOTungseth thinks an additional sentence of help text for it would be beneficial though, I'm not opposed to it.

I agree, but I think if the flag has been raised maybe we could consider it.

@drewdaemon the code has been refactored to leverage the user message system as discussed offline.

Will follow another couple of fixes for the remaining issues/feedback.

[dej611]

Added the on panel hover behaviour to fade-in the wrench icon. Is this what you meant @MichaelMarcialis ?

[MichaelMarcialis]

Thanks so much for making those changes, @dej611. This looks excellent. I'm adding some final small comments for your re-review, but nothing worth holding you up over. Assuming you're able to address these last items, this looks good from my perspective. Approving now.

• Would it be possible to remove the leading 0 before the decimal point in the slider values? Doing so will ensure the tick labels don't collide with each other at small viewport sizes.

  

[MichaelMarcialis]

Thanks for adding this aside element. Now rather than applying padding to the child elements within it, would it be better to apply padding to this new aside element? Then you could also use margining (instead of the current padding) to the child elements to correct the excessive spacing between the layer list item elements to be 16px (instead of the current 32px caused by two 16px paddings). Thoughts?

[dej611]

Fixed in 4dcabe6

If it's ok for you I will add perform more stylistic tuning in a followup where I'll have to add also the color palette for each item in the list.

[dej611]

Thanks so much for making those changes, @dej611. This looks excellent. I'm adding some final small comments for your re-review, but nothing worth holding you up over. Assuming you're able to address these last items, this looks good from my perspective. Approving now.

• Would it be possible to remove the leading 0 before the decimal point in the slider values? Doing so will ensure the tick labels don't collide with each other at small viewport sizes.
  

Fixed in 8346cfa

[dej611]

@elasticmachine merge upstream

[stratoula]

This looks great to me! Thanx for applying all these comments Marco, LGTM!

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 6791228

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
lens	891	906	+15

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
lens	1.3MB	1.3MB	+16.5KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
securitySolution	432	435	+3

Total ESLint disabled count

id	before	after	diff
securitySolution	512	515	+3

History

• 💔 Build #117593 failed 3af9981
• 💔 Build #117586 failed 4dcabe6
• 💔 Build #117472 failed 2e46209
• 💔 Build #117428 failed 4209655
• 💔 Build #117399 failed cdda86a
• 💛 Build #117010 was flaky fc28090

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream