Skip to content

[uiSettings/theme/version] override label to aid KBN_OPTIMIZER_THEMES discovery#96069

Merged
spalger merged 4 commits intoelastic:masterfrom
spalger:implement/ui-settings-theme-disable
Apr 8, 2021
Merged

[uiSettings/theme/version] override label to aid KBN_OPTIMIZER_THEMES discovery#96069
spalger merged 4 commits intoelastic:masterfrom
spalger:implement/ui-settings-theme-disable

Conversation

@spalger
Copy link
Contributor

@spalger spalger commented Apr 1, 2021

In #94834 we removed the other options from the uiSetting when the KBN_OPTIMIZER_THEME environment variable isn't set, which is a little confusing and doesn't really help people know what to do when they need to test v7. This change overrides the label for the option when only one option is available so that people will be reminded to use the KBN_OPTIMIZER_THEMES environment variable in order to get additional options.

Screen Shot 2021-04-02 at 11 03 22 AM

@spalger spalger added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v8.0.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Apr 1, 2021
@spalger spalger marked this pull request as ready for review April 1, 2021 19:28
@spalger spalger requested a review from a team as a code owner April 1, 2021 19:28
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@spalger
Copy link
Contributor Author

spalger commented Apr 1, 2021

cc @cchaos

@cchaos
Copy link
Contributor

cchaos commented Apr 1, 2021

😄 Thanks @spalger ! Any way to put that text as the helpText of the EuiFormRow, then we could even just disable the whole input if there's less than 2 options???? Perhaps, maybe?

@spalger
Copy link
Contributor Author

spalger commented Apr 1, 2021

I tried using the readonly flag to disable the select box and moving the text to the description but doing that actually hides the setting from the advanced settings page, so I think that's more confusing. I also think the help text is useful but a lot less obvious than putting the hint right into the value. For this dev only feature I think it makes sense to save people time by going with the ugly but effective approach personally.

@cchaos
Copy link
Contributor

cchaos commented Apr 2, 2021

Ha, yeah. I was just thinking that you can't easily highlight the text of a select option for a dev to easily copy/paste. I'm pushing you a commit that then moves the message to the description and simplifies the option label.

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

LGTM

@spalger
Copy link
Contributor Author

spalger commented Apr 8, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@spalger spalger merged commit 3336db0 into elastic:master Apr 8, 2021
@spalger spalger deleted the implement/ui-settings-theme-disable branch April 8, 2021 22:13
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 release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants