[ML] EUI Visual Refresh (tokens) by walterra · Pull Request #203518 · elastic/kibana

walterra · 2024-12-10T07:54:34Z

Summary

Part of #199715 (EUI Visual Refresh).

Recommend to review with white-space diff disabled: https://github.com/elastic/kibana/pull/203518/files?w=1

All references to renamed tokens have been updated to use the new token name.
All usage of color palette tokens and functions now pull from the theme, and correctly update to use new colors when the theme changes from Borealis to Amsterdam and vice versa.
Migrated some data visualizer related SCSS to emotion, part of [ML] Replace legacy SCSS overwrites with css-in-js #140695.
It makes use of EUI's own useEuiTheme() instead of our own hook variants. So this gets rid of useCurrentEuiThemeVars(), useFieldStatsFlyoutThemeVars(), useCurrentEuiTheme(), useCurrentThemeVars().
Renamed components used to edit Anomaly Detection jobs from JobDetails, Detectors, Datafeed to EditJobDetailsTab, EditDetectorsTab, EditDatafeedTab to make them less ambiguous.
Added unit tests for ner_output.tsx.
Adds checks to pick euiColorVis* colors suitable for the theme.

Some of our code used colors like euiColorVis5_behindText. In Borealis *_behindText is no longer available since the original colors like euiColorVis5 have been updated to be suitable to be used behind text. For that reason and for simplicity's sake I removed the border from the custom badges we use to render NER items:

NER labels Amsterdam:

NER labels Borealis:

Checklist

Unit or functional tests were updated or added to match the most common scenarios
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

elasticmachine · 2024-12-11T18:24:33Z

Pinging @elastic/ml-ui (:ml)

...orm/plugins/shared/ml/public/application/components/collapsible_panel/panel_header_items.tsx

x-pack/platform/plugins/shared/ml/public/application/explorer/annotation_timeline.tsx

peteharverson

Tested and visually LGTM - tested Borealis (dark theme) and Amsterdam (light theme).

andreadelrio

Left some comments about visColors.

andreadelrio · 2024-12-12T22:57:04Z

.../plugins/shared/ml/public/application/model_management/test_models/models/ner/ner_output.tsx

    icon: 'visMapCoordinate',
-    color: 'euiColorVis1_behindText',
-    borderColor: 'euiColorVis1',
+    color: 'euiColorVis1',


Hi @walterra, visColors mapping has changed in Borealis (as seen here):

So here if you want to preserve the blue you'll probably want to do euiColorVis2 instead.

@walterra @andreadelrio Fyi, changing colors like this means the current color in Amsterdam changes as well. (in Amsterdam it will be red now)
If that's not wanted, we'd need to use conditional color definitions.

Thanks @mgadewoll. Afaict these vis colors are not being used in a semantic way (e.g. low/healthy, warning, danger, etc) so we should be safe to make this change this way. Can you confirm @walterra?

andreadelrio · 2024-12-12T22:57:20Z

.../plugins/shared/ml/public/application/model_management/test_models/models/ner/ner_output.tsx

    icon: 'questionInCircle',
-    color: 'euiColorVis7_behindText',
-    borderColor: 'euiColorVis7',
+    color: 'euiColorVis7',


Suggested change

color: 'euiColorVis7',

color: 'euiColorVis8',

andreadelrio · 2024-12-12T22:57:42Z

.../plugins/shared/ml/public/application/model_management/test_models/models/ner/ner_output.tsx

  icon: 'questionInCircle',
-  color: 'euiColorVis5_behindText',
-  borderColor: 'euiColorVis5',
+  color: 'euiColorVis5',


Suggested change

color: 'euiColorVis5',

color: 'euiColorVis9',

andreadelrio · 2024-12-12T22:58:09Z

...ication/model_management/test_models/models/question_answering/question_answering_output.tsx

  return (
    <EuiBadge
-      color={euiTheme.euiColorVis5_behindText}
+      color={euiTheme.colors.vis.euiColorVis5}


Suggested change

color={euiTheme.colors.vis.euiColorVis5}

color={euiTheme.colors.vis.euiColorVis9}

…_flyout.tsx

walterra · 2024-12-17T15:30:04Z

@mgadewoll @andreadelrio went through all the vis colors and added a flag an appropriate colors where necessary.

walterra · 2024-12-17T15:37:04Z

@rbrtj Thanks for spotting these issues! Fixed the analytics map icons in 8250674. Fixed the histogram chart colors in d4346ac. Please have another look!

rbrtj

Latest changes LGTM 👍

...lugins/shared/ml/public/application/data_frame_analytics/pages/job_map/components/legend.tsx

mgadewoll

Thanks for the additional changes and making sure Amsterdam colors have parity. 👏
✅ Changes LGTM from EUI side

elasticmachine · 2024-12-18T13:46:41Z

💚 Build Succeeded

Buildkite Build
Commit: dc5b902

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dataVisualizer`	736	725	-11
`ml`	2138	2137	-1
`transform`	489	487	-2
total			-14

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/ml-kibana-theme`	3	1	-2

Async chunks

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

id	before	after	diff
`aiops`	621.5KB	620.7KB	-747.0B
`dataVisualizer`	616.4KB	614.6KB	-1.8KB
`ml`	4.7MB	4.7MB	+1.4KB
`transform`	475.9KB	475.4KB	-419.0B
total			-1.6KB

Page load bundle

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

id	before	after	diff
`aiops`	17.3KB	17.5KB	+208.0B
`dataVisualizer`	26.3KB	26.3KB	-54.0B
`ml`	78.4KB	77.8KB	-608.0B
`transform`	18.6KB	18.5KB	-54.0B
total			-508.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/ml-kibana-theme`	5	2	-3

History

💚 Build #261454 succeeded 8250674
💛 Build #261266 was flaky 199f989
💚 Build #261223 succeeded 368ebd0
💔 Build #261151 failed f9f0a06
💛 Build #260656 was flaky 47ab04c
💔 Build #260518 failed cc2f35e

cc @walterra

## Summary Part of elastic#199715 (EUI Visual Refresh). Recommend to review with white-space diff disabled: https://github.com/elastic/kibana/pull/203518/files?w=1 - All references to renamed tokens have been updated to use the new token name. - All usage of color palette tokens and functions now pull from the theme, and correctly update to use new colors when the theme changes from Borealis to Amsterdam and vice versa. - Migrated some data visualizer related SCSS to emotion, part of elastic#140695. - It makes use of EUI's own `useEuiTheme()` instead of our own hook variants. So this gets rid of `useCurrentEuiThemeVars()`, `useFieldStatsFlyoutThemeVars()`, `useCurrentEuiTheme()`, `useCurrentThemeVars()`. - Renamed components used to edit Anomaly Detection jobs from `JobDetails`, `Detectors`, `Datafeed` to `EditJobDetailsTab`, `EditDetectorsTab`, `EditDatafeedTab` to make them less ambiguous. - Added unit tests for `ner_output.tsx`. - Adds checks to pick `euiColorVis*` colors suitable for the theme. ---- Some of our code used colors like `euiColorVis5_behindText`. In Borealis `*_behindText` is no longer available since the original colors like `euiColorVis5` have been updated to be suitable to be used behind text. For that reason and for simplicity's sake I removed the border from the custom badges we use to render NER items: NER labels Amsterdam: ![CleanShot 2024-12-18 at 11 37 45@2x](https://github.com/user-attachments/assets/d82bca3a-2ad1-411a-94cd-748de6b4b0e9) NER labels Borealis: ![CleanShot 2024-12-18 at 11 38 45@2x](https://github.com/user-attachments/assets/36987779-fab2-4ad7-8e31-6853d48079a1) ### Checklist - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

walterra self-assigned this Dec 10, 2024

walterra force-pushed the ml-borealis-fixes-color-palettes branch from b18326f to 63e1448 Compare December 10, 2024 13:38

walterra added 4 commits December 10, 2024 18:30

theme fixes

53841ed

wip: ner output fixes

2ed192b

ner_output color fixes

8344d67

qa_output color fixes

a1ec963

walterra force-pushed the ml-borealis-fixes-color-palettes branch from 63e1448 to a1ec963 Compare December 10, 2024 17:32

JasonStoltz added the EUI Visual Refresh label Dec 10, 2024

walterra added 7 commits December 11, 2024 10:43

update time series explorer styles

ae7f54d

euiColorVis*

ea7175b

tabColor

5f1d0f7

euiColorVis*

a7e5ed2

remove useCurrentEuiTheme()

0ff287d

remove useCurrentEuiThemeVars()

6f414b4

linting

14c4402

walterra force-pushed the ml-borealis-fixes-color-palettes branch from d186aa6 to 14c4402 Compare December 11, 2024 14:12

walterra marked this pull request as ready for review December 11, 2024 18:23

walterra requested a review from a team as a code owner December 11, 2024 18:23

walterra requested review from peteharverson and rbrtj December 11, 2024 18:24

walterra added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting v9.0.0 :ml labels Dec 11, 2024

peteharverson mentioned this pull request Dec 12, 2024

[ML] Increase Test Coverage 8.18.0 #201813

Closed

11 tasks

mgadewoll reviewed Dec 12, 2024

View reviewed changes

...orm/plugins/shared/ml/public/application/components/collapsible_panel/panel_header_items.tsx Outdated Show resolved Hide resolved

x-pack/platform/plugins/shared/ml/public/application/explorer/annotation_timeline.tsx Outdated Show resolved Hide resolved

peteharverson approved these changes Dec 12, 2024

View reviewed changes

simplify border

a664eee

andreadelrio requested changes Dec 12, 2024

View reviewed changes

walterra added 13 commits December 17, 2024 09:07

euiColorVis event_rate_chart.tsx

e0b73f6

euiColorVis group_color_utils.ts

f30a476

euiColorVis use_color_range.ts

f0a5893

euiColorVis cytoscape_options.tsx

db1f8a3

euiColorVis /job_map/components/legend.tsx

de6695a

euiColorVis jobs_list/components/datafeed_chart_flyout/datafeed_chart…

86a0d8e

…_flyout.tsx

euiColorVis ner_output.tsx

f3d368f

euiColorVis question_answering_output.tsx

9d0ce59

euiColorVis forecastColor

199f989

refactor _top_values.scss to emotion

4814bd8

fix barColor in field stats expanded row

3de1397

remove obsolete scss

4372840

fix chart colors

d4346ac

fix analytics map colors

8250674

walterra mentioned this pull request Dec 18, 2024

[ML] Replace legacy SCSS overwrites with css-in-js #140695

Closed

71 tasks

walterra changed the title ~~[ML] Borealis theme fixes (tokens)~~ [ML] EUI Visual Refresh (tokens) Dec 18, 2024

fix NER badge colors (again)

532f718

rbrtj approved these changes Dec 18, 2024

View reviewed changes

mgadewoll reviewed Dec 18, 2024

View reviewed changes

...lugins/shared/ml/public/application/data_frame_analytics/pages/job_map/components/legend.tsx Outdated Show resolved Hide resolved

revert change to euiBorderThin

dc5b902

mgadewoll approved these changes Dec 18, 2024

View reviewed changes

walterra merged commit 0b14669 into elastic:main Dec 18, 2024

walterra deleted the ml-borealis-fixes-color-palettes branch December 18, 2024 13:50

	color={euiTheme.colors.vis.euiColorVis5}
	color={euiTheme.colors.vis.euiColorVis9}

Conversation

walterra commented Dec 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

elasticmachine commented Dec 11, 2024

Uh oh!

Uh oh!

Uh oh!

peteharverson left a comment

Choose a reason for hiding this comment

Uh oh!

andreadelrio left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreadelrio Dec 12, 2024

Choose a reason for hiding this comment

Uh oh!

mgadewoll Dec 16, 2024

Choose a reason for hiding this comment

Uh oh!

andreadelrio Dec 16, 2024

Choose a reason for hiding this comment

Uh oh!

andreadelrio Dec 12, 2024

Choose a reason for hiding this comment

Uh oh!

andreadelrio Dec 12, 2024

Choose a reason for hiding this comment

Uh oh!

andreadelrio Dec 12, 2024

Choose a reason for hiding this comment

Uh oh!

walterra commented Dec 17, 2024

Uh oh!

walterra commented Dec 17, 2024

Uh oh!

rbrtj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mgadewoll left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Dec 18, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

walterra commented Dec 10, 2024 •

edited

Loading

andreadelrio left a comment •

edited

Loading