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

[Lens] Fixes color mappings special tockens i18n #168152

Merged
merged 4 commits into from
Oct 6, 2023

Conversation

markov00
Copy link
Member

@markov00 markov00 commented Oct 5, 2023

Summary

fix #168151

To test:

  • create a cartesian charts with breakdowns that contains both the Other group and an (empty) value
  • apply a manual color mapping that use both these terms by select them from the combo-box, not by typing them.
  • Then change the Kibana language to a different one other than English and see if the color mapping applies correctly.

@markov00 markov00 added bug Fixes for quality problems that affect the customer experience Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.11.0 v8.12.0 backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes and removed backport:skip This commit does not require backporting labels Oct 5, 2023
Copy link
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

Looks good to me. Tested vis with non-auto-assign values of (empty) and Other starting with fr-FR then en then finally ja-JP and all color mappings are correctly assigned even for concatenated fields, even with adding a manual (empty) and Other bucket.

Simple fields

image

Concatenated fields

image

Probably a rare edge case but I think this would break for fields labeled as (empty) or Other. Not worth trying to fix that but if possible in the future would be nice to pass down the original keyed values instead of inverting from a formatted string value.

@stratoula stratoula changed the title [Lens] fix color mappings special tockens i18n [Lens] Fixes color mappings special tockens i18n Oct 6, 2023
@stratoula stratoula marked this pull request as ready for review October 6, 2023 06:51
@stratoula stratoula requested a review from a team as a code owner October 6, 2023 06:51
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

I had to change the translations keys to make CI happy but I added the new keys to our translations files too to make sure that they will be translated.

This works fine, I tested it with french in Chrome

@kibana-ci
Copy link
Collaborator

💚 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
expressionPartitionVis 40.6KB 40.8KB +158.0B
expressionTagcloud 23.0KB 23.1KB +163.0B
expressionXY 131.1KB 131.2KB +158.0B
lens 1.4MB 1.4MB +169.0B
total +648.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@stratoula stratoula merged commit c828ff4 into elastic:main Oct 6, 2023
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 6, 2023
## Summary

fix elastic#168151

To test:
- create a cartesian charts with breakdowns that contains both the
`Other` group and an `(empty)` value
- apply a manual color mapping that use both these terms by select them
from the combo-box, not by typing them.
- Then change the Kibana language to a different one other than English
and see if the color mapping applies correctly.

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
(cherry picked from commit c828ff4)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.11

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 Oct 6, 2023
…68189)

# Backport

This will backport the following commits from `main` to `8.11`:
- [[Lens] Fixes color mappings special tockens i18n
(#168152)](#168152)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Marco
Vettorello","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-10-06T09:21:41Z","message":"[Lens]
Fixes color mappings special tockens i18n (#168152)\n\n##
Summary\r\n\r\nfix
https://github.com/elastic/kibana/issues/168151\r\n\r\nTo test:\r\n-
create a cartesian charts with breakdowns that contains both
the\r\n`Other` group and an `(empty)` value\r\n- apply a manual color
mapping that use both these terms by select them\r\nfrom the combo-box,
not by typing them.\r\n- Then change the Kibana language to a different
one other than English\r\nand see if the color mapping applies
correctly.\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli
<[email protected]>","sha":"c828ff49dbf7f4e8ef28adf832ab40bd495ea2db","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Team:Visualizations","release_note:skip","Feature:Lens","backport:prev-minor","v8.11.0","v8.12.0"],"number":168152,"url":"https://github.com/elastic/kibana/pull/168152","mergeCommit":{"message":"[Lens]
Fixes color mappings special tockens i18n (#168152)\n\n##
Summary\r\n\r\nfix
https://github.com/elastic/kibana/issues/168151\r\n\r\nTo test:\r\n-
create a cartesian charts with breakdowns that contains both
the\r\n`Other` group and an `(empty)` value\r\n- apply a manual color
mapping that use both these terms by select them\r\nfrom the combo-box,
not by typing them.\r\n- Then change the Kibana language to a different
one other than English\r\nand see if the color mapping applies
correctly.\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli
<[email protected]>","sha":"c828ff49dbf7f4e8ef28adf832ab40bd495ea2db"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/168152","number":168152,"mergeCommit":{"message":"[Lens]
Fixes color mappings special tockens i18n (#168152)\n\n##
Summary\r\n\r\nfix
https://github.com/elastic/kibana/issues/168151\r\n\r\nTo test:\r\n-
create a cartesian charts with breakdowns that contains both
the\r\n`Other` group and an `(empty)` value\r\n- apply a manual color
mapping that use both these terms by select them\r\nfrom the combo-box,
not by typing them.\r\n- Then change the Kibana language to a different
one other than English\r\nand see if the color mapping applies
correctly.\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli
<[email protected]>","sha":"c828ff49dbf7f4e8ef28adf832ab40bd495ea2db"}}]}]
BACKPORT-->

Co-authored-by: Marco Vettorello <[email protected]>
dej611 pushed a commit to dej611/kibana that referenced this pull request Oct 17, 2023
## Summary

fix elastic#168151

To test:
- create a cartesian charts with breakdowns that contains both the
`Other` group and an `(empty)` value
- apply a manual color mapping that use both these terms by select them
from the combo-box, not by typing them.
- Then change the Kibana language to a different one other than English
and see if the color mapping applies correctly.

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) bug Fixes for quality problems that affect the customer experience Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.11.0 v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens] Internationalise color mapping special tokens
6 participants