[SharedUX] Modify code editor theme definition and resolution implementation to account for color modes in EUI Theme #203337

eokoneyo · 2024-12-06T19:45:23Z

Summary

Closes #202782

This PR reworks how custom themes used within the kibana code editor for the default visual look and ones specific to supported languages are defined to accomodate the upcoming visual refresh, the approach here leverages the euiTheme object value returned from the useEuiTheme hook, now a single theme declaration is all that is required such that using either the colorMode value or the euiTheme from the provided UseEUITheme value it's possible to craft a theme that's in the context of kIbana, color mode aware and the editor would be able to resolve the appropriate colors depending on the user's color mode.

This required some modification to monaco itself; now when defining languages if the CustomLanguageType specification is being followed, a function that resolves to a standard monaco theme can be provided on the property languageThemeResolver which will be passed the euiTheme when registering this theme. It's worth mentioning that this can also be done manually by leveraging the custom method registerLanguageThemeResolver added on the monaco editor object, like so

 monaco.editor.registerLanguageThemeResolver(LanguageID, languageThemeResolver);

However one should take note that when calling this method directly, the ID passed must correlate to a registered language ID, else the theme will not be available for use after Monaco is initialised, hence the theme name must equal an existing language ID if it's to be used for a specific language.

How to test

Enable borealis, like so;
- in your kibana.dev.yml file include the following config;
```
 uiSettings.experimental.themeSwitcherEnabled: true
```
- start kibana using the following command; KBN_OPTIMIZER_THEMES="borealislight,borealisdark,v8light,v8dark" yarn start --run-examples
Tryout all downstream of the code editor to ascertain the code editor colors are as should be for both Amsterdam and Borealis; downstreams include;
- ES|QL editor, navigate to discover and click the "try ES|QL" button
- Dev tools, on clicking the nav hamburger menu under the management menu group there's a menu item that links to all dev tools
- Index Management use cases; navigate to stack management, under Index management select any existing index, then navigate to it's settings this should load up the code editor.
- Saved Object use case; navigate to stack management, under saved objects attempt inspecting any saved object we'd be presented with the code editor etc.

eokoneyo · 2024-12-06T19:52:14Z

packages/kbn-monaco/src/register_globals.ts

- * Register custom themes
- */
-registerTheme(ESQL_LIGHT_THEME_ID, buildESQLTheme({ darkMode: false }));
-registerTheme(ESQL_DARK_THEME_ID, buildESQLTheme({ darkMode: true }));
-registerTheme(CONSOLE_THEME_ID, buildConsoleTheme());
-registerTheme(CODE_EDITOR_LIGHT_THEME_ID, buildLightTheme());
-registerTheme(CODE_EDITOR_DARK_THEME_ID, buildDarkTheme());
-registerTheme(CODE_EDITOR_LIGHT_THEME_TRANSPARENT_ID, buildLightTransparentTheme());
-registerTheme(CODE_EDITOR_DARK_THEME_TRANSPARENT_ID, buildDarkTransparentTheme());
-


All of this is now handled in the actual component implementation of the code editor, see https://github.com/elastic/kibana/pull/203337/files#diff-21b735fba0b5df628e2664a3605f8401d082a4eb32ab0b1ede5fae2db5251030R177-R193

eokoneyo · 2024-12-06T19:59:12Z

packages/kbn-monaco/src/code_editor/constants.ts

-export const CODE_EDITOR_LIGHT_THEME_ID = 'codeEditorLightTheme';
-export const CODE_EDITOR_DARK_THEME_ID = 'codeEditorDarkTheme';
-export const CODE_EDITOR_LIGHT_THEME_TRANSPARENT_ID = 'codeEditorLightTransparentTheme';
-export const CODE_EDITOR_DARK_THEME_TRANSPARENT_ID = 'codeEditorDarkTransparentTheme';


There's no need for light and dark variations of the same theme anymore, we resolve this automatically now

eokoneyo · 2024-12-18T09:56:38Z

@ek-so Could I trouble you to help review this, whilst it's been held up by #204482, I'd like to pre-emptively get feedback about the color selection, so that once the aforementioned PR gets merged in, we can pretty much we merge this one in?

ek-so

Ok, it's looking good. But tbh, I was not able to figure out what some "lightestShades"/"emptyShades" etc. legacy colors do there. It's currently allowed to use them afaik, so I guess we can merge like this and figure out later.

ek-so · 2024-12-18T14:21:05Z

x-pack/plugins/painless_lab/public/styles/_index.scss

This removes grey surface under tab bar

ek-so · 2024-12-18T14:23:21Z

src/plugins/console/public/application/containers/editor/editor.tsx

This seems to be just unnecessary (we can have only subdued panel prop, that's all)

ek-so · 2024-12-18T14:26:09Z

src/plugins/console/public/application/containers/config/config.tsx

mattkime

Codeowner changes look good and appear to work well 🎉

stratoula

This has many changes in comparison with the existing theme cc @ryankeairns

This PR

Main branch

…ifically opting to override

eokoneyo · 2025-01-03T10:24:09Z

This has many changes in comparison with the existing theme cc @ryankeairns

This PR

Main branch

Hi @stratoula , I took a second look at this, it's true the previous changes made it such that identifiers were colored and not matching the overall text color, and I think with suggestions the colors were wrong, I made some adjustments in b30bc9e that I think should resolve these concerns.

See screenshot below,

Could you help take a second look?

stratoula · 2025-01-03T10:27:14Z

@eokoneyo thanx, much appreciated. Even from the screenshot I can see that the columns (new or existing) such as the order_date in your example is colored wrongly. It is kinda pinky while it should be same color as the index pattern

eokoneyo · 2025-01-03T11:44:06Z

@eokoneyo thanx, much appreciated. Even from the screenshot I can see that the columns (new or existing) such as the order_date in your example is colored wrongly. It is kinda pinky while it should be same color as the index pattern

Thanks for pointing this out @stratoula. I've resolved this in 3252105, here's how it all looks now. Apologies for the noise.

stratoula

Except for a small question, now it looks ok to me

I tested this at light dark mode for Amsterdam and Borealis themes. LGTM

packages/kbn-monaco/src/esql/lib/esql_theme.ts

elasticmachine · 2025-01-03T13:08:27Z

💚 Build Succeeded

Buildkite Build
Commit: 3252105

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/monaco`	124	134	+10

Async chunks

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

id	before	after	diff
`console`	204.3KB	204.2KB	-51.0B
`esql`	203.1KB	203.0KB	-99.0B
`searchIndices`	164.1KB	164.0KB	-71.0B
`security`	542.4KB	542.2KB	-147.0B
`serverlessSearch`	352.1KB	352.1KB	-71.0B
total			-439.0B

Page load bundle

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

id	before	after	diff
`console`	35.6KB	35.6KB	-54.0B
`kbnUiSharedDeps-srcJs`	3.5MB	3.5MB	+509.0B
`painlessLab`	10.0KB	10.0KB	-3.0B
total			+452.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/code-editor`	41	40	-1
`@kbn/monaco`	124	134	+10
total			+9

ESLint disabled line counts

id	before	after	diff
`@kbn/monaco`	5	6	+1

References to deprecated APIs

id	before	after	diff
`@kbn/monaco`	8	1	-7

Total ESLint disabled count

id	before	after	diff
`@kbn/monaco`	13	14	+1

History

💚 Build #263642 succeeded 965dc0d
💛 Build #263575 was flaky a94080b
💛 Build #263315 was flaky c985625
💔 Build #263284 failed 9519a20
💔 Build #262426 failed b689aae
💔 Build #262402 failed ec62a57

cc @eokoneyo

Dosant

Nice work! reviewed the code and smoke checked different editors in light/dark theme

…ntation to account for color modes in EUI Theme (elastic#203337) ## Summary  Closes elastic#202782 This PR reworks how custom themes used within the kibana code editor for the default visual look and ones specific to supported languages are defined to accomodate the upcoming visual refresh, the approach here leverages the `euiTheme` object value returned from the `useEuiTheme` hook, now a single theme declaration is all that is required such that using either the `colorMode `value or the `euiTheme` from the provided `UseEUITheme` value it's possible to craft a theme that's in the context of kIbana, color mode aware and the editor would be able to resolve the appropriate colors depending on the user's color mode. This required some modification to monaco itself; now when defining languages if the `CustomLanguageType` specification is being followed, a function that resolves to a standard monaco theme can be provided on the property `languageThemeResolver` which will be passed the `euiTheme` when registering this theme. It's worth mentioning that this can also be done manually by leveraging the custom method `registerLanguageThemeResolver` added on the monaco editor object, like so ```tsx monaco.editor.registerLanguageThemeResolver(LanguageID, languageThemeResolver); ``` However one should take note that when calling this method directly, the ID passed must correlate to a registered language ID, else the theme will not be available for use after Monaco is initialised, hence the theme name must equal an existing language ID if it's to be used for a specific language. ## How to test - Enable borealis, like so; - in your `kibana.dev.yml` file include the following config; ```yml uiSettings.experimental.themeSwitcherEnabled: true ``` - start kibana using the following command; `KBN_OPTIMIZER_THEMES="borealislight,borealisdark,v8light,v8dark" yarn start --run-examples` - Tryout all downstream of the code editor to ascertain the code editor colors are as should be for both Amsterdam and Borealis; downstreams include; - ES|QL editor, navigate to discover and click the "try ES|QL" button - Dev tools, on clicking the nav hamburger menu under the management menu group there's a menu item that links to all dev tools - Index Management use cases; navigate to stack management, under Index management select any existing index, then navigate to it's settings this should load up the code editor. - Saved Object use case; navigate to stack management, under saved objects attempt inspecting any saved object we'd be presented with the code editor etc.  --------- Co-authored-by: ek-so <[email protected]> Co-authored-by: kibanamachine <[email protected]>

eokoneyo added release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) labels Dec 6, 2024

eokoneyo self-assigned this Dec 6, 2024

eokoneyo commented Dec 6, 2024

View reviewed changes

eokoneyo changed the title ~~Rework code editor theme resolution to account for color modes~~ [SharedUX] Rework code editor theme resolution to account for color modes in borealis Dec 6, 2024

eokoneyo commented Dec 6, 2024

View reviewed changes

eokoneyo force-pushed the fix/resolve-202782 branch 3 times, most recently from a4234d4 to 9c0bf5c Compare December 9, 2024 09:53

eokoneyo changed the title ~~[SharedUX] Rework code editor theme resolution to account for color modes in borealis~~ [SharedUX] Modify code editor theme definition and resolution implementation to account for color modes in EUI Theme Dec 9, 2024

eokoneyo force-pushed the fix/resolve-202782 branch 5 times, most recently from 442bd20 to 096c71b Compare December 10, 2024 12:02

This comment was marked as outdated.

Sign in to view

eokoneyo force-pushed the fix/resolve-202782 branch from 096c71b to f28fb80 Compare December 10, 2024 14:54

JasonStoltz added the EUI Visual Refresh label Dec 10, 2024

eokoneyo force-pushed the fix/resolve-202782 branch from f28fb80 to fbd5730 Compare December 11, 2024 16:09

eokoneyo added the backport:skip This commit does not require backporting label Dec 12, 2024

eokoneyo force-pushed the fix/resolve-202782 branch from fbd5730 to cfe6d1d Compare December 12, 2024 15:39

eokoneyo requested a review from ek-so December 18, 2024 09:52

ek-so approved these changes Dec 18, 2024

View reviewed changes

ek-so reviewed Dec 18, 2024

View reviewed changes

x-pack/plugins/painless_lab/public/styles/_index.scss Outdated

Copy link

Contributor

ek-so Dec 18, 2024

Choose a reason for hiding this comment

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

This removes grey surface under tab bar

ek-so reviewed Dec 18, 2024

View reviewed changes

src/plugins/console/public/application/containers/config/config.tsx Outdated

Copy link

Contributor

ek-so Dec 18, 2024

Choose a reason for hiding this comment

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

eokoneyo force-pushed the fix/resolve-202782 branch from 45b7b51 to e0db868 Compare December 18, 2024 16:32

eokoneyo marked this pull request as ready for review December 19, 2024 12:31

eokoneyo requested a review from a team as a code owner December 19, 2024 12:31

mattkime approved these changes Dec 31, 2024

View reviewed changes

stratoula requested changes Jan 2, 2025

View reviewed changes

eokoneyo force-pushed the fix/resolve-202782 branch from 3db0b7a to 965dc0d Compare January 2, 2025 11:16

eokoneyo and others added 12 commits January 3, 2025 11:16

scaffold for supporting eui color mode aware themes in code editor

793086a

update test to reflect new implementation

31e14de

include tests and use much more suited name for theme augmented methods

6ff606c

switch hardcoded colors for eui colors

d381aeb

Changed some colors

3a73f21

Config > Console settings — made white (grey bg looks buggy)

2bbc5cc

slight adjustments

1eade54

resolve lint type issues

2031cd8

[CI] Auto-commit changed files from 'node scripts/notice'

0305fbf

update test snapshot to match new code editor expectation

cb97cb4

make adjustment to esql theme to keep text colors plain

b30bc9e

throw if an existing theme definition is being overriden without spec…

c724ae5

…ifically opting to override

eokoneyo force-pushed the fix/resolve-202782 branch from 965dc0d to c724ae5 Compare January 3, 2025 10:17

use correct color for base token group

3252105

stratoula approved these changes Jan 3, 2025

View reviewed changes

packages/kbn-monaco/src/esql/lib/esql_theme.ts Show resolved Hide resolved

Dosant approved these changes Jan 3, 2025

View reviewed changes

eokoneyo merged commit 8f01c30 into elastic:main Jan 3, 2025
8 checks passed

eokoneyo deleted the fix/resolve-202782 branch January 3, 2025 14:02

kibanamachine added the v9.0.0 label Jan 3, 2025

SoniaSanzV mentioned this pull request Jan 9, 2025

[Console] When dark mode is set to system console is rendered with the wrong theme #201790

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SharedUX] Modify code editor theme definition and resolution implementation to account for color modes in EUI Theme #203337

[SharedUX] Modify code editor theme definition and resolution implementation to account for color modes in EUI Theme #203337

eokoneyo commented Dec 6, 2024 •

edited

Loading

eokoneyo Dec 6, 2024

eokoneyo Dec 6, 2024

This comment was marked as outdated.

eokoneyo commented Dec 18, 2024

ek-so left a comment

ek-so Dec 18, 2024

ek-so Dec 18, 2024

ek-so Dec 18, 2024

mattkime left a comment

stratoula left a comment

eokoneyo commented Jan 3, 2025

stratoula commented Jan 3, 2025

eokoneyo commented Jan 3, 2025

stratoula left a comment

elasticmachine commented Jan 3, 2025

API count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

Dosant left a comment

[SharedUX] Modify code editor theme definition and resolution implementation to account for color modes in EUI Theme #203337

[SharedUX] Modify code editor theme definition and resolution implementation to account for color modes in EUI Theme #203337

Conversation

eokoneyo commented Dec 6, 2024 • edited Loading

Summary

How to test

eokoneyo Dec 6, 2024

Choose a reason for hiding this comment

eokoneyo Dec 6, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

eokoneyo commented Dec 18, 2024

ek-so left a comment

Choose a reason for hiding this comment

ek-so Dec 18, 2024

Choose a reason for hiding this comment

ek-so Dec 18, 2024

Choose a reason for hiding this comment

ek-so Dec 18, 2024

Choose a reason for hiding this comment

mattkime left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

eokoneyo commented Jan 3, 2025

stratoula commented Jan 3, 2025

eokoneyo commented Jan 3, 2025

stratoula left a comment

Choose a reason for hiding this comment

elasticmachine commented Jan 3, 2025

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

Dosant left a comment

Choose a reason for hiding this comment

eokoneyo commented Dec 6, 2024 •

edited

Loading