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.

Sign up for GitHub

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] user can change axis color #120018

Closed
wants to merge 12 commits into from
Closed

[Lens] user can change axis color #120018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1ea7e04
changing axis color updates visualization state
drewdaemon Nov 29, 2021
a6c9acf
Apply axis colors from state
drewdaemon Nov 30, 2021
00db787
improve assertion style
drewdaemon Nov 30, 2021
a663f5d
Apply label to color picker
drewdaemon Nov 30, 2021
0b0bb3e
Merge branch 'main' into 113755/axis-color-option
drewdaemon Nov 30, 2021
fa78361
fix test typing
drewdaemon Nov 30, 2021
4e1d60b
import default axis color from elastic-charts
drewdaemon Nov 30, 2021
67c0f7b
Preserve color state when generating XY suggestions
drewdaemon Dec 1, 2021
33e933e
Merge branch 'main' of github.com:elastic/kibana into 113755/axis-col…
drewdaemon Dec 2, 2021
a02d1e2
allow for undefined axis colors
drewdaemon Dec 2, 2021
a8f84dd
Remove default color from color picker
drewdaemon Dec 2, 2021
9129a1f
remove old imports
drewdaemon Dec 3, 2021
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
49 changes: 49 additions & 0 deletions x-pack/plugins/lens/common/expressions/xy_chart/axis_config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ export interface AxesSettingsConfig {
yRight: boolean;
}

Copy link
Member

Choose a reason for hiding this comment

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

I'm creating a comment here just to group the conversation under the same thread (is not a comment about that specific line of code):

Quoting the original issue: #113755

When I have multiple axis
And I have a reference line bound to each axis
I need to match the axis to the threshold line color
So I can easily see which threshold belongs to which axis
2nd use case: also improve the integration on canvas where the background color is more likely to be something custom

The described case to me is really specific to a main use case: multiple axis and reference line or, less descriptive case: within Canvas

If we allow the user to freely choose the axis color (and/or the label color) we are achieving the following, unwanted (I hope) situation where:

  1. color is a subjective preference, doesn't necessarily bring the same kind of information and joyfulness to different users. A completely free form selection of an axis color doesn't help and can push us in a dead-end, where are no more able to bring back control over the color choices.
  2. this doesn't align with our effort of helping the user make the right choices
  3. brings us back to the TSVB era, where also the background color was freely selectable by the user and color contrast checks needs to be developed and applied just to that "freedom"
  4. this doesn't work out of the box with the dark/light theme mode we currently have: what happens when a user, in light mode, selects black as color and the next user visualizes the same chart in dark mode?

IMHO you should rediscuss this effort and the possible outcomes

export interface AxesSettingsStringConfig {
x?: string;
yLeft?: string;
yRight?: string;
}

export interface AxisExtentConfig {
mode: 'full' | 'dataBounds' | 'custom';
lowerBound?: number;
Expand Down Expand Up @@ -44,6 +50,49 @@ export interface YConfig {
textVisibility?: boolean;
}

export type AxisColorsConfigResult = AxesSettingsStringConfig & {
type: 'lens_xy_axisColorsConfig';
};

export const axisColorsConfig: ExpressionFunctionDefinition<
'lens_xy_axisColorsConfig',
null,
AxesSettingsStringConfig,
AxisColorsConfigResult
> = {
name: 'lens_xy_axisColorsConfig',
aliases: [],
type: 'lens_xy_axisColorsConfig',
help: `Configure the xy chart's axis titles appearance`,
inputTypes: ['null'],
args: {
x: {
types: ['string'],
help: i18n.translate('xpack.lens.xyChart.xAxisColor.help', {
defaultMessage: 'The color of the x-axis.',
}),
},
yLeft: {
types: ['string'],
help: i18n.translate('xpack.lens.xyChart.yLeftAxisColor.help', {
defaultMessage: 'The color of the left y-axis.',
}),
},
yRight: {
types: ['string'],
help: i18n.translate('xpack.lens.xyChart.yRightAxisColor.help', {
defaultMessage: 'The color of the right y-axis.',
}),
},
},
fn: function fn(input: unknown, args: AxesSettingsStringConfig) {
return {
type: 'lens_xy_axisColorsConfig',
...args,
};
},
};

export type AxisTitlesVisibilityConfigResult = AxesSettingsConfig & {
type: 'lens_xy_axisTitlesVisibilityConfig';
};
Expand Down
7 changes: 6 additions & 1 deletion x-pack/plugins/lens/common/expressions/xy_chart/xy_args.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
* 2.0.
*/

import type { AxisExtentConfigResult, AxisTitlesVisibilityConfigResult } from './axis_config';
import type {
AxisColorsConfigResult,
AxisExtentConfigResult,
AxisTitlesVisibilityConfigResult,
} from './axis_config';
import type { FittingFunction } from './fitting_function';
import type { GridlinesConfigResult } from './grid_lines_config';
import type { LayerArgs } from './layer_config';
Expand All @@ -30,6 +34,7 @@ export interface XYArgs {
layers: LayerArgs[];
fittingFunction?: FittingFunction;
axisTitlesVisibilitySettings?: AxisTitlesVisibilityConfigResult;
axisColorSettings?: AxisColorsConfigResult;
tickLabelsVisibilitySettings?: TickLabelsConfigResult;
gridlinesVisibilitySettings?: GridlinesConfigResult;
labelsOrientation?: LabelsOrientationConfigResult;
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/lens/common/expressions/xy_chart/xy_chart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ export const xyChart: ExpressionFunctionDefinition<
defaultMessage: 'Show x and y axes titles',
}),
},
axisColorSettings: {
types: ['lens_xy_axisColorsConfig'],
help: i18n.translate('xpack.lens.xyChart.axisColorSettings.help', {
defaultMessage: 'Defines axis colors',
}),
},
layers: {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
types: ['lens_xy_layer'] as any,
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/lens/public/expressions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
axisExtentConfig,
yAxisConfig,
axisTitlesVisibilityConfig,
axisColorsConfig,
} from '../common/expressions/xy_chart/axis_config';
import { gridlinesConfig } from '../common/expressions/xy_chart/grid_lines_config';
import { labelsOrientationConfig } from '../common/expressions/xy_chart/labels_orientation_config';
Expand Down Expand Up @@ -53,6 +54,7 @@ export const setupExpressions = (
datatableColumn,
tickLabelsConfig,
axisTitlesVisibilityConfig,
axisColorsConfig,
axisExtentConfig,
labelsOrientationConfig,
getDatatable(formatFactory),
Expand Down
56 changes: 56 additions & 0 deletions x-pack/plugins/lens/public/xy_visualization/__snapshots__/expression.test.tsx.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 23 additions & 1 deletion x-pack/plugins/lens/public/xy_visualization/__snapshots__/to_expression.test.ts.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading