[Visual Refresh] Borealis visualization palettes #201015
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nickofthyme
added
ci:project-deploy-elasticsearch
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
PR Project deployment started at: https://buildkite.com/elastic/kibana-deploy-project-from-pr/builds/145 |
9 tasks
nickofthyme
added
ci:project-deploy-elasticsearch
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
Project deployed, see credentials at: https://buildkite.com/elastic/kibana-deploy-project-from-pr/builds/145 |
nickofthyme
removed
ci:project-deploy-elasticsearch
…alis-vis-palettes
sebelga
approved these changes
Dec 5, 2024
nickofthyme
commented
Dec 5, 2024
src/plugins/charts/public/services/palettes/decrease_opacity.ts
Outdated
Show resolved
Hide resolved
nickofthyme
commented
Dec 5, 2024
x-pack/plugins/cases/public/components/visualizations/actions/open_modal.test.tsx
Outdated
Show resolved
Hide resolved
Ikuni17
approved these changes
Dec 5, 2024
adcoelho
approved these changes
Dec 9, 2024
markov00
reviewed
Dec 10, 2024
markov00
reviewed
Dec 10, 2024
| isDarkMode | ||
| ), | ||
| ]; | ||
| ? Array.from({ length: palette.colorCount }, (d, i) => palette.getColor(i)) |
There was a problem hiding this comment.
The previous palette management provides a way to require a color by "looping" the palette color set or by not looping that.
Here instead it looks like we always loop by default.
I believe this is fine (we always want to loop) but I'm wondering if we are missing a requirements there where we don't want to loop
There was a problem hiding this comment.
Yeah I wasn't sure where this looping came into play. Is there a way in the UI to set the specialAssignments such that the colors do not loop? If not I would agree to remove it until it is needed.
src/plugins/chart_expressions/expression_tagcloud/public/components/tagcloud_component.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
+1754
to
+1760
| palettes={ | ||
| KbnPalettes { | ||
| "get": [Function], | ||
| "getAll": [Function], | ||
| "query": [Function], | ||
| } | ||
| } |
There was a problem hiding this comment.
If you look right above that there is also another thing called paletteService how can we replace that and how can we avoid having such similar expressions?
There was a problem hiding this comment.
Yeah this is my attempt to clarify the usage of the legacy charts/palette service as the paletteService and the new @kbn/palettes as palettes.
Previously the legacy palette is called palettes, paletteService, paletteRegistry, etc.
I think this is something we tackle when we deprecate the legacy charts palette service. For now they live side-by-side.
There was a problem hiding this comment.
ok thanks, can you write a simple issue to solve this tech debt? thanks
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Any counts in public APIs
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
async chunk count
History
|
markov00
approved these changes
Dec 11, 2024
nickofthyme
changed the title
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this pull request
Dec 12, 2024
Adds support for toggling between the old Amsterdam themed visualizations palettes and the new Borealis themed palettes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for toggling between the old Amsterdam themed visualizations palettes, to the new Borealis themed palettes.
When toggling between the
AmsterdamandBorealistheme, the palettes are now theme-based. So any visualization that uses the default kibana palette will now use the Borealis palettes. The same applies to gradients excludingcustomgradient palettes, see note below.Key changes:
@kbn/palettepackage. This uses a customMap-like class for getting palettes based on id. This allows for aliasing new palettes to old palette ids at runtime. So accessingpalettes.get('old_amseterdam')would return the new default palette when the borealis theme is selected. These palettes are generated differently based on the selected theme.charts/services/palettes) hard codes color stops in the SO even for non-custompalettes. This poses an issue for swapping the default palettes to use Borealis, this goes for categorical and gradient palettes. Luckily, any change to the palette will convert the palette tocustom. We use this behavior to update the default palettes to use the new colors by ignoring the hardcoded colors and generating new colors for the current named palette except forcustompalettes. In short non-custom palette color stops are overridden based on the selected palette.Closes #192848
Running locally
Enabled the theme naming toggle in advanced settings.
Start kibana with
experimentaltheme files loaded.Checklist
release_note:*label is applied per the guidelines