[TSVB] Integrates the color service #93749

stratoula · 2021-03-05T09:44:11Z

Summary

This PR integrates the new palette service to TSVB. TSVB uses three palettes to color the series when a user split the series by Terms. If the user changes the palette dropdown without having split the series by terms this setting is not applied. I kept the same logic in order to not introduce breaking changes.

The integration with the new service provides us the following advantages:

We use the same system with Lens and XY axis plugin
Now the color sync works on a dashboard for the TSVB visualizations too
Now the user has more palettes choices (I kept the existing three and added the rest EUI palettes).

Note: I tried to be backwards compatible and avoid writing a migration script so the old key split_color_mode is kept and used to distinguish between the new and the old visualizations.

TSVB with an EUI Palette:

Palette Picker with the existing gradient and rainbow palettes

Color sync on dashboard (TSVB and Lens charts):

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

stratoula · 2021-03-08T08:51:59Z

@elasticmachine merge upstream

stratoula · 2021-03-09T10:37:43Z

@elasticmachine merge upstream

stratoula · 2021-03-09T15:25:10Z

@elasticmachine merge upstream

stratoula · 2021-03-10T14:01:38Z

@elasticmachine merge upstream

stratoula · 2021-03-15T08:39:46Z

@elasticmachine merge upstream

stratoula · 2021-03-16T07:17:08Z

@elasticmachine merge upstream

stratoula · 2021-03-16T09:17:55Z

src/plugins/vis_type_timeseries/public/application/components/vis_types/timeseries/config.js

+
+  const handlePaletteChange = (val) => {
+    props.onChange({
+      split_color_mode: null,


In order to be backwards compatible I haven't removed the split_color_mode. Otherwise, I had to write a migration script to get its value and use it as the palette name. I preferred to skip the migration script.

stratoula · 2021-03-18T06:33:39Z

@elasticmachine merge upstream

flash1293

Just a note: The gradient color seems to look very slightly different (should be fine):

An extisting saved TSVB vis renders like this though:

Also I can't find a configuration of the "Default palette" in the old version - can a chart still look like this?

src/plugins/vis_type_timeseries/public/application/visualizations/views/timeseries/index.js

stratoula · 2021-03-18T16:31:14Z

@elasticmachine merge upstream

stratoula · 2021-03-18T16:50:22Z

@flash1293 thanx a lot for the review, I will look into your comments soonish. About the Default palette it seems that is actually the vislib palette.

Old implementation Default Palette

New implementation kibana_palette

I just saw that the mapping is not exactly the same - I will look into this - but I think that the current one is correct as I have created the same chart on the xy axis plugin and the color mapping is the same.

flash1293 · 2021-03-18T16:57:19Z

I just saw that the mapping is not exactly the same - I will look into this - but I think that the current one is correct as I have created the same chart on the xy axis plugin and the color mapping is the same.

Yeah, that's what I meant. I'm realizing this just now, but it looks like it's shifted by one and otherwise the same.

stratoula · 2021-03-22T06:36:58Z

@elasticmachine merge upstream

stratoula · 2021-03-22T12:28:43Z

@flash1293 I am fetching the palettes on the renderer as you proposed. About the other issues:

Gradient palette. Tbh I can't see the difference. Can you help me here? For example. here is a chart on master and on my branch. They seem the same to me. Unless I am missing something here.

About the default palette. It seems that the legacy color service excludes the first color of the series (it applies this to the count aggregation and then uses the rest colors for the terms). But this doesn't make sense to TSVB. The count agg color is given by the user so I think that the current implementation is the correct one (also matches Lens and XY axis plugin). wdyt?

flash1293 · 2021-03-22T12:51:37Z

I recorded this gif, switching between the two versions. It super close, I wouldn't worry

About the default palette. It seems that the legacy color service excludes the first color of the series (it applies this to the count aggregation and then uses the rest colors for the terms). But this doesn't make sense to TSVB. The count agg color is given by the user so I think that the current implementation is the correct one (also matches Lens and XY axis plugin). wdyt?

Oh, that's actually an important point - if you are using the palette service, visualization:colorMapping won't be respected anymore (we did that on purpose because we didn't want to pull in this behavior in Lens). This means vis_type_xy (and TSVB after this PR) would drop support for visualization:colorMapping which can be regarded as a breaking change. Not sure what we should do here tbh...

Legacy TSVB visualizations work fine now btw.

stratoula · 2021-03-22T13:13:23Z

After syncing offline with Joe, we decided that we want the advanced setting visualization:colorMapping to be respected for the compatibility palette. I have created an issue here

flash1293

Tested and works as expected, LGMT

The only weird thing I noticed is that empty string is not treated the same way but we can also fix this on the Lens side (it's a pending tech debt item)

stratoula · 2021-03-22T17:42:22Z

Hmmm good point, and on xy axis works like that

I suggest to fix TSVB to work as XY axis. It uses the color from the editor and not the palette mapping. I will fix it tmr

flash1293 · 2021-03-22T17:46:22Z

I suggest to fix TSVB to work as XY axis, wdyt

I'm fine with that as well. If you do, could you leave a note here #68060 so this doesn't break once we clean up the handling in Lens?

stratoula · 2021-03-22T17:47:26Z

of course :)

stratoula · 2021-03-22T18:17:29Z

Fixed, now (empty) has the same color for Lens, XY and TSVB viz

kibanamachine · 2021-03-22T20:26:01Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`visTypeTimeseries`	472	477	+5

Async chunks

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

id	before	after	diff
`visTypeTimeseries`	1.6MB	1.6MB	+13.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`visTypeTimeseries`	123.9KB	124.4KB	+421.0B

History

💚 Build #114248 succeeded 72f99c9
💔 Build #114232 failed 2209dfe
💚 Build #114043 succeeded 4bf3c08
💚 Build #113906 succeeded 2ba5bd6
💚 Build #113315 succeeded 9cebf0f

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

* [TSVB] Integrates the color service * Fix i18n failure * Sync colors :) * Fix unit tests * Apply the multiple colors also for gauge * Fix * More unit tests * Cleanup * Be backwards compatible * Fetch palettesService on vis renderer * Fix eslint * Fix jest test * Fix color mapping for empty labels Co-authored-by: Kibana Machine <[email protected]>

* [TSVB] Integrates the color service * Fix i18n failure * Sync colors :) * Fix unit tests * Apply the multiple colors also for gauge * Fix * More unit tests * Cleanup * Be backwards compatible * Fetch palettesService on vis renderer * Fix eslint * Fix jest test * Fix color mapping for empty labels Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

stratoula added 3 commits March 5, 2021 11:42

[TSVB] Integrates the color service

f255ceb

Fix i18n failure

5c01295

Sync colors :)

ee76843

kibanamachine and others added 2 commits March 8, 2021 03:52

Merge branch 'master' into tsvb-color-service

98b1458

Fix unit tests

1ca740e

kibanamachine and others added 2 commits March 9, 2021 05:37

Merge branch 'master' into tsvb-color-service

86825b2

Apply the multiple colors also for gauge

6fe3a53

Merge branch 'master' into tsvb-color-service

faa375b

kibanamachine and others added 2 commits March 10, 2021 09:01

Merge branch 'master' into tsvb-color-service

803d816

Fix

63fc286

Merge branch 'master' into tsvb-color-service

7ae5bda

kibanamachine and others added 3 commits March 16, 2021 03:17

Merge branch 'master' into tsvb-color-service

73a55d3

More unit tests

10a8d10

Cleanup

9cebf0f

stratoula commented Mar 16, 2021

View reviewed changes

stratoula marked this pull request as ready for review March 16, 2021 13:01

stratoula requested a review from a team March 16, 2021 13:01

Merge branch 'master' into tsvb-color-service

2ba5bd6

flash1293 reviewed Mar 18, 2021

View reviewed changes

src/plugins/vis_type_timeseries/public/application/visualizations/views/timeseries/index.js Outdated Show resolved Hide resolved

Merge branch 'master' into tsvb-color-service

4bf3c08

kibanamachine and others added 5 commits March 22, 2021 02:37

Merge branch 'master' into tsvb-color-service

b784936

Be backwards compatible

08f54ab

Fetch palettesService on vis renderer

aedea65

Fix eslint

2209dfe

Fix jest test

72f99c9

stratoula mentioned this pull request Mar 22, 2021

Color mapping should work for the compatibility palette #95052

Closed

2 tasks

flash1293 approved these changes Mar 22, 2021

View reviewed changes

Fix color mapping for empty labels

643cfa6

stratoula merged commit 7d303eb into elastic:master Mar 23, 2021

stratoula mentioned this pull request Mar 23, 2021

[7.x] [TSVB] Integrates the color service (#93749) #95132

Merged

stratoula mentioned this pull request Mar 23, 2021

[Lens] Refactor handling of empty values #68060

Closed

stratoula mentioned this pull request Apr 23, 2021

[TSVB] Visualization crashes when it is opened from Metrics Ui #98120

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TSVB] Integrates the color service #93749

[TSVB] Integrates the color service #93749

stratoula commented Mar 5, 2021 •

edited

Loading

stratoula commented Mar 8, 2021

stratoula commented Mar 9, 2021

stratoula commented Mar 9, 2021

stratoula commented Mar 10, 2021

stratoula commented Mar 15, 2021

stratoula commented Mar 16, 2021

stratoula Mar 16, 2021 •

edited

Loading

stratoula commented Mar 18, 2021

flash1293 left a comment

stratoula commented Mar 18, 2021

stratoula commented Mar 18, 2021

flash1293 commented Mar 18, 2021

stratoula commented Mar 22, 2021

stratoula commented Mar 22, 2021

flash1293 commented Mar 22, 2021 •

edited

Loading

stratoula commented Mar 22, 2021

flash1293 left a comment

stratoula commented Mar 22, 2021 •

edited

Loading

flash1293 commented Mar 22, 2021

stratoula commented Mar 22, 2021

stratoula commented Mar 22, 2021

kibanamachine commented Mar 22, 2021

[TSVB] Integrates the color service #93749

[TSVB] Integrates the color service #93749

Conversation

stratoula commented Mar 5, 2021 • edited Loading

Summary

Checklist

stratoula commented Mar 8, 2021

stratoula commented Mar 9, 2021

stratoula commented Mar 9, 2021

stratoula commented Mar 10, 2021

stratoula commented Mar 15, 2021

stratoula commented Mar 16, 2021

stratoula Mar 16, 2021 • edited Loading

Choose a reason for hiding this comment

stratoula commented Mar 18, 2021

flash1293 left a comment

Choose a reason for hiding this comment

stratoula commented Mar 18, 2021

stratoula commented Mar 18, 2021

flash1293 commented Mar 18, 2021

stratoula commented Mar 22, 2021

stratoula commented Mar 22, 2021

flash1293 commented Mar 22, 2021 • edited Loading

stratoula commented Mar 22, 2021

flash1293 left a comment

Choose a reason for hiding this comment

stratoula commented Mar 22, 2021 • edited Loading

flash1293 commented Mar 22, 2021

stratoula commented Mar 22, 2021

stratoula commented Mar 22, 2021

kibanamachine commented Mar 22, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

stratoula commented Mar 5, 2021 •

edited

Loading

stratoula Mar 16, 2021 •

edited

Loading

flash1293 commented Mar 22, 2021 •

edited

Loading

stratoula commented Mar 22, 2021 •

edited

Loading