Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Controls] Re-add filtering settings #172857

Conversation

Heenawter
Copy link
Contributor

@Heenawter Heenawter commented Dec 7, 2023

Closes #162985

Summary

This PR re-adds UI for the filtering settings that allow authors to determine whether or not they want the unified search bar / time picker to sync with the control group.

To provide some context, we used to have these settings, but they were deemed over complicated and the UI to control them was removed back in v8.3. Here's a screenshot of what they used to look like:

Note

These settings still existed in the code, even after v8.3 - all we did was remove the UI to control them.

After customer feedback, we decided to re-add the UI to control these settings, with some simplification - specifically, after investigating how other apps (Maps, Lens, etc.) handle unified search bar settings, I noticed that the control group was the only place that considered the query bar to be different than / seperate from filter pills. Therefore, rather than having three toggles like we did previously (filter pills, query bar, and time picker), I combined the filter pills + query bar settings into a single "Apply global filters" toggle. So now, our unified search bar settings have only two toggles, like so:

This is not only simpler than it was in versions less than v8.3.0, it is also more consistent with how other apps do it.

Important

After some design feedback, I moved the old descriptions for the "Validate user selections" and "Chain controls" settings into tooltips + switched to a compressed EuiSwitch for all the settings.

All of the screenshots in this PR description have been updated to reflect this change, but some screenshots in the comments below may be out of date.

Summary of Control Group Settings by Version

Version Screenshot
Version <= v8.2.0
v8.3.0 <= version <= v8.12.0
This branch (v8.13.0)

Checklist

For maintainers

@Heenawter Heenawter added release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:small Small Level of Effort impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Project:Controls labels Dec 7, 2023
@Heenawter Heenawter self-assigned this Dec 7, 2023
@Heenawter Heenawter force-pushed the controls-ignore-global-filters-settings_2023-12-06 branch from 7681953 to 733e7bd Compare December 8, 2023 16:04
@Heenawter
Copy link
Contributor Author

/ci

@Heenawter Heenawter force-pushed the controls-ignore-global-filters-settings_2023-12-06 branch from 120535b to 04b2358 Compare December 8, 2023 23:23
@Heenawter Heenawter force-pushed the controls-ignore-global-filters-settings_2023-12-06 branch from 04b2358 to 5ad6a15 Compare December 18, 2023 17:37
@Heenawter
Copy link
Contributor Author

/ci

@Heenawter Heenawter force-pushed the controls-ignore-global-filters-settings_2023-12-06 branch from c546135 to 0cb4bd1 Compare December 18, 2023 20:04
@Heenawter Heenawter force-pushed the controls-ignore-global-filters-settings_2023-12-06 branch 5 times, most recently from 0c50e8b to 974f2ea Compare December 19, 2023 15:02
@Heenawter Heenawter force-pushed the controls-ignore-global-filters-settings_2023-12-06 branch from 974f2ea to 9dc7d69 Compare December 19, 2023 15:03
Comment on lines +282 to +288
getUseGlobalFiltersTitle: () =>
i18n.translate('controls.controlGroup.management.filtering.useGlobalFilters', {
defaultMessage: 'Apply global filters to controls',
}),
getUseGlobalTimeRangeTitle: () =>
i18n.translate('controls.controlGroup.management.filtering.useGlobalTimeRange', {
defaultMessage: 'Apply global time range to controls',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, there is very little consistency in the copy different apps use for their "unified search bar" setting toggles:

          App           Screenshot
Maps
Lens
TSVB

So, since there didn't seem to be universal phrasing, I decided that the Maps copy is the best match in this case and went with that - open to suggestions, though 🤔

Screenshot 2023-12-18 at 1 34 06 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @amyjtechwriter for the copy here 👀

@Heenawter Heenawter marked this pull request as ready for review December 19, 2023 15:06
@Heenawter Heenawter requested a review from a team as a code owner December 19, 2023 15:06
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@Heenawter
Copy link
Contributor Author

Heenawter commented Dec 19, 2023

As part of this, I did a small redesign of the control group settings menu. Could you take a quick look @andreadelrio? Just want to make sure the new form group labels I added make sense - thank you!! 🙇

image

UPDATE
Made the following changes based on offline feedback:

Screenshot 2023-12-19 at 11 31 16 AM

Copy link
Contributor

@amyjtechwriter amyjtechwriter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - copy wise. I really appreciate that you took the time to provide context (I would have been lost without it) and look at the other copy used in other apps!

@nreese nreese self-requested a review December 19, 2023 20:39
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
code review only

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
controls 157 158 +1

Async chunks

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

id before after diff
controls 200.2KB 199.7KB -524.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
controls 39.8KB 39.9KB +109.0B

History

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

cc @Heenawter

@Heenawter Heenawter merged commit 2486ed5 into elastic:main Dec 19, 2023
37 checks passed
@Heenawter Heenawter deleted the controls-ignore-global-filters-settings_2023-12-06 branch December 19, 2023 23:24
@Heenawter Heenawter added backport:skip This commit does not require backporting and removed impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Dec 19, 2023
@VVKWork
Copy link

VVKWork commented Dec 20, 2023

Thanks. We will adopt as early as possible. Sometimes, it takes time to adopt/ usual cycles; organisational processes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting loe:small Small Level of Effort Project:Controls release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Controls] Allow option to ignore global filters
7 participants