Changing where CodeEditor fields get useDarkMode value #159638
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kc13greiner
changed the title
|
@elasticmachine merge upstream |
|
merge conflict between base and head |
kc13greiner
added
Team:Security
|
Pinging @elastic/kibana-security (Team:Security) |
cnasikas
reviewed
Jul 11, 2023
x-pack/plugins/stack_alerts/public/rule_types/es_query/expression/es_query_expression.tsx
Outdated
Show resolved
Hide resolved
mbondyra
approved these changes
Jul 11, 2023
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
kibanamachine
added
v8.10.0
backport:skip
kc13greiner
added
backport:prev-minor
jbudz
removed
the
backport:prev-minor
jbudz
added
backport:prev-minor
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Jul 11, 2023
## Summary Closes elastic#157203 Closes elastic#158051 Closes elastic#158049 With the addition of Per User Dark Mode, components can no longer rely on `uiSettings` as the source of truth for theme. CodeEditor fields used to call `uiSettings` to determine if Dark Mode was enabled, which had been provided in each callers Kibana React Context. The new source of truth for theme is the `CoreStart > ThemeServiceStart`. Currently, not all callers of CodeEditor provide the `theme` service in their Kibana Context in a similar way and some callers don't provide it at all. This PR updates CodeEditor to get theme values from the Kibana Context using a new `useKibanaTheme` react hook. It also attempts audit the callers of CodeEditor to see if their Kibana Context contains the theme service at the top level (Where I could add theme to a caller's Context without major changes, I did. Some cases will require CodeOwner guidance). The new `useKibanaTheme` react hook will throw a TypeError if theme isn't found in the top level of the Kibana Context, this will help with testing as the component will not render. I will remove this after testing so as not to introduce breaking changes. ## Testing Please review files for which you are CODEOWNER. I've attempted to tag all usages of `CodeEditor`/`CodeEditorFIeld` with a TODO comment with one of the following scenarios: 1) a note where theme was provided already/where I made changes to provide it in the appropriate context 2) I've asked for CODEOWNER guidance For scenario 1, please pull and test that CodeEditor locally: 1. Enable Dark Mode from Edit User Profiles by clicking on the Profile Icon on the top right, and updating your profile. 2. Navigate to the CodeEditors in the plugins you own. If they render and display in Dark Mode - add a green check to the table below - and you're done! 3. If it is not rendering, please help me figure out where the theme service should be provided in the context. For scenario 2, we will need to figure out where to make changes so your context is providing theme. Some of the more complex usages may need to addressed in separate issues. ## Tracking | Team | Plugin | Theme in Context ? | Verified Working | | - | - | - | - | | apm-ui | apm | APM Storybook broken | ? | | kibana-presentation | presentation_util | Yes. | Yes | | response-ops | trigger_actions_ui | Yes | Yes | | response-ops | stack_alerts | Yes | Yes | | kibana-security | security | Yes | Yes | | security-defend-workflows | osquery | Yes | Yes | | kibana-app-services | examples/expression_explorer | Yes | Yes | | ml-ui | transform | Yes | Yes | | ml-ui | ml | Yes | Yes | | uptime | synthetics | Yes | Yes | | kibana-gis | maps | Yes | Yes | | kibana-gis | file_upload | Yes | Yes | | platform-deployment-management | watcher | Yes | [AG] Yes | | platform-deployment-management | snapshot_restore | Yes | [AG] Yes | | platform-deployment-management | runtime_fields | Yes | [AG] Yes | | platform-deployment-management | painless_lab | Yes | [AG] Yes | | platform-deployment-management | ingest_pipelines | Yes | [AG] Yes | | platform-deployment-management | index_management | Yes | [AG] Yes | | platform-deployment-management | grokdebugger | Yes | [AG] Yes | | platform-deployment-management | es_ui_shared | Yes | [AG] Yes | | fleet | fleet | Yes | Yes | | enterprise-search-frontend | enterprise_search | Yes | [AG] Yes | | kibana-cloud-security-posture | cloud-security-posture | Yes | yes | | sec-cloudnative-integrations | cloud_defend | Yes | Yes | | kibana-visualizations/kibana-data-discovery | data | Yes | Yes | | kibana-visualizations | examples/testing_embedded_lens | Yes | Yes | | kibana-visualizations | vis_types | Yes | Yes | | kibana-visualizations | vis_default_editor | Yes | Yes | | kibana-visualizations | unified_search | Yes | Yes | | kibana-visualizations | packages/kbn-text-based-editor | Yes | Yes | | kibana-visualizatons | lens | Yes | Yes| | kibana-core | saved_objects_management | Yes | Yes | | kibana-presentation | inspector | Yes | Yes | | kibana-presentation | canvas | Yes | Yes | | kibana-data-discovery | discover | Yes | Yes | | kibana-data-discovery | data_view_management | Yes | Yes | | kibana-data-discovery | data_view_field_editor | Yes | Yes | | appex-sharedux | advanced_settings | Yes | Yes | | enterprise-search-frontend | serverless_search | Yes | [AG] Yes | | - | - | - | - | ## Unit tests Currently, many tests are failing since they are probably not providing `theme` in the context. Once CODEOWNERs have weighed in on CodeEditors usages that require discussion, I will update the accompanying tests. ## Release note - Fixes theming of CodeEditors --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Alison Goryachev <[email protected]> Co-authored-by: Dima Arnautov <[email protected]> Co-authored-by: Dima Arnautov <[email protected]> (cherry picked from commit 323b047)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Jul 11, 2023
#161707) # Backport This will backport the following commits from `main` to `8.9`: - [Changing where CodeEditor fields get useDarkMode value (#159638)](#159638) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Kurt","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-07-11T18:02:42Z","message":"Changing where CodeEditor fields get useDarkMode value (#159638)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana/issues/157203\r\nCloses https://github.com/elastic/kibana/issues/158051\r\nCloses https://github.com/elastic/kibana/issues/158049\r\n\r\nWith the addition of Per User Dark Mode, components can no longer rely\r\non `uiSettings` as the source of truth for theme.\r\n\r\nCodeEditor fields used to call `uiSettings` to determine if Dark Mode\r\nwas enabled, which had been provided in each callers Kibana React\r\nContext.\r\n\r\nThe new source of truth for theme is the `CoreStart >\r\nThemeServiceStart`.\r\n\r\nCurrently, not all callers of CodeEditor provide the `theme` service in\r\ntheir Kibana Context in a similar way and some callers don't provide it\r\nat all.\r\n\r\nThis PR updates CodeEditor to get theme values from the Kibana Context\r\nusing a new `useKibanaTheme` react hook.\r\n\r\nIt also attempts audit the callers of CodeEditor to see if their Kibana\r\nContext contains the theme service at the top level (Where I could add\r\ntheme to a caller's Context without major changes, I did. Some cases\r\nwill require CodeOwner guidance).\r\n\r\nThe new `useKibanaTheme` react hook will throw a TypeError if theme\r\nisn't found in the top level of the Kibana Context, this will help with\r\ntesting as the component will not render. I will remove this after\r\ntesting so as not to introduce breaking changes.\r\n\r\n## Testing\r\n\r\nPlease review files for which you are CODEOWNER.\r\n\r\nI've attempted to tag all usages of `CodeEditor`/`CodeEditorFIeld` with\r\na TODO comment with one of the following scenarios:\r\n\r\n1) a note where theme was provided already/where I made changes to\r\nprovide it in the appropriate context\r\n2) I've asked for CODEOWNER guidance\r\n\r\nFor scenario 1, please pull and test that CodeEditor locally:\r\n\r\n1. Enable Dark Mode from Edit User Profiles by clicking on the Profile\r\nIcon on the top right, and updating your profile.\r\n2. Navigate to the CodeEditors in the plugins you own. If they render\r\nand display in Dark Mode - add a green check to the table below - and\r\nyou're done!\r\n3. If it is not rendering, please help me figure out where the theme\r\nservice should be provided in the context.\r\n\r\nFor scenario 2, we will need to figure out where to make changes so your\r\ncontext is providing theme. Some of the more complex usages may need to\r\naddressed in separate issues.\r\n\r\n\r\n## Tracking\r\n| Team | Plugin | Theme in Context ? | Verified Working |\r\n| - | - | - | - |\r\n| apm-ui | apm | APM Storybook broken | ? |\r\n| kibana-presentation | presentation_util | Yes. | Yes |\r\n| response-ops | trigger_actions_ui | Yes | Yes |\r\n| response-ops | stack_alerts | Yes | Yes |\r\n| kibana-security | security | Yes | Yes |\r\n| security-defend-workflows | osquery | Yes | Yes |\r\n| kibana-app-services | examples/expression_explorer | Yes | Yes |\r\n| ml-ui | transform | Yes | Yes |\r\n| ml-ui | ml | Yes | Yes |\r\n| uptime | synthetics | Yes | Yes |\r\n| kibana-gis | maps | Yes | Yes |\r\n| kibana-gis | file_upload | Yes | Yes |\r\n| platform-deployment-management | watcher | Yes | [AG] Yes |\r\n| platform-deployment-management | snapshot_restore | Yes | [AG] Yes |\r\n| platform-deployment-management | runtime_fields | Yes | [AG] Yes |\r\n| platform-deployment-management | painless_lab | Yes | [AG] Yes |\r\n| platform-deployment-management | ingest_pipelines | Yes | [AG] Yes |\r\n| platform-deployment-management | index_management | Yes | [AG] Yes |\r\n| platform-deployment-management | grokdebugger | Yes | [AG] Yes |\r\n| platform-deployment-management | es_ui_shared | Yes | [AG] Yes |\r\n| fleet | fleet | Yes | Yes |\r\n| enterprise-search-frontend | enterprise_search | Yes | [AG] Yes |\r\n| kibana-cloud-security-posture | cloud-security-posture | Yes | yes |\r\n| sec-cloudnative-integrations | cloud_defend | Yes | Yes |\r\n| kibana-visualizations/kibana-data-discovery | data | Yes | Yes |\r\n| kibana-visualizations | examples/testing_embedded_lens | Yes | Yes |\r\n| kibana-visualizations | vis_types | Yes | Yes |\r\n| kibana-visualizations | vis_default_editor | Yes | Yes |\r\n| kibana-visualizations | unified_search | Yes | Yes |\r\n| kibana-visualizations | packages/kbn-text-based-editor | Yes | Yes |\r\n| kibana-visualizatons | lens | Yes | Yes|\r\n| kibana-core | saved_objects_management | Yes | Yes |\r\n| kibana-presentation | inspector | Yes | Yes |\r\n| kibana-presentation | canvas | Yes | Yes |\r\n| kibana-data-discovery | discover | Yes | Yes |\r\n| kibana-data-discovery | data_view_management | Yes | Yes |\r\n| kibana-data-discovery | data_view_field_editor | Yes | Yes |\r\n| appex-sharedux | advanced_settings | Yes | Yes |\r\n| enterprise-search-frontend | serverless_search | Yes | [AG] Yes |\r\n| - | - | - | - |\r\n\r\n## Unit tests\r\n\r\nCurrently, many tests are failing since they are probably not providing\r\n`theme` in the context. Once CODEOWNERs have weighed in on CodeEditors\r\nusages that require discussion, I will update the accompanying tests.\r\n\r\n## Release note\r\n- Fixes theming of CodeEditors\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Alison Goryachev <[email protected]>\r\nCo-authored-by: Dima Arnautov <[email protected]>\r\nCo-authored-by: Dima Arnautov <[email protected]>","sha":"323b0477e35cb3e49cc01b56b23969fb72c7111e","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:APM","Team:Security","Team:uptime","Feature:Drilldowns","Team:Fleet","Feature:Security/User Profile","backport:prev-minor","v8.10.0"],"number":159638,"url":"https://github.com/elastic/kibana/pull/159638","mergeCommit":{"message":"Changing where CodeEditor fields get useDarkMode value (#159638)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana/issues/157203\r\nCloses https://github.com/elastic/kibana/issues/158051\r\nCloses https://github.com/elastic/kibana/issues/158049\r\n\r\nWith the addition of Per User Dark Mode, components can no longer rely\r\non `uiSettings` as the source of truth for theme.\r\n\r\nCodeEditor fields used to call `uiSettings` to determine if Dark Mode\r\nwas enabled, which had been provided in each callers Kibana React\r\nContext.\r\n\r\nThe new source of truth for theme is the `CoreStart >\r\nThemeServiceStart`.\r\n\r\nCurrently, not all callers of CodeEditor provide the `theme` service in\r\ntheir Kibana Context in a similar way and some callers don't provide it\r\nat all.\r\n\r\nThis PR updates CodeEditor to get theme values from the Kibana Context\r\nusing a new `useKibanaTheme` react hook.\r\n\r\nIt also attempts audit the callers of CodeEditor to see if their Kibana\r\nContext contains the theme service at the top level (Where I could add\r\ntheme to a caller's Context without major changes, I did. Some cases\r\nwill require CodeOwner guidance).\r\n\r\nThe new `useKibanaTheme` react hook will throw a TypeError if theme\r\nisn't found in the top level of the Kibana Context, this will help with\r\ntesting as the component will not render. I will remove this after\r\ntesting so as not to introduce breaking changes.\r\n\r\n## Testing\r\n\r\nPlease review files for which you are CODEOWNER.\r\n\r\nI've attempted to tag all usages of `CodeEditor`/`CodeEditorFIeld` with\r\na TODO comment with one of the following scenarios:\r\n\r\n1) a note where theme was provided already/where I made changes to\r\nprovide it in the appropriate context\r\n2) I've asked for CODEOWNER guidance\r\n\r\nFor scenario 1, please pull and test that CodeEditor locally:\r\n\r\n1. Enable Dark Mode from Edit User Profiles by clicking on the Profile\r\nIcon on the top right, and updating your profile.\r\n2. Navigate to the CodeEditors in the plugins you own. If they render\r\nand display in Dark Mode - add a green check to the table below - and\r\nyou're done!\r\n3. If it is not rendering, please help me figure out where the theme\r\nservice should be provided in the context.\r\n\r\nFor scenario 2, we will need to figure out where to make changes so your\r\ncontext is providing theme. Some of the more complex usages may need to\r\naddressed in separate issues.\r\n\r\n\r\n## Tracking\r\n| Team | Plugin | Theme in Context ? | Verified Working |\r\n| - | - | - | - |\r\n| apm-ui | apm | APM Storybook broken | ? |\r\n| kibana-presentation | presentation_util | Yes. | Yes |\r\n| response-ops | trigger_actions_ui | Yes | Yes |\r\n| response-ops | stack_alerts | Yes | Yes |\r\n| kibana-security | security | Yes | Yes |\r\n| security-defend-workflows | osquery | Yes | Yes |\r\n| kibana-app-services | examples/expression_explorer | Yes | Yes |\r\n| ml-ui | transform | Yes | Yes |\r\n| ml-ui | ml | Yes | Yes |\r\n| uptime | synthetics | Yes | Yes |\r\n| kibana-gis | maps | Yes | Yes |\r\n| kibana-gis | file_upload | Yes | Yes |\r\n| platform-deployment-management | watcher | Yes | [AG] Yes |\r\n| platform-deployment-management | snapshot_restore | Yes | [AG] Yes |\r\n| platform-deployment-management | runtime_fields | Yes | [AG] Yes |\r\n| platform-deployment-management | painless_lab | Yes | [AG] Yes |\r\n| platform-deployment-management | ingest_pipelines | Yes | [AG] Yes |\r\n| platform-deployment-management | index_management | Yes | [AG] Yes |\r\n| platform-deployment-management | grokdebugger | Yes | [AG] Yes |\r\n| platform-deployment-management | es_ui_shared | Yes | [AG] Yes |\r\n| fleet | fleet | Yes | Yes |\r\n| enterprise-search-frontend | enterprise_search | Yes | [AG] Yes |\r\n| kibana-cloud-security-posture | cloud-security-posture | Yes | yes |\r\n| sec-cloudnative-integrations | cloud_defend | Yes | Yes |\r\n| kibana-visualizations/kibana-data-discovery | data | Yes | Yes |\r\n| kibana-visualizations | examples/testing_embedded_lens | Yes | Yes |\r\n| kibana-visualizations | vis_types | Yes | Yes |\r\n| kibana-visualizations | vis_default_editor | Yes | Yes |\r\n| kibana-visualizations | unified_search | Yes | Yes |\r\n| kibana-visualizations | packages/kbn-text-based-editor | Yes | Yes |\r\n| kibana-visualizatons | lens | Yes | Yes|\r\n| kibana-core | saved_objects_management | Yes | Yes |\r\n| kibana-presentation | inspector | Yes | Yes |\r\n| kibana-presentation | canvas | Yes | Yes |\r\n| kibana-data-discovery | discover | Yes | Yes |\r\n| kibana-data-discovery | data_view_management | Yes | Yes |\r\n| kibana-data-discovery | data_view_field_editor | Yes | Yes |\r\n| appex-sharedux | advanced_settings | Yes | Yes |\r\n| enterprise-search-frontend | serverless_search | Yes | [AG] Yes |\r\n| - | - | - | - |\r\n\r\n## Unit tests\r\n\r\nCurrently, many tests are failing since they are probably not providing\r\n`theme` in the context. Once CODEOWNERs have weighed in on CodeEditors\r\nusages that require discussion, I will update the accompanying tests.\r\n\r\n## Release note\r\n- Fixes theming of CodeEditors\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Alison Goryachev <[email protected]>\r\nCo-authored-by: Dima Arnautov <[email protected]>\r\nCo-authored-by: Dima Arnautov <[email protected]>","sha":"323b0477e35cb3e49cc01b56b23969fb72c7111e"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/159638","number":159638,"mergeCommit":{"message":"Changing where CodeEditor fields get useDarkMode value (#159638)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana/issues/157203\r\nCloses https://github.com/elastic/kibana/issues/158051\r\nCloses https://github.com/elastic/kibana/issues/158049\r\n\r\nWith the addition of Per User Dark Mode, components can no longer rely\r\non `uiSettings` as the source of truth for theme.\r\n\r\nCodeEditor fields used to call `uiSettings` to determine if Dark Mode\r\nwas enabled, which had been provided in each callers Kibana React\r\nContext.\r\n\r\nThe new source of truth for theme is the `CoreStart >\r\nThemeServiceStart`.\r\n\r\nCurrently, not all callers of CodeEditor provide the `theme` service in\r\ntheir Kibana Context in a similar way and some callers don't provide it\r\nat all.\r\n\r\nThis PR updates CodeEditor to get theme values from the Kibana Context\r\nusing a new `useKibanaTheme` react hook.\r\n\r\nIt also attempts audit the callers of CodeEditor to see if their Kibana\r\nContext contains the theme service at the top level (Where I could add\r\ntheme to a caller's Context without major changes, I did. Some cases\r\nwill require CodeOwner guidance).\r\n\r\nThe new `useKibanaTheme` react hook will throw a TypeError if theme\r\nisn't found in the top level of the Kibana Context, this will help with\r\ntesting as the component will not render. I will remove this after\r\ntesting so as not to introduce breaking changes.\r\n\r\n## Testing\r\n\r\nPlease review files for which you are CODEOWNER.\r\n\r\nI've attempted to tag all usages of `CodeEditor`/`CodeEditorFIeld` with\r\na TODO comment with one of the following scenarios:\r\n\r\n1) a note where theme was provided already/where I made changes to\r\nprovide it in the appropriate context\r\n2) I've asked for CODEOWNER guidance\r\n\r\nFor scenario 1, please pull and test that CodeEditor locally:\r\n\r\n1. Enable Dark Mode from Edit User Profiles by clicking on the Profile\r\nIcon on the top right, and updating your profile.\r\n2. Navigate to the CodeEditors in the plugins you own. If they render\r\nand display in Dark Mode - add a green check to the table below - and\r\nyou're done!\r\n3. If it is not rendering, please help me figure out where the theme\r\nservice should be provided in the context.\r\n\r\nFor scenario 2, we will need to figure out where to make changes so your\r\ncontext is providing theme. Some of the more complex usages may need to\r\naddressed in separate issues.\r\n\r\n\r\n## Tracking\r\n| Team | Plugin | Theme in Context ? | Verified Working |\r\n| - | - | - | - |\r\n| apm-ui | apm | APM Storybook broken | ? |\r\n| kibana-presentation | presentation_util | Yes. | Yes |\r\n| response-ops | trigger_actions_ui | Yes | Yes |\r\n| response-ops | stack_alerts | Yes | Yes |\r\n| kibana-security | security | Yes | Yes |\r\n| security-defend-workflows | osquery | Yes | Yes |\r\n| kibana-app-services | examples/expression_explorer | Yes | Yes |\r\n| ml-ui | transform | Yes | Yes |\r\n| ml-ui | ml | Yes | Yes |\r\n| uptime | synthetics | Yes | Yes |\r\n| kibana-gis | maps | Yes | Yes |\r\n| kibana-gis | file_upload | Yes | Yes |\r\n| platform-deployment-management | watcher | Yes | [AG] Yes |\r\n| platform-deployment-management | snapshot_restore | Yes | [AG] Yes |\r\n| platform-deployment-management | runtime_fields | Yes | [AG] Yes |\r\n| platform-deployment-management | painless_lab | Yes | [AG] Yes |\r\n| platform-deployment-management | ingest_pipelines | Yes | [AG] Yes |\r\n| platform-deployment-management | index_management | Yes | [AG] Yes |\r\n| platform-deployment-management | grokdebugger | Yes | [AG] Yes |\r\n| platform-deployment-management | es_ui_shared | Yes | [AG] Yes |\r\n| fleet | fleet | Yes | Yes |\r\n| enterprise-search-frontend | enterprise_search | Yes | [AG] Yes |\r\n| kibana-cloud-security-posture | cloud-security-posture | Yes | yes |\r\n| sec-cloudnative-integrations | cloud_defend | Yes | Yes |\r\n| kibana-visualizations/kibana-data-discovery | data | Yes | Yes |\r\n| kibana-visualizations | examples/testing_embedded_lens | Yes | Yes |\r\n| kibana-visualizations | vis_types | Yes | Yes |\r\n| kibana-visualizations | vis_default_editor | Yes | Yes |\r\n| kibana-visualizations | unified_search | Yes | Yes |\r\n| kibana-visualizations | packages/kbn-text-based-editor | Yes | Yes |\r\n| kibana-visualizatons | lens | Yes | Yes|\r\n| kibana-core | saved_objects_management | Yes | Yes |\r\n| kibana-presentation | inspector | Yes | Yes |\r\n| kibana-presentation | canvas | Yes | Yes |\r\n| kibana-data-discovery | discover | Yes | Yes |\r\n| kibana-data-discovery | data_view_management | Yes | Yes |\r\n| kibana-data-discovery | data_view_field_editor | Yes | Yes |\r\n| appex-sharedux | advanced_settings | Yes | Yes |\r\n| enterprise-search-frontend | serverless_search | Yes | [AG] Yes |\r\n| - | - | - | - |\r\n\r\n## Unit tests\r\n\r\nCurrently, many tests are failing since they are probably not providing\r\n`theme` in the context. Once CODEOWNERs have weighed in on CodeEditors\r\nusages that require discussion, I will update the accompanying tests.\r\n\r\n## Release note\r\n- Fixes theming of CodeEditors\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Alison Goryachev <[email protected]>\r\nCo-authored-by: Dima Arnautov <[email protected]>\r\nCo-authored-by: Dima Arnautov <[email protected]>","sha":"323b0477e35cb3e49cc01b56b23969fb72c7111e"}}]}] BACKPORT--> Co-authored-by: Kurt <[email protected]>
kc13greiner
deleted the
bugfix/provide_theme_in_context_for_useTheme_hook
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Closes #157203
Closes #158051
Closes #158049
With the addition of Per User Dark Mode, components can no longer rely on
uiSettingsas the source of truth for theme.CodeEditor fields used to call
uiSettingsto determine if Dark Mode was enabled, which had been provided in each callers Kibana React Context.The new source of truth for theme is the
CoreStart > ThemeServiceStart.Currently, not all callers of CodeEditor provide the
themeservice in their Kibana Context in a similar way and some callers don't provide it at all.This PR updates CodeEditor to get theme values from the Kibana Context using a new
useKibanaThemereact hook.It also attempts audit the callers of CodeEditor to see if their Kibana Context contains the theme service at the top level (Where I could add theme to a caller's Context without major changes, I did. Some cases will require CodeOwner guidance).
The new
useKibanaThemereact hook will throw a TypeError if theme isn't found in the top level of the Kibana Context, this will help with testing as the component will not render. I will remove this after testing so as not to introduce breaking changes.Testing
Please review files for which you are CODEOWNER.
I've attempted to tag all usages of
CodeEditor/CodeEditorFIeldwith a TODO comment with one of the following scenarios:For scenario 1, please pull and test that CodeEditor locally:
For scenario 2, we will need to figure out where to make changes so your context is providing theme. Some of the more complex usages may need to addressed in separate issues.
Tracking
Unit tests
Currently, many tests are failing since they are probably not providing
themein the context. Once CODEOWNERs have weighed in on CodeEditors usages that require discussion, I will update the accompanying tests.Release note