Skip to content

[Dashboard] [Controls] Remove options list "Allow <x>" toggles#147216

Merged
Heenawter merged 15 commits intoelastic:mainfrom
Heenawter:remove-options-list-allow-toggles_2022-12-07
Dec 14, 2022
Merged

[Dashboard] [Controls] Remove options list "Allow <x>" toggles#147216
Heenawter merged 15 commits intoelastic:mainfrom
Heenawter:remove-options-list-allow-toggles_2022-12-07

Conversation

@Heenawter
Copy link
Contributor

@Heenawter Heenawter commented Dec 7, 2022

Closes #147140

Summary

After discussion surrounding the "author" versus "analyst" experience for the controls, we have come to the conclusion that a simpler create experience is necessary to streamline our current process. This means removing all options that are not absolutely necessary - specifically, this PR removes the UX for the following "Additional settings" toggles from the creating/editing flyout:

  • Allow selections to be excluded (hideExclude)
  • Allow exists query (hideExists)
  • Allow dynamic sorting of suggestions (hideSort)

This will leave only two additional settings:

  • Allow multiple selections in dropdown
  • Ignore timeout for results

While the UX for the removed toggles will be removed, the logic is kept - this means that, for the Control Group API that solutions will be using, this functionality could still be exposed to the consumers.

Migration

Consider a scenario where, in 8.6, a user creates an options list control with hideExists set to true- that is, the Exists option is not displayed in the options list suggestions. After upgrading to 8.7, they realize that they want to turn the Exists query back on - but they can't, because the toggle has been removed so hideExists is stuck as true! Their only choice is to remove the control and start over.

In order to avoid this scenario, I added an 8.7 migration that goes through all existing control group panels and, if a control is an options list, it removes the hideExclude and hideExists keys from the explicit input. This is effectively the same was setting these to false since they are optional.

Note
Because the hideExclude and hideExists toggles were added back in 8.6.0 but hideSort was only added in 8.7.0 (which has not yet reached feature freeze), that is why only the hideExclude and hideExists keys are removed. There is no need to add a migration for hideSort because there will be no customer impact from removing the "Allow dynamic sorting of suggestions" toggle.

How to test

For the sake of convenience, I have exported various dashboard saved objects from 8.6 (all of which used the demo "Kibana Sample Data eCommerce" data view) in order to test the migration to 8.7:

  • 8.6_OneControlNoMigrations.ndjson
    This dashboard had a single options list control that should require no migration because it was created with the default settings:

    image

  • 8.6_OneControlTogglesOff.ndjson
    This dashboard had a single options list control with every single toggle turned off:

    image

  • 8.6_TwoOptionsListControls.ndjson
    This dashboard had two options list controls with the following settings:

    TwoOptionsListControls

    The first control also had the exists query selected with exclude set to true, while the second control had non-default size settings, and I made some selections and saved them as part of the dashboard.

  • 8.6_MultipleControlTypes.ndjson
    This dashboard had four controls: two options list controls with the following settings, a range slider control, and a time slider control:

    MultipleControlTypes

These can each be found in the following ZIP file:

Warning
DashboardSavedObjectsForTestingMigration.zip

In order to test this PR, you should (at minimum) download the previous ZIP, individually import each dashboard into Kibana 8.7, and ensure that:

  1. the resulting JSON looks as expected, with no hideExists or hideExclude keys present, and
  2. the dashboard loads correctly and each options list control has the ability to include/exclude, sort, and create an exists query regardless of the previous state of hideExists or hideExclude

Checklist

For maintainers

@Heenawter Heenawter added Feature:Dashboard Dashboard related features release_note:fix Feature:Input Control Input controls visualization Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// loe:medium Medium Level of Effort impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Project:Controls v8.7.0 labels Dec 7, 2022
@Heenawter Heenawter self-assigned this Dec 7, 2022
@Heenawter Heenawter force-pushed the remove-options-list-allow-toggles_2022-12-07 branch 2 times, most recently from 6568aae to 8eda24f Compare December 8, 2022 16:42
@Heenawter Heenawter added the backport:skip This PR does not require backporting label Dec 8, 2022
@Heenawter Heenawter force-pushed the remove-options-list-allow-toggles_2022-12-07 branch from 8eda24f to 7f5a221 Compare December 8, 2022 18:49
Comment on lines +68 to +69
delete explicitInput.hideExclude;
delete explicitInput.hideExists;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that both the exists and include/exclude PRs made it in to 8.6.0 but sorting only made it into 8.7.0. So, since we're not at the FF for 8.7 yet, I can safely remove the "Allow dynamic sorting of suggestions" toggle without any migration.

@Heenawter Heenawter marked this pull request as ready for review December 8, 2022 20:00
@Heenawter Heenawter requested review from a team as code owners December 8, 2022 20:00
@elasticmachine
Copy link
Contributor

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

@Heenawter
Copy link
Contributor Author

@elasticmachine merge upstream

@Heenawter
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
controls 454.3KB 454.3KB +5.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 35.5KB 32.5KB -3.0KB
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 60 66 +6
osquery 109 115 +6
securitySolution 445 451 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 69 75 +6
osquery 110 117 +7
securitySolution 521 527 +6
total +21

History

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

cc @Heenawter

Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Tested locally, importing all of the test dashboards, and everything works as expected, no controls are stuck!

Also looked through the code, and everything looks great! Nice unit testing, good mock additions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Project:Controls release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.7.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Dashboard] [Controls] Remove all new "Allow <x>" toggles

6 participants