palette context type alignment

[ppisljar]

Various teams writing expression functions and renderers come across requirement to work with color palletes. This is an effort to align on palette context type and a set of functions around it that can serve the needs of all teams and will allow us to integrate better.

some related issues:
#61977
#69349

canvas current state:
currently canvas has a pallete function, which accepts array of colors and some aditional parameters (gradient, reverse) and outputs the pallete context type, with the following interface:

interface Output {
  type: 'palette';
  colors: string[];
  gradient: boolean;
}

lens current state:
lens is still working on color palette support, but they plan to use EUI color palettes http://eui.elastic.co/#/utilities/color-palettes
for that reason they are planing on providing a set of expression functions to work with them #69800

proposal:

• app arch would take ownership over palette context type and palette function and move them over to expressions plugin
• we would change the palette type to have the following form:

interface Palette {
  type: 'palette',
  name: 'name-of-palette',
  params: { // additional palette params }
}

• we would provide getPallete(palette: Pallete): PalleteHANDLER utility function which would return a set of functions to help with color generation

interface PalleteHANDLER<T> {
  getCategoricalColor: (series: SeriesDefinition, params: T): string,
  getColors: (size: number): string[],
  getGradientColor: (...),
}

• pallete context type would define a converter from array of strings to the pallete context type. this way we don't need to migrate any existing canvas expressions as an array of colors (what canvas currently has) would get migrated to

{
  type: 'pallete',
  name: 'custom',
  params: {
    colors: [original, array, of, colors],
  }
}

• we would extend palette expression function to optionally take in a palette name and in this case lookup the palette with EUI palettes. Of course we will also change it to output correct new palette context type

• extend palette function with theme support, to support theming in canvas (and in lens/visualize/dashboard in the future)

• extend palette function with global overrides support, allowing you to override the color for specific series in all the charts in specific workpad/dashboard

• extend palette function with user overrides support, allowing setting themes/colors on user level (uiSettings)

• provide functions for specific palettes as a wrapper around main palette function (for easier consumption)

example usage:

pie palette={palette} // gets you default palette
pie palette={palette name='euiPaletteColorBlind'} // gets you the eui colorblind palette
pie palette={euiPaletteColorBlind} // wrapper function for colorblind palette
pie palette={euiPaletteColorBlind rotations=3} // wrapper function for colorblind palette
pie palette={palette 'custom' color='#fffff' color='#aaaaa'}  // palette with custom provided colors
pie palette={customPalette color='#fffff' color='#aaaaa' gradient=true}  // custom palette wrapper = current palette function

[elasticmachine]

Pinging @elastic/kibana-app-arch (Team:AppArch)

[elasticmachine]

Pinging @elastic/kibana-canvas (Team:Canvas)

[ppisljar]

how getColors utility function could look like (probably oversimplified):

const paletteMap = {
  euiPaletteComplimentary,
  euiPaletteForStatus,
  euiPaletteForTemperature,
  euiPaletteCool,
  euiPaletteWarm,
  euiPaletteNegative,
  euiPalettePositive,
  euiPaletteGray,
  euiPaletteColorBlind,
  colorPalette,
}

export const getColors(palette: Pallete, size: number) {
  if (palette.name === 'euiPaletteColorBlind') {
    return euiPaletteColorBlind({ ...palette.params });
  } else if (palette.name === 'custom') {
    return colorPalette(palette.params.colors, size);
  }
  return paletteMap[palette.name](size)
}

in the future we could extend this with palette registry to which you could register a palette + its custom getColors function

[clintandrewhall]

Some initial feedback:

• We should just start with theme, of which palette is a member, and deprecate the palette parameter.
  • We know we're going there, we should avoid adding palette as a top-level function parameter.
  • Otherwise, it's a breaking change to existing Canvas workpads, (pie already has palette, and we're changing the API)
  • e.g. pie theme={theme palette={palette name="euiFooBar"} font={font name="Arial"} }}
  • this also will prevent breaking changes to Canvas workpads
• Defining an array of colors individually, rather than as an array, seems tedious.
• Have we given any other thought to defaulting arguments, particularly the theme?

[clintandrewhall]

Honestly, replace palette with theme in most of the comments, e.g. "theme registry". We're not going to stop with color palettes, I guarantee it. The font function currently lives in the expressions plugin, and we're already seeing cases where Reporting will need to access a font in order to produce a PDF... there will be more.

[ppisljar]

I agree we'll get to the theme point soon, yet this alignment across teams is not easy so thats why i think it makes sense to start with a smaller task.

changing palette type should not be a breaking change for canvas due to the from-converter we will define in the palette content type which should be able to convert from the current canvas palette content type to the new one. (will do a PoC about this)

changing the function implementation could be, so maybe we should leave the old palette function, deprecate it and add a new one colorPalette to avoid conflicts

the defaulting arguments or rather how can we override colors per theme, per user setting or per some global thing was thought thru, the idea being that we have this single function for colors and when we introduce themes this function will be checking (when no arguments provided) to see if theme color was defined or any additional overrides. The exact details were not flushed out yet as that seems to be a second step and we are fairly convinced this approach will work.

lets keep this a palette discussion and try to align on this one. We can start the theme discussion at the same time, but my preference would be to take a look at aligning on font usage and then we can go from there and define a theme as font + colors together and hopefully have an easier task to do as we will have aligned on font and colors by then.

[flash1293]

Just a side note - when we implement the theme mode (override the chosen palette based on execution context), we should provide a flag to opt out of that. There are cases where you want to color everything in the same way except for some specific (parts of) visualizations. Example: A "traffic light" kind of visualization which only makes sense when the "status" palette is used (even if the rest of the workpad / dashboard is using something else)

[ppisljar]

@flash1293 agreed, but lets discuss that here: #71064

[clintandrewhall]

@ppisljar We really, really should start with theme. You have to remember: Canvas workpads can be exported. Every time we deprecate and change the expression signature, we have to deal with inconsistencies between minors:

1/ someone creates a workpad in 7.8, then imports it in 7.10, we have to migrate the workpad every time it's uploaded. I'm fine with adding, but removing becomes a major hurdle we haven't had to tackle yet because we avoid it.
2/ if someone happens to create a workpad in 7.9, offers it in a blog post, and someone can't use it in 7.8.

If we know we want to get to theme in the expression before 8.0 (and we do), we should start there.

I mean, what is the difference we're talking about here? Adding colorPalette to pie (and anything else) and figuring out a deprecation strategy for palette... and then, deprecating colorPalette with theme...? That's a lot of churn we can avoid:

pie palette={palette "xyz"}
pie palette={...} colorPalette={palette "xyz"} <- why do we need to do this at all?
pie palette={...} colorPalette={...} theme={theme colorPalette={palette "xyz"}}

[flash1293]

If we define theme as { type: 'theme', colorPalette: ColorPalette } (without any functionality) I can hardly see that make a difference in practice.

@ppisljar I can include this in my initial PR setting up palettes in the charts plugin.

[flash1293]

Synced with @clintandrewhall and @poffdeluxe and we would like to start with this:

Using a unified infrastructure for palettes makes sense and we should definitely do this as it opens up lots of options in the long-term. However overloading the palette function is not the right thing to do right now because it would allow users to accidentally use palettes they shouldn't use (because the palettes we define for Lens are not a good fit for canvas in their current shape for various reasons). To avoid this, we can go with two functions:
{palette "color1" "color2" "color3"} - used in Canvas
{system_palette name="eui_warm"} - used in Lens (tagged so it doesn't show up in Canvas editor)

Both of these functions are generating the same context:

interface Palette {
  type: 'palette',
  name: 'name-of-palette',
  params: { // additional palette params }
}

The palette function will create a context like this:

{
  type: 'palette',
  name: 'custom',
  params: {
    colors: ['red', 'green', 'blue']
  }
}

Everything else works like defined in the original description, the actual logic for picking colors is provided by the charts plugin, and both Lens and Canvas use the same infrastructure to color their charts. This allows us to use the palette defined in Canvas in an embedded Lens visualization, without accidentally exposing internals in Canvas. In a later step (probably 8.0 because it breaks some basic assumptions) we can also expose those system palettes in Canvas. For now, the Canvas palettes will stay the same.

[flash1293]

cc @ppisljar see the above addition to the proposal. If you are fine with this, I think I can get started on it.

[stacey-gammon]

I want to try to rename the issue title to be more "outside in". Would this be accurate:

"Add support for Canvas Themes in Lens Embeddables"?

Should this issue be owned by KibanaApp, with Canvas as the requester? (Dependency:Canvas)?

[ppisljar]

@flash1293 sounds good to me

[flash1293]

Great, I am going to implement this soon.

[flash1293]

Closed by #75309