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] Categorical color palettes #75309

Merged
merged 100 commits into from
Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from 94 commits
Commits
Show all changes
100 commits
Select commit Hold shift + click to select a range
bebac29
add toolbar api
flash1293 Jun 16, 2020
e983014
fix teests
flash1293 Jun 16, 2020
3fbff62
fix types
flash1293 Jun 16, 2020
426f2ce
fix functional test
flash1293 Jun 16, 2020
44a17c3
Merge remote-tracking branch 'upstream/master' into lens/toolbar
flash1293 Jun 18, 2020
8c7381e
add back title
flash1293 Jun 18, 2020
64ee8ef
remove separator line
flash1293 Jun 18, 2020
9bb7843
Merge remote-tracking branch 'upstream/master' into lens/toolbar
flash1293 Jun 18, 2020
c1ddaaa
fix i18n
flash1293 Jun 18, 2020
e51ff5e
Update x-pack/plugins/lens/public/editor_frame_service/editor_frame/_…
flash1293 Jun 18, 2020
e3c6f94
Merge remote-tracking branch 'upstream/master' into lens/toolbar
flash1293 Jun 18, 2020
a278772
Merge branch 'lens/toolbar' of github.com:flash1293/kibana into lens/…
flash1293 Jun 18, 2020
bfb6898
Merge remote-tracking branch 'upstream/master' into lens/toolbar
flash1293 Jun 18, 2020
b3fb6e8
Merge remote-tracking branch 'upstream/master' into lens/toolbar
flash1293 Jun 18, 2020
2434486
do not show unsaved when there is no cahrt
flash1293 Jun 18, 2020
85c128e
revert unnecessary changes
flash1293 Jun 18, 2020
1ece895
Merge remote-tracking branch 'upstream/master' into lens/toolbar
flash1293 Jun 23, 2020
34cff58
always show title if available
flash1293 Jun 23, 2020
59ac920
start implementing color picker fundamentals
flash1293 Jun 23, 2020
51ac271
implement palette service
flash1293 Jun 24, 2020
57cfda7
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jun 24, 2020
f65b9fb
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jun 29, 2020
35b421e
keep working on color palettes
flash1293 Jun 29, 2020
e152365
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jun 29, 2020
01d1cd8
fix some things
flash1293 Jun 29, 2020
bf81263
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jun 30, 2020
c3cab97
fix tests
flash1293 Jun 30, 2020
0082554
render single layer tree maps correctly
flash1293 Jun 30, 2020
99266bc
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jul 1, 2020
2094bb4
fix tests
flash1293 Jul 1, 2020
d1cfe2b
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jul 1, 2020
d232351
add tests and so on
flash1293 Jul 1, 2020
b6a36fb
fix tests
flash1293 Jul 1, 2020
b482572
fix types
flash1293 Jul 1, 2020
7e743fc
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jul 2, 2020
d912bcf
improve colors
flash1293 Jul 2, 2020
e1b12b2
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jul 2, 2020
e5e5e56
fix bugs
flash1293 Jul 2, 2020
1707055
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jul 3, 2020
1dc530a
remove unused import
flash1293 Jul 3, 2020
20d28a6
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jul 3, 2020
776e0a0
fix type error
flash1293 Jul 3, 2020
5d9d582
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jul 3, 2020
c13a2a6
fix tests
flash1293 Jul 3, 2020
611d4b5
Merge remote-tracking branch 'upstream/master' into lens/color-palettes
flash1293 Jul 6, 2020
6b7ccc2
format palette picker nicely
flash1293 Jul 6, 2020
5c46182
Merge branch 'master' into lens/color-palettes
elasticmachine Jul 6, 2020
fea7428
update PR
flash1293 Aug 18, 2020
7a99aea
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Aug 19, 2020
3b49960
start moving palettes out of Lens
flash1293 Aug 19, 2020
aafbbda
wire up
flash1293 Aug 20, 2020
161c1d5
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Aug 20, 2020
86137ef
wire up palette service with expression
flash1293 Aug 20, 2020
83cf453
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Sep 14, 2020
99e3d4a
fix some references
flash1293 Sep 14, 2020
0b20bdc
fix type
flash1293 Sep 14, 2020
977450b
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 12, 2020
1df62ba
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 12, 2020
2903a51
wire up palettes in Canvas and start moving Lens palette state manage…
flash1293 Oct 13, 2020
d251edf
move current palette into expression
flash1293 Oct 13, 2020
548a785
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 13, 2020
5e69f35
implement palettes in a bunch of places
flash1293 Oct 16, 2020
c7e8107
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 16, 2020
21583dd
wire up palettes
flash1293 Oct 16, 2020
d0e13f0
fix expression serialization
flash1293 Oct 16, 2020
adc5b3e
pass through palette from canvas
flash1293 Oct 16, 2020
624dec1
wire up palettes in Canvas
flash1293 Oct 16, 2020
c655724
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 16, 2020
dbbb5b3
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 19, 2020
5c78a2c
fix broken tests
flash1293 Oct 19, 2020
8c0cf88
fix broken tests and types
flash1293 Oct 19, 2020
b216c89
fix tests and types
flash1293 Oct 19, 2020
2316149
fix types
flash1293 Oct 19, 2020
babe0de
fix color matching bug
flash1293 Oct 19, 2020
e4b36e0
fix various problems
flash1293 Oct 19, 2020
417346e
lazy load palettes
flash1293 Oct 19, 2020
4508492
fix references
flash1293 Oct 20, 2020
b93e511
add tests
flash1293 Oct 20, 2020
9d25e07
add more palettes and fix existing ones
flash1293 Oct 20, 2020
90c8cd6
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 21, 2020
3875de0
rename and move legacy palette
flash1293 Oct 21, 2020
e558cb1
Merge branch 'master' into lens/categorical-colors
kibanamachine Oct 22, 2020
559d048
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 27, 2020
c54b88d
add tests and rotations to default palette
flash1293 Oct 27, 2020
ba3da6c
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 27, 2020
348e045
fix order of functional tests
flash1293 Oct 27, 2020
dcb7897
Merge branch 'master' into lens/categorical-colors
kibanamachine Oct 28, 2020
64cf4bb
fix multi layer xy chart color assignment
flash1293 Oct 28, 2020
163c8e1
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 29, 2020
5a23904
fix types
flash1293 Oct 29, 2020
e4fd331
Merge remote-tracking branch 'origin/master' into HEAD
wylieconlon Oct 29, 2020
382f71e
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Oct 30, 2020
c2de739
review comments
flash1293 Oct 30, 2020
49f2294
review comments
flash1293 Oct 30, 2020
2575403
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Nov 2, 2020
df33fa2
review comments
flash1293 Nov 2, 2020
7c085ed
refactor color assignment
flash1293 Nov 2, 2020
79065b4
Merge remote-tracking branch 'upstream/master' into lens/categorical-…
flash1293 Nov 3, 2020
6926a6c
review comments
flash1293 Nov 3, 2020
0d20d56
Merge branch 'master' into lens/categorical-colors
kibanamachine Nov 4, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/plugins/charts/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@
*/

export const COLOR_MAPPING_SETTING = 'visualization:colorMapping';
export * from './palette';
107 changes: 107 additions & 0 deletions src/plugins/charts/common/palette.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import {
palette,
defaultCustomColors,
systemPalette,
PaletteOutput,
CustomPaletteState,
} from './palette';
import { functionWrapper } from 'src/plugins/expressions/common/expression_functions/specs/tests/utils';

describe('palette', () => {
flash1293 marked this conversation as resolved.
Show resolved Hide resolved
const fn = functionWrapper(palette()) as (
context: null,
args?: { color?: string[]; gradient?: boolean; reverse?: boolean }
) => PaletteOutput<CustomPaletteState>;

it('results a palette', () => {
const result = fn(null);
expect(result).toHaveProperty('type', 'palette');
});

describe('args', () => {
describe('color', () => {
it('sets colors', () => {
const result = fn(null, { color: ['red', 'green', 'blue'] });
expect(result.params!.colors).toEqual(['red', 'green', 'blue']);
});

it('defaults to pault_tor_14 colors', () => {
const result = fn(null);
expect(result.params!.colors).toEqual(defaultCustomColors);
});
});

describe('gradient', () => {
it('sets gradient', () => {
let result = fn(null, { gradient: true });
expect(result.params).toHaveProperty('gradient', true);

result = fn(null, { gradient: false });
expect(result.params).toHaveProperty('gradient', false);
});

it('defaults to false', () => {
const result = fn(null);
expect(result.params).toHaveProperty('gradient', false);
});
});

describe('reverse', () => {
it('reverses order of the colors', () => {
const result = fn(null, { reverse: true });
expect(result.params!.colors).toEqual(defaultCustomColors.reverse());
});

it('keeps the original order of the colors', () => {
const result = fn(null, { reverse: false });
expect(result.params!.colors).toEqual(defaultCustomColors);
});

it(`defaults to 'false`, () => {
const result = fn(null);
expect(result.params!.colors).toEqual(defaultCustomColors);
});
});
});
});

describe('system_palette', () => {
const fn = functionWrapper(systemPalette()) as (
context: null,
args: { name: string; params?: unknown }
) => PaletteOutput<unknown>;

it('results a palette', () => {
const result = fn(null, { name: 'test' });
expect(result).toHaveProperty('type', 'palette');
});

it('returns the name', () => {
const result = fn(null, { name: 'test' });
expect(result).toHaveProperty('name', 'test');
});

it('passes params through', () => {
const result = fn(null, { name: 'test', params: { customProp: 123 } });
expect(result).toHaveProperty('params', { customProp: 123 });
});
});
175 changes: 175 additions & 0 deletions src/plugins/charts/common/palette.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { ExpressionFunctionDefinition } from 'src/plugins/expressions/common';
import { i18n } from '@kbn/i18n';

export interface CustomPaletteArguments {
color?: string[];
gradient: boolean;
reverse?: boolean;
}

export interface CustomPaletteState {
colors: string[];
gradient: boolean;
}

export interface SystemPaletteArguments {
name: string;
params?: unknown;
}

export interface PaletteOutput<T = unknown> {
type: 'palette';
name: string;
params?: T;
}
export const defaultCustomColors = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export const defaultCustomColors = [
// This set of defaults originated in Canvas, which, at present, is the primary
// consumer of this function. Changing this default requires a change in Canvas
// logic, which would likely be a breaking change in 7.x.
export const defaultCustomColors = [

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, forgot about that, thanks for the great suggestion!

'#882E72',
'#B178A6',
'#D6C1DE',
'#1965B0',
'#5289C7',
'#7BAFDE',
'#4EB265',
'#90C987',
'#CAE0AB',
'#F7EE55',
'#F6C141',
'#F1932D',
'#E8601C',
'#DC050C',
];
Copy link
Contributor

Choose a reason for hiding this comment

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

This choice of palette for the fallback doesn't make a lot of sense to me- why not default to EUI, or throw an error when missing a palette definition?

I worry that we could get stuck with a palette that can never be changed, similar to the 7-color default vislib palette. Also, the name of the palette here is Paul Tol 14

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is done for BWC with Canvas. The palette function is only used by canvas and is already in use there (with these defaults). We can think about changing this with 8.0

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, can you add a code comment about this?


export function palette(): ExpressionFunctionDefinition<
'palette',
null,
CustomPaletteArguments,
PaletteOutput<CustomPaletteState>
> {
return {
name: 'palette',
aliases: [],
type: 'palette',
inputTypes: ['null'],
help: i18n.translate('charts.functions.paletteHelpText', {
defaultMessage: 'Creates a color palette.',
}),
args: {
color: {
aliases: ['_'],
multi: true,
types: ['string'],
help: i18n.translate('charts.functions.palette.args.colorHelpText', {
defaultMessage:
'The palette colors. Accepts an {html} color name, {hex}, {hsl}, {hsla}, {rgb}, or {rgba}.',
values: {
html: 'HTML',
rgb: 'RGB',
rgba: 'RGBA',
hex: 'HEX',
hsl: 'HSL',
hsla: 'HSLA',
},
}),
required: false,
},
gradient: {
types: ['boolean'],
default: false,
help: i18n.translate('charts.functions.palette.args.gradientHelpText', {
defaultMessage: 'Make a gradient palette where supported?',
}),
options: [true, false],
},
reverse: {
types: ['boolean'],
default: false,
help: i18n.translate('charts.functions.palette.args.reverseHelpText', {
defaultMessage: 'Reverse the palette?',
}),
options: [true, false],
},
},
fn: (input, args) => {
const { color, reverse, gradient } = args;
const colors = ([] as string[]).concat(color || defaultCustomColors);

return {
type: 'palette',
name: 'custom',
params: {
colors: reverse ? colors.reverse() : colors,
gradient,
},
};
},
};
}

export function systemPalette(): ExpressionFunctionDefinition<
'system_palette',
null,
SystemPaletteArguments,
PaletteOutput
> {
return {
name: 'system_palette',
aliases: [],
type: 'palette',
inputTypes: ['null'],
help: i18n.translate('charts.functions.systemPaletteHelpText', {
defaultMessage: 'Creates a dynamic color palette.',
}),
args: {
name: {
types: ['string'],
help: i18n.translate('charts.functions.systemPalette.args.nameHelpText', {
defaultMessage: 'Name of the palette in the palette list',
}),
flash1293 marked this conversation as resolved.
Show resolved Hide resolved
options: [
'default',
flash1293 marked this conversation as resolved.
Show resolved Hide resolved
'kibana_palette',
'custom',
'status',
'temperature',
'complimentary',
'negative',
'positive',
'cool',
'warm',
'gray',
],
},
params: {
help: i18n.translate('charts.functions.systemPalette.args.paramsHelpText', {
defaultMessage: 'Palette specific params of the palette',
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what this is supposed to contain- or even what the datatype is. Do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not used right now, but this is the place system palettes would use that to serialize custom params. E.g.
{system_palette id="myFancyPalette" params={fancy_palette_params somethingCustom=true }}. We can also remove it for now if you think that would be better.

Copy link
Contributor

Choose a reason for hiding this comment

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

My preference would be to remove it until needed.

},
},
fn: (input, args) => {
return {
type: 'palette',
name: args.name,
params: args.params,
};
},
};
}
1 change: 1 addition & 0 deletions src/plugins/charts/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"version": "kibana",
"server": true,
"ui": true,
"requiredPlugins": ["expressions"],
"requiredBundles": ["visDefaultEditor"]
}
10 changes: 8 additions & 2 deletions src/plugins/charts/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@ import { ChartsPlugin } from './plugin';

export const plugin = () => new ChartsPlugin();

export type ChartsPluginSetup = ReturnType<ChartsPlugin['setup']>;
export type ChartsPluginStart = ReturnType<ChartsPlugin['start']>;
export { ChartsPluginSetup, ChartsPluginStart } from './plugin';

export * from './static';
export * from './services/palettes/types';
export {
PaletteOutput,
CustomPaletteArguments,
CustomPaletteState,
SystemPaletteArguments,
} from '../common';
10 changes: 7 additions & 3 deletions src/plugins/charts/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,28 @@

import { ChartsPlugin } from './plugin';
import { themeServiceMock } from './services/theme/mock';
import { colorsServiceMock } from './services/colors/mock';
import { colorsServiceMock } from './services/legacy_colors/mock';
import { getPaletteRegistry, paletteServiceMock } from './services/palettes/mock';

export type Setup = jest.Mocked<ReturnType<ChartsPlugin['setup']>>;
export type Start = jest.Mocked<ReturnType<ChartsPlugin['start']>>;

const createSetupContract = (): Setup => ({
colors: colorsServiceMock,
legacyColors: colorsServiceMock,
theme: themeServiceMock,
palettes: paletteServiceMock.setup({} as any, {} as any),
});

const createStartContract = (): Start => ({
colors: colorsServiceMock,
legacyColors: colorsServiceMock,
theme: themeServiceMock,
palettes: paletteServiceMock.setup({} as any, {} as any),
});

export { colorMapsMock } from './static/color_maps/mock';

export const chartPluginMock = {
createSetupContract,
createStartContract,
createPaletteRegistry: getPaletteRegistry,
};
Loading