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][Table] Add color by terms with color mappings #189895

Merged
merged 50 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
6c9ff96
[Lens][Table] Add color by terms with color mappings
nickofthyme Aug 5, 2024
4dfb857
Merge branch 'main' into color-mapping-tables
nickofthyme Aug 5, 2024
1737e6d
fix: cell style updates
nickofthyme Aug 5, 2024
160d3dd
fix initial palette when adding new dimension
nickofthyme Aug 5, 2024
b81bb4e
Add color mapping to row dimensions
nickofthyme Aug 5, 2024
5471c2a
Style & copy tweaks to align with mockups
nickofthyme Aug 5, 2024
1c0c53a
derive categories across all matching transposed columns
nickofthyme Aug 6, 2024
8dd3c34
Merge branch 'main' into color-mapping-tables
nickofthyme Aug 6, 2024
5c2a577
fix i18n issues
nickofthyme Aug 7, 2024
3c555cf
Merge remote-tracking branch 'origin/color-mapping-tables' into color…
nickofthyme Aug 7, 2024
82bc97e
Merge branch 'main' into color-mapping-tables
nickofthyme Aug 7, 2024
020dd91
fix type errors in datatable expression
nickofthyme Aug 8, 2024
3613870
fix types and test errors
nickofthyme Aug 13, 2024
ca97848
Merge branch 'main' into color-mapping-tables
nickofthyme Aug 13, 2024
369e020
style: remove empty color when color is applied
nickofthyme Aug 13, 2024
fd5a754
fix legacy color sync on table palettes
nickofthyme Aug 14, 2024
eb2cf04
test: add and fix unit tests
nickofthyme Aug 16, 2024
f3814f9
Merge branch 'main' into color-mapping-tables
nickofthyme Aug 16, 2024
f4c1ab5
cleanup cell value merged logic
nickofthyme Aug 16, 2024
a3ed882
fix i18n error
nickofthyme Aug 19, 2024
c27f325
Merge branch 'main' into color-mapping-tables
nickofthyme Aug 19, 2024
01a70e3
chore: simplify `color_categories.ts` logic
nickofthyme Aug 21, 2024
8517989
chore: color all bucketed columns by terms
nickofthyme Aug 21, 2024
3401c34
chore: set default palette based on color type
nickofthyme Aug 21, 2024
d01d4c3
fix: color bucketed numerics by term
nickofthyme Aug 21, 2024
c9dcb25
chore: update tests for new by terms logic
nickofthyme Aug 21, 2024
75d583b
Merge branch 'main' into color-mapping-tables
nickofthyme Aug 21, 2024
afda04d
test: fix flaky color sync tests
nickofthyme Aug 22, 2024
c4ad8ad
test: fix jest tests due to changes
nickofthyme Aug 22, 2024
46abc0a
test: move getColorStops to lens utils, fix color stops for custom pa…
nickofthyme Aug 22, 2024
70a1939
fix: jest test/type error
nickofthyme Aug 22, 2024
1a8b94d
fix: bad search predicate
nickofthyme Aug 22, 2024
ebdec6f
Merge branch 'main' into color-mapping-tables
nickofthyme Aug 22, 2024
aead73b
chore: remove old nick
nickofthyme Aug 22, 2024
c059e28
fix: range column formatting
nickofthyme Aug 22, 2024
320f160
fix: setting switch font size
nickofthyme Aug 22, 2024
60d44bc
style: revert text style on EuiButtonEmpty
nickofthyme Aug 22, 2024
b6f4754
chore: remove unneeded flex group
nickofthyme Aug 22, 2024
0fb22f6
style: minor UI tweaks
nickofthyme Aug 23, 2024
a94a571
update limits due to increased size of `@kbn/coloring`
nickofthyme Aug 23, 2024
df48141
chore: add `colorMapping` telemetry
nickofthyme Aug 23, 2024
3be2d4f
Merge branch 'main' into color-mapping-tables
nickofthyme Aug 23, 2024
5f0b622
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Aug 23, 2024
dee8b55
Merge branch 'main' into color-mapping-tables
mbondyra Aug 26, 2024
034334a
chore: apply code review changes
nickofthyme Aug 26, 2024
f0f0f47
chore: fix linting errors
nickofthyme Aug 26, 2024
77cd409
fix: palette issue on datasource update
nickofthyme Aug 27, 2024
fed3089
fix: design styles for padding and icon coloring
nickofthyme Aug 27, 2024
5a873fa
Merge branch 'main' into color-mapping-tables
nickofthyme Aug 27, 2024
472dd0f
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Aug 27, 2024
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
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,26 @@ import { Container } from './components/container/container';
import { ColorMapping } from './config';
Copy link
Contributor

Choose a reason for hiding this comment

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

I observed this small bug when switching between categorical/numerical color palettes.

  1. Create a datatable with a last value dimension and categorical color palette set.
  2. switch to a numerical operation, like 'sum'
  3. Click on the color palette - the palette input is not filled properly. I'll pass you the video in a PM (github limits won't allow me to do so)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed this in 77cd409

import { uiReducer } from './state/ui';

export interface ColorMappingInputCategoricalData {
type: 'categories';
/** an ORDERED array of categories rendered in the visualization */
categories: Array<string | string[]>;
}

export interface ColorMappingInputContinuousData {
type: 'ranges';
min: number;
max: number;
bins: number;
}

/**
* A configuration object that is required to populate correctly the visible categories
* or the ranges in the CategoricalColorMapping component
*/
export type ColorMappingInputData =
| {
type: 'categories';
/** an ORDERED array of categories rendered in the visualization */
categories: Array<string | string[]>;
}
| {
type: 'ranges';
min: number;
max: number;
bins: number;
};
| ColorMappingInputCategoricalData
| ColorMappingInputContinuousData;

/**
* The props of the CategoricalColorMapping component
Expand Down
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function rangeMatch(rule: ColorMapping.RuleRange, value: number) {
}

// TODO: move in some data/table related package
export const SPECIAL_TOKENS_STRING_CONVERTION = new Map([
export const SPECIAL_TOKENS_STRING_CONVERSION = new Map([
[
'__other__',
i18n.translate('coloring.colorMapping.terms.otherBucketLabel', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export const Match: React.FC<{
return (
<EuiFlexItem style={{ minWidth: 1, width: 1 }}>
<EuiComboBox
isClearable
data-test-subj={`lns-colorMapping-assignmentsItem${index}`}
fullWidth={true}
aria-label={i18n.translate('coloring.colorMapping.assignments.autoAssignedTermAriaLabel', {
Expand All @@ -82,7 +83,7 @@ export const Match: React.FC<{
placeholder={i18n.translate(
'coloring.colorMapping.assignments.autoAssignedTermPlaceholder',
{
defaultMessage: 'Auto assigned',
defaultMessage: 'Auto assigning term',
}
)}
options={convertedOptions}
Expand All @@ -103,7 +104,6 @@ export const Match: React.FC<{
}
}}
isCaseSensitive
isClearable={false}
compressed
/>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export function AssignmentsConfig({
'coloring.colorMapping.container.mapValuesPromptDescription.mapValuesPromptDetail',
{
defaultMessage:
'Add new assignments to begin associating terms in your data with specified colors.',
'Add a new assignment to manually associate terms with specified colors.',
}
)}
</p>
Expand All @@ -214,7 +214,7 @@ export function AssignmentsConfig({
</EuiButton>,
<EuiButtonEmpty
data-test-subj="lns-colorMapping-assignmentsPromptAddAll"
color="primary"
color="text"
MichaelMarcialis marked this conversation as resolved.
Show resolved Hide resolved
size="xs"
onClick={onClickAddAllCurrentCategories}
>
Expand Down Expand Up @@ -251,6 +251,7 @@ export function AssignmentsConfig({
button={
<EuiButtonIcon
iconType="boxesVertical"
color="text"
aria-label={i18n.translate(
'coloring.colorMapping.container.OpenAdditionalActionsButtonLabel',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export function Container({
>
<EuiButtonIcon
data-test-subj="lns-colorMapping-invertGradient"
color="text"
iconType="merge"
size="xs"
aria-label={i18n.translate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export function ScaleMode({ getPaletteFn }: { getPaletteFn: ReturnType<typeof ge
>
<EuiButtonGroup
legend="Mode"
buttonSize="compressed"
data-test-subj="lns_colorMapping_scaleSwitch"
options={[
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { AVAILABLE_PALETTES, getPalette } from '../palettes';
import { EUIAmsterdamColorBlindPalette } from '../palettes/eui_amsterdam';
import { NeutralPalette } from '../palettes/neutral';
import { getColor, getGradientColorScale } from '../color/color_handling';
import { DEFAULT_FALLBACK_PALETTE, PaletteOutput, PaletteRegistry } from '../../../palettes';

export const DEFAULT_NEUTRAL_PALETTE_INDEX = 1;
export const DEFAULT_OTHER_ASSIGNMENT_INDEX = 0;
Expand All @@ -37,6 +38,22 @@ export const DEFAULT_COLOR_MAPPING_CONFIG: ColorMapping.Config = {
},
};

/**
* Returns array of colors for provided colorMapping or palette
*/
export function getColorStops<T>(
paletteService: PaletteRegistry,
isDarkMode: boolean,
palette?: PaletteOutput<T>,
colorMapping?: ColorMapping.Config
): string[] {
return colorMapping
? getColorsFromMapping(isDarkMode, colorMapping)
: paletteService
.get(palette?.name || DEFAULT_FALLBACK_PALETTE)
.getCategoricalColors(10, palette);
}

export function getPaletteColors(
isDarkMode: boolean,
colorMappings?: ColorMapping.Config
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ export type { ColorMappingInputData } from './categorical_color_mapping';
export type { ColorMapping } from './config';
export * from './palettes';
export * from './color/color_handling';
export { SPECIAL_TOKENS_STRING_CONVERTION } from './color/rule_matching';
export { SPECIAL_TOKENS_STRING_CONVERSION } from './color/rule_matching';
export {
DEFAULT_COLOR_MAPPING_CONFIG,
DEFAULT_OTHER_ASSIGNMENT_INDEX,
getPaletteColors,
getColorsFromMapping,
getColorStops,
} from './config/default_color_mapping';
47 changes: 29 additions & 18 deletions src/plugins/chart_expressions/common/color_categories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,38 @@ import { isMultiFieldKey } from '@kbn/data-plugin/common';
*/
export function getColorCategories(
rows: DatatableRow[],
accessor?: string
accessor?: string,
isTransposed?: boolean,
exclude?: any[]
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
): Array<string | string[]> {
return accessor
? rows.reduce<{ keys: Set<string>; categories: Array<string | string[]> }>(
(acc, r) => {
const value = r[accessor];
if (value === undefined) {
return acc;
}
const ids = isTransposed
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
? Object.keys(rows[0]).filter((key) => accessor && key.endsWith(accessor))
: accessor
? [accessor]
: [];

return rows
.flatMap((r) =>
ids
.map((id) => r[id])
.filter((v) => !(v === undefined || exclude?.includes(v)))
.map((v) => {
// The categories needs to be stringified in their unformatted version.
// We can't distinguish between a number and a string from a text input and the match should
// work with both numeric field values and string values.
const key = (isMultiFieldKey(value) ? [...value.keys] : [value]).map(String);
const key = (isMultiFieldKey(v) ? [...v.keys] : [v]).map(String);
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
const stringifiedKeys = key.join(',');
if (!acc.keys.has(stringifiedKeys)) {
acc.keys.add(stringifiedKeys);
acc.categories.push(key.length === 1 ? key[0] : key);
}
return acc;
},
{ keys: new Set(), categories: [] }
).categories
: [];
return { key, stringifiedKeys };
})
)
.reduce<{ keys: Set<string>; categories: Array<string | string[]> }>(
(acc, { key, stringifiedKeys }) => {
if (!acc.keys.has(stringifiedKeys)) {
acc.keys.add(stringifiedKeys);
acc.categories.push(key.length === 1 ? key[0] : key);
}
return acc;
},
{ keys: new Set(), categories: [] }
).categories;
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ export const getAllSeries = (
/**
* This function joins every data series name available on each layer by the same color palette.
* The returned function `getRank` should return the position of a series name in this unified list by palette.
*
*/
export function getColorAssignments(
layers: CommonXYLayerConfig[],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
getPalette,
AVAILABLE_PALETTES,
NeutralPalette,
SPECIAL_TOKENS_STRING_CONVERTION,
SPECIAL_TOKENS_STRING_CONVERSION,
} from '@kbn/coloring';
import { getColorCategories } from '@kbn/chart-expressions-common';
import { isDataLayer } from '../../common/utils/layer_types_guards';
Expand Down Expand Up @@ -333,7 +333,7 @@ const getColor: GetColorFn = (

const name = getSeriesNameFn(series)?.toString() || '';

const overwriteColors: Record<string, string> = uiState?.get ? uiState.get('vis.colors', {}) : {};
const overwriteColors: Record<string, string> = uiState?.get?.('vis.colors', {}) ?? {};

if (Object.keys(overwriteColors).includes(name)) {
return overwriteColors[name];
Expand Down Expand Up @@ -493,15 +493,15 @@ export const getSeriesProps: GetSeriesPropsFn = ({
// if colorMapping exist then we can apply it, if not let's use the legacy coloring method
layer.colorMapping && splitColumnIds.length > 0
? getColorSeriesAccessorFn(
JSON.parse(layer.colorMapping), // the color mapping is at this point just a strinfigied JSON
JSON.parse(layer.colorMapping), // the color mapping is at this point just a stringified JSON
getPalette(AVAILABLE_PALETTES, NeutralPalette),
isDarkMode,
{
type: 'categories',
categories: getColorCategories(table.rows, splitColumnIds[0]),
},
splitColumnIds[0],
SPECIAL_TOKENS_STRING_CONVERTION
SPECIAL_TOKENS_STRING_CONVERSION
)
: (series) =>
getColor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { i18n } from '@kbn/i18n';
import type { ExecutionContext } from '@kbn/expressions-plugin/common';
import type { FormatFactory, RowHeightMode } from '../../types';
import type { ColumnConfigArg } from './datatable_column';
import type { DatatableColumnConfigArgs } from './datatable_column';
import type { DatatableExpressionFunction } from './types';

export interface SortingState {
Expand All @@ -24,7 +24,7 @@ export interface PagingState {
export interface DatatableArgs {
title: string;
description?: string;
columns: ColumnConfigArg[];
columns: DatatableColumnConfigArgs[];
sortingColumnId: SortingState['columnId'];
sortingDirection: SortingState['direction'];
fitRowToContent?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,25 @@
*/

import type { Direction } from '@elastic/eui';
import type { PaletteOutput, CustomPaletteParams } from '@kbn/coloring';
import type { PaletteOutput, CustomPaletteParams, ColorMapping } from '@kbn/coloring';
import type { CustomPaletteState } from '@kbn/charts-plugin/common';
import type { ExpressionFunctionDefinition, DatatableColumn } from '@kbn/expressions-plugin/common';
import type { SortingHint } from '../../types';
import { CollapseFunction } from '../collapse';

const LENS_DATATABLE_COLUMN = 'lens_datatable_column';

export type LensGridDirection = 'none' | Direction;

export interface ColumnConfig {
columns: ColumnConfigArg[];
export interface DatatableColumnConfig {
columns: DatatableColumnConfigArgs[];
sortingColumnId: string | undefined;
sortingDirection: LensGridDirection;
}

export type ColumnConfigArg = Omit<ColumnState, 'palette'> & {
type: 'lens_datatable_column';
export type DatatableColumnConfigArgs = Omit<ColumnState, 'palette' | 'colorMapping'> & {
palette?: PaletteOutput<CustomPaletteState>;
colorMapping?: string;
summaryRowValue?: unknown;
sortingHint?: SortingHint;
};
Expand All @@ -41,25 +43,28 @@ export interface ColumnState {
bucketValues?: Array<{ originalBucketColumn: DatatableColumn; value: unknown }>;
alignment?: 'left' | 'right' | 'center';
palette?: PaletteOutput<CustomPaletteParams>;
colorMapping?: ColorMapping.Config;
colorMode?: 'none' | 'cell' | 'text';
summaryRow?: 'none' | 'sum' | 'avg' | 'count' | 'min' | 'max';
summaryLabel?: string;
collapseFn?: CollapseFunction;
isMetric?: boolean;
}

export type DatatableColumnResult = ColumnState & { type: 'lens_datatable_column' };
export type DatatableColumnFunction = ExpressionFunctionDefinition<
'lens_datatable_column',
export type DatatableColumnResult = DatatableColumnConfigArgs & {
type: typeof LENS_DATATABLE_COLUMN;
};
export type DatatableColumnFn = ExpressionFunctionDefinition<
typeof LENS_DATATABLE_COLUMN,
null,
ColumnState & { sortingHint?: SortingHint },
DatatableColumnConfigArgs,
DatatableColumnResult
>;

export const datatableColumn: DatatableColumnFunction = {
name: 'lens_datatable_column',
export const datatableColumn: DatatableColumnFn = {
name: LENS_DATATABLE_COLUMN,
aliases: [],
type: 'lens_datatable_column',
type: LENS_DATATABLE_COLUMN,
help: '',
inputTypes: ['null'],
args: {
Expand All @@ -76,12 +81,16 @@ export const datatableColumn: DatatableColumnFunction = {
types: ['palette'],
help: '',
},
colorMapping: {
types: ['string'],
help: '',
},
summaryRow: { types: ['string'], help: '' },
summaryLabel: { types: ['string'], help: '' },
},
fn: function fn(input: unknown, args: ColumnState) {
fn: function fn(input, args) {
return {
type: 'lens_datatable_column',
type: LENS_DATATABLE_COLUMN,
...args,
};
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@

export * from './datatable_column';
export * from './datatable';
export { isTransposeId, getOriginalId } from './transpose_helpers';

export type { DatatableProps, DatatableExpressionFunction } from './types';
Loading