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

[ML] AIOps: Field stats fix #204403

Merged
merged 1 commit into from
Dec 16, 2024
Merged

[ML] AIOps: Field stats fix #204403

merged 1 commit into from
Dec 16, 2024

Conversation

walterra
Copy link
Contributor

Summary

Fixes a regression introduced in #203579.

EuiText accepts EUI colors like primary as well as hex colors. The PR above introduced getPercentageColor() which assumed all colors passed in are hex colors. In case it was an EUI color token the code threw an error and the fields stats popover wouldn't fully render. This fix adds an additional check to apply makeHighContrastColor only if the color provided is a hex color.

Before:

CleanShot 2024-12-16 at 14 50 15@2x

After:

CleanShot 2024-12-16 at 14 49 20@2x

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

@walterra walterra added :ml release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v9.0.0 EUI Visual Refresh labels Dec 16, 2024
@walterra walterra self-assigned this Dec 16, 2024
@walterra walterra requested a review from a team as a code owner December 16, 2024 13:53
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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
aiops 621.4KB 621.5KB +53.0B
apm 3.3MB 3.3MB +53.0B
discover 786.1KB 786.1KB +53.0B
lens 1.5MB 1.5MB +53.0B
ml 4.7MB 4.7MB +53.0B
securitySolution 14.7MB 14.7MB +53.0B
slo 851.1KB 851.2KB +53.0B
transform 475.8KB 475.9KB +53.0B
total +424.0B

cc @walterra

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested (Borealis and Amsterdam themes) and LGTM.

@walterra walterra merged commit be1f60a into elastic:main Dec 16, 2024
19 checks passed
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Dec 19, 2024
## Summary

Fixes a regression introduced in
elastic#203579.

`EuiText` accepts EUI colors like `primary` as well as hex colors. The
PR above introduced `getPercentageColor()` which assumed all colors
passed in are hex colors. In case it was an EUI color token the code
threw an error and the fields stats popover wouldn't fully render. This
fix adds an additional check to apply `makeHighContrastColor` only if
the color provided is a hex color.

Before:

![CleanShot 2024-12-16 at 14 50
15@2x](https://github.com/user-attachments/assets/53a5df17-da4a-4188-a80c-5dc52cae9f39)

After:

![CleanShot 2024-12-16 at 14 49
20@2x](https://github.com/user-attachments/assets/75fc093a-6cf6-4a21-bc10-05dd7d253c70)

### 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)
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Jan 13, 2025
## Summary

Fixes a regression introduced in
elastic#203579.

`EuiText` accepts EUI colors like `primary` as well as hex colors. The
PR above introduced `getPercentageColor()` which assumed all colors
passed in are hex colors. In case it was an EUI color token the code
threw an error and the fields stats popover wouldn't fully render. This
fix adds an additional check to apply `makeHighContrastColor` only if
the color provided is a hex color.

Before:

![CleanShot 2024-12-16 at 14 50
15@2x](https://github.com/user-attachments/assets/53a5df17-da4a-4188-a80c-5dc52cae9f39)

After:

![CleanShot 2024-12-16 at 14 49
20@2x](https://github.com/user-attachments/assets/75fc093a-6cf6-4a21-bc10-05dd7d253c70)

### 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting EUI Visual Refresh :ml release_note:skip Skip the PR/issue when compiling release notes v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants