Skip to content

[Lens][Partition] Fix behind text coloring for syncColors#209632

Merged
nickofthyme merged 6 commits intoelastic:mainfrom
nickofthyme:fix-behind-text-coloring
Feb 11, 2025
Merged

[Lens][Partition] Fix behind text coloring for syncColors#209632
nickofthyme merged 6 commits intoelastic:mainfrom
nickofthyme:fix-behind-text-coloring

Conversation

@nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Feb 4, 2025

Summary

When switching to borealis palettes there is no longer a need for euiPaletteColorBlindBehindText palette. Removing this created the linked issue as there was no default applied when accessing the mapped colors. This change attempts to lookup behind text color otherwise defaults to normal color. This was only reachable when the dashboard syncColors option was enabled.

Fixes #209610

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

Release note

This fixes and issues where behind text colors were not correctly assigned, such as in Pie, Treemap and Mosaic charts.

@nickofthyme nickofthyme added release_note:fix Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// Feature:Lens backport:prev-minor labels Feb 4, 2025
@nickofthyme nickofthyme requested a review from a team as a code owner February 4, 2025 19:09
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

@nickofthyme nickofthyme changed the title [Lens][Parition] Fix behind text coloring for syncColors Feb 4, 2025
});

it('should return the same index of the behind text palette for same key', () => {
itIf(legacy)('should return the same index of the behind text palette for same key', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: why not use a simple condition here?

if(legacy){
  it('...')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Idk, I wanted something like playwright allows...

test('skip this test', async ({ page, browserName }) => {
  test.skip(browserName === 'firefox', 'Still working on it');
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 8930532

@nickofthyme nickofthyme requested a review from dej611 February 5, 2025 17:02
@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
charts 17.4KB 17.4KB +32.0B

History

@nickofthyme nickofthyme enabled auto-merge (squash) February 11, 2025 01:53
@nickofthyme nickofthyme merged commit 155577b into elastic:main Feb 11, 2025
9 checks passed
@nickofthyme nickofthyme deleted the fix-behind-text-coloring branch February 11, 2025 03:41
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 9.0

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

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Feb 11, 2025
…209632)

## Summary

When switching to borealis palettes there is no longer a need for
`euiPaletteColorBlindBehindText` palette. Removing this created the
linked issue as there was no default applied when accessing the mapped
colors. This change attempts to lookup behind text color otherwise
defaults to normal color. This was only reachable when the dashboard
`syncColors` option was enabled.

Fixes elastic#209610

### Checklist

- [x] [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)

## Release note

This fixes and issues where behind text colors were not correctly
assigned, such as in `Pie`, `Treemap` and `Mosaic` charts.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 155577b)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
9.0

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Feb 11, 2025
…09632) (#210487)

# Backport

This will backport the following commits from `main` to `9.0`:
- [[Lens][Partition] Fix behind text coloring for &#x60;syncColors&#x60;
(#209632)](#209632)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Nick
Partridge","email":"nicholas.partridge@elastic.co"},"sourceCommit":{"committedDate":"2025-02-11T03:41:09Z","message":"[Lens][Partition]
Fix behind text coloring for `syncColors` (#209632)\n\n##
Summary\r\n\r\nWhen switching to borealis palettes there is no longer a
need for\r\n`euiPaletteColorBlindBehindText` palette. Removing this
created the\r\nlinked issue as there was no default applied when
accessing the mapped\r\ncolors. This change attempts to lookup behind
text color otherwise\r\ndefaults to normal color. This was only
reachable when the dashboard\r\n`syncColors` option was
enabled.\r\n\r\nFixes #209610\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] The PR
description includes the appropriate Release Notes section,\r\nand the
correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n##
Release note\r\n\r\nThis fixes and issues where behind text colors were
not correctly\r\nassigned, such as in `Pie`, `Treemap` and `Mosaic`
charts.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"155577b74bb9da71a1b77c568092b879a09b754a","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Visualizations","Feature:Lens","backport:prev-minor","v9.1.0"],"title":"[Lens][Partition]
Fix behind text coloring for
`syncColors`","number":209632,"url":"https://github.com/elastic/kibana/pull/209632","mergeCommit":{"message":"[Lens][Partition]
Fix behind text coloring for `syncColors` (#209632)\n\n##
Summary\r\n\r\nWhen switching to borealis palettes there is no longer a
need for\r\n`euiPaletteColorBlindBehindText` palette. Removing this
created the\r\nlinked issue as there was no default applied when
accessing the mapped\r\ncolors. This change attempts to lookup behind
text color otherwise\r\ndefaults to normal color. This was only
reachable when the dashboard\r\n`syncColors` option was
enabled.\r\n\r\nFixes #209610\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] The PR
description includes the appropriate Release Notes section,\r\nand the
correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n##
Release note\r\n\r\nThis fixes and issues where behind text colors were
not correctly\r\nassigned, such as in `Pie`, `Treemap` and `Mosaic`
charts.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"155577b74bb9da71a1b77c568092b879a09b754a"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/209632","number":209632,"mergeCommit":{"message":"[Lens][Partition]
Fix behind text coloring for `syncColors` (#209632)\n\n##
Summary\r\n\r\nWhen switching to borealis palettes there is no longer a
need for\r\n`euiPaletteColorBlindBehindText` palette. Removing this
created the\r\nlinked issue as there was no default applied when
accessing the mapped\r\ncolors. This change attempts to lookup behind
text color otherwise\r\ndefaults to normal color. This was only
reachable when the dashboard\r\n`syncColors` option was
enabled.\r\n\r\nFixes #209610\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] The PR
description includes the appropriate Release Notes section,\r\nand the
correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n##
Release note\r\n\r\nThis fixes and issues where behind text colors were
not correctly\r\nassigned, such as in `Pie`, `Treemap` and `Mosaic`
charts.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"155577b74bb9da71a1b77c568092b879a09b754a"}}]}]
BACKPORT-->

Co-authored-by: Nick Partridge <nicholas.partridge@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Lens release_note:fix Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v9.0.0 v9.1.0

4 participants