[One Discover] Update log.level indicators color

[tonyghiani]

📓 Summary

Closes #202258

This change updates the colors scale for Discover's log.level indicators to differentiate errors from other levels better.

N.B. As this relies on some hard-coded values defined here, it is not a definitive version, but a middle step to enhance the scale in v8.x versions.
With the introduction of the Borealis theme in v9, a new scale token-based will replace this.

[elasticmachine]

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

[ryankeairns]

@tonyghiani without having run it locally 😬 , have you checked the color contrast levels? I am wondering if those pass our WCAG AA target.

[davismcphee]

Thanks @tonyghiani! The colours seem to match the mockups for the most part, just seeing one issue in dark mode with Amsterdam.

Amsterdam light
I believe this looks as expected:

Amsterdam dark
It looks like fatal here is lighter than the lower error levels:

Also I know we don't plan to release Borealis in 8.x, but if we merge this PR as is, we'd need to make sure we implement the final colours in main before 9.0 FF since otherwise they don't match at all in Borealis.

Borealis light

Borealis dark

Also attaching some screenshots from main for comparison:

Amsterdam light in main

Amsterdam dark in main

Borealis light in main

Borealis dark in main

@ryankeairns The EuiBadge component is supposed to automatically adjust the text colour based on the background, but I agree it doesn't really seem like there's enough contrast for some of the colours. I think we raised this when we first implemented the badges since there's a similar issue for the original colours, but were told by design that there was enough contrast.

[tonyghiani]

It looks like fatal here is lighter than the lower error levels:

@davismcphee you are right, and this is probably due the fact that some of these colors are hard-coded in the palette definition provided in #186273 (comment).
I don't know if those colors come from a palette value that we can generate, @gvnmagni can you precise how did you generate those colors please? Is there a dark mode alternative for them?

@ryankeairns regarding the contrast on the badge, I noticed the badge switched it automatically to adjust the contrast, so I guess it was good enough given it's an automated check in EUI.

[tonyghiani]

@davismcphee I removed these assertions against the color because it make the test much more difficult to maintain (manual converting colors to rgba) and it is already unit tested in get_log_level_color.test.ts module.

[gvnmagni]

Few notes here, thank you for raising the issue:

This solution for Amsterdam has been created adjusting the previously existing color palette. This has been done interpolating colors from the EUI Danger color to our Datavis Red-ish tone, as you can see from the following image.

One important thing to notice is that this was meant to be a quick fix for Amsterdam so that we could stop using a sort of Orange for Alerts, that was misleading, but in the end what I am planning to do is to create a proper Severity Color Palette for Borealis that could be shared across the product because right now we are using completely different set of colors in different places.

I am currently in the middle of this phase and I am testing out possibilities for this palette, I will probably be able to share it next week and after we discuss about it all together we can plan for a proper release. I am not sure when we can have it, it really depends on EUI Team capacity, I would target 9.1, but maybe we can find a way to fit that in 9.0... it really need to be discussed first though, take all this with a grain of salt

[tonyghiani]

Thanks for the details @gvnmagni, the problem raised by Davis was that, if we cannot rely on a colors generator that does the interpolation and correctly generates good colors in dark mode too, hard-coding them might create unexpected colors scheme in dark mode. Is it possible to generate those interpolation colors dynamically with EUI utils instead of hardcoding them?

[gvnmagni]

there might actually be, in EUI there are a few functions for generating color palettes (https://eui.elastic.co/#/utilities/color-palettes), if you look into the Quantitative Color Scale you would see that there is a RED color function that might help us.

I would need to understand a detail though, and pardon me because I am clearly missing part of the context and I am trying to catch up, why this become an issue only after this update? I mean, before changing these set of red shades that we introduce with this PR, didn't we have this same issue with the previously hard-coded values?

[tonyghiani]

euiPaletteForStatus seems the one to go for, but it doesn't reflect the colors you had in the screenshot. If the colors generated by that functions looks good to you, I'll go for them.

[gvnmagni]

I would not go for the status because it includes green shades that we don't want and a yellow inflection point that is undesired since it will create orange shades.

I am still not fully understanding why we have to rely on EUI functions though, we had hardcoded values before this PR right? Why can't we keep having them but just use the updated version?

[tonyghiani]

I am still not fully understanding why we have to rely on EUI functions though, we had hardcoded values before this PR right? Why can't we keep having them but just use the updated version?

We didn't have hard-coded values, they were from a palette generated with euiPaletteForStatus, but those values didn't match the colors on the palette you proposed. We can keep using that palette as we do now, but only using the upper part of the palette with red colors.

Alternatively, for the error badges, we can use the colors from the red palette highlighted in the following screenshot, which would give us a gradient of the 5 colors we need for handling errors.

[gvnmagni]

I am still not fully understanding why we have to rely on EUI functions though, we had hardcoded values before this PR right? Why can't we keep having them but just use the updated version?

We didn't have hard-coded values, they were from a palette generated with euiPaletteForStatus, but those values didn't match the colors on the palette you proposed. We can keep using that palette as we do now, but only using the upper part of the palette with red colors.

Alternatively, for the error badges, we can use the colors from the red palette highlighted in the following screenshot, which would give us a gradient of the 5 colors we need for handling errors.

Ooooh now I see, my original assumption was completely wrong, thank you for making it clear to me!

Please proceed as you mention, using the highlighted color palette from EUI so that we have better control of everything. I just want to be sure that this is ok for everybody @LucaWintergerst @davismcphee @ninoslavmiskovic

[tonyghiani]

@gvnmagni @davismcphee added the latest update using the red palette.

[LucaWintergerst]

thanks @gvnmagni and @tonyghiani - looks good to me, thanks for getting ahead of the dark mode issues and apologies for the confusion

[davismcphee]

Latest code changes look good, and the new colours looks great in both light and dark mode! Thanks @tonyghiani and @gvnmagni for the design work 👍

Amsterdam light

Amsterdam dark

They even look nice in Borealis. I know we intend to change these anyway, but it's nice that the colours look good in all configs in main until then.

Borealis light

Borealis dark

[ninoslavmiskovic]

Looks good 👏

[gvnmagni]

Linking this new proposal for Borealis so that people who were interested/involved in this issues could have a look: #203387

[mgadewoll]

Changes LGTM from EUI side 👍

[yngrdyn]

LGTM 👨🏼‍🎨

[elasticmachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 94bb880

Failed CI Steps

• FTR Configs #92

Test Failures

• [job] [logs] FTR Configs #92 / Entity Manager _search API includes source and additional metadata fields

Metrics [docs]

Async chunks

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

id	before	after	diff
discover	785.8KB	785.9KB	+106.0B
unifiedDocViewer	115.8KB	115.9KB	+107.0B
total			+213.0B

History

• 💔 Build #259736 failed e1b08b9
• 💚 Build #259127 succeeded 4305109
• 💚 Build #257515 succeeded b9dcd3e
• 💚 Build #257286 succeeded 44c1401
• 💔 Build #257048 failed d95ad08

[kibanamachine]

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12299215623

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

[mgadewoll]

ℹ️ We noticed too late that this PR actually is not Borealis specific.
Hence we need to revert the suggested change as it won't work on backports.

[tonyghiani]

PR to revert the change: #204054

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 202985 locally

[tonyghiani]

The backport for this PR was necessary, but it would have failed due to a mistakenly used token from the new EUI theme.
The issue was addressed and everything was backported with #204092