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

Adding a temporary theme value to uiSettings for Code Editor usage #158793

Closed

Conversation

kc13greiner
Copy link
Contributor

@kc13greiner kc13greiner commented Jun 1, 2023

Summary

Adding a computed theme value to the uiSettings that will be used by Code Editors

The Code Editor fields, found here, are currently relying on uiSettings > 'theme:darkMode' to be the source of truth for which theme they should be rendered in.

All callers of the Code Editor currently provide uiSettings in their context which is then accessed by Code Editor component.

After the introduction of Per User Dark Mode, the new source of truth for theme is the Core Theme Service. However, not all plugins currently provide the theme service in a consistent way (or even at all). All callers would need to be updated to provide the theme to their context in a similar fashion, or, pass the theme down to where the Code Editor component is being called and passed in as a prop (once the value is retrieved from the theme$ observable).

Here is a list of callers for CodeEditor:
Screenshot 2023-06-01 at 10 49 29 AM

That is a lot of plugins to update/test/review by 8.8.1 FF, I attempted to make the changes, but working on unfamiliar plugins is very challenging and risky. Some of the calls in the list above are wrappers around CodeEditor, which are then called many other places.

The changes in this PR are a 'hack' to take advantage of uiSettings already being provided in all necessary contexts, but now providing a correctly computed value (it is the same value that is being provided to theme).

I would like to put this in place temporarily, until either all plugins can make changes to provide theme (with more codeowner oversight) or take advantage of future functionality (such as this new context feature)

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jun 1, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #37 / endpoint Response Actions Responder from alerts should show Responder from alert details under alerts list page
  • [job] [logs] FTR Configs #64 / management kibana settings state:storeInSessionStorage defaults to null
  • [job] [logs] FTR Configs #64 / management kibana settings state:storeInSessionStorage defaults to null
  • [job] [logs] Jest Tests #5 / RenderingService preboot() render() renders "core" page
  • [job] [logs] Jest Tests #5 / RenderingService preboot() render() renders "core" page driven by settings
  • [job] [logs] Jest Tests #5 / RenderingService preboot() render() renders "core" page for blank basepath
  • [job] [logs] Jest Tests #5 / RenderingService preboot() render() renders "core" page for unauthenticated requests
  • [job] [logs] Jest Tests #5 / RenderingService preboot() render() renders "core" page with global settings
  • [job] [logs] Jest Tests #5 / RenderingService preboot() render() renders "core" with excluded global user settings
  • [job] [logs] Jest Tests #5 / RenderingService preboot() render() renders "core" with excluded user settings
  • [job] [logs] Jest Tests #5 / RenderingService setup() render() renders "core" page
  • [job] [logs] Jest Tests #5 / RenderingService setup() render() renders "core" page driven by settings
  • [job] [logs] Jest Tests #5 / RenderingService setup() render() renders "core" page for blank basepath
  • [job] [logs] Jest Tests #5 / RenderingService setup() render() renders "core" page for unauthenticated requests
  • [job] [logs] Jest Tests #5 / RenderingService setup() render() renders "core" page with global settings
  • [job] [logs] Jest Tests #5 / RenderingService setup() render() renders "core" with excluded global user settings
  • [job] [logs] Jest Tests #5 / RenderingService setup() render() renders "core" with excluded user settings
  • [job] [logs] FTR Configs #21 / Stack Management registers all UI Settings in the UsageStats interface
  • [job] [logs] FTR Configs #21 / Stack Management registers all UI Settings in the UsageStats interface

Metrics [docs]

Page load bundle

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

id before after diff
kibanaReact 51.9KB 51.9KB +6.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 400 404 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 480 484 +4
total +6

History

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

@kc13greiner kc13greiner changed the title Adding a uiSetting on the fly that can be used by code editors to determine theme Adding a temporary theme value to uiSettings for Code Editor usage Jun 1, 2023
@kc13greiner kc13greiner deleted the bugfix/code_editor_theme branch January 26, 2024 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants