From dc03598737b1fc8d621f10a2cefb4b1ceea6f800 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 7 Sep 2020 14:19:59 +0200 Subject: [PATCH 01/18] implement color indicators --- .../config_panel/_layer_panel.scss | 9 ++ .../editor_frame/config_panel/layer_panel.tsx | 83 ++++++++++++++----- x-pack/plugins/lens/public/types.ts | 5 +- .../xy_visualization/xy_visualization.tsx | 37 ++++++++- 4 files changed, 109 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss index 62bc6d7ed7cc8a..50f95bb22357ec 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss @@ -47,3 +47,12 @@ width: $euiSize * 30; padding: $euiSizeS; } + +.lnsLayerPanel__colorIndicator { + margin-left: $euiSizeS; +} + +.lnsLayerPanel__colorIndicator--solidColor { + width: $euiSizeS; + height: $euiSizeS; +} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index b45dd13bfa4fd6..b522b8210733a5 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -14,6 +14,7 @@ import { EuiButtonEmpty, EuiFormRow, EuiTabbedContent, + EuiIcon, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; @@ -171,7 +172,9 @@ export function LayerPanel( } > <> - {group.accessors.map((accessor) => { + {group.accessors.map((accessorConfig) => { + const accessor = + typeof accessorConfig === 'string' ? accessorConfig : accessorConfig.columnId; const tabs = [ { id: 'datasource', @@ -275,27 +278,63 @@ export function LayerPanel( accessor={accessor} groupId={group.groupId} trigger={ - { - if (popoverState.isOpen) { - setPopoverState(initialPopoverState); - } else { - setPopoverState({ - isOpen: true, - openId: accessor, - addingToGroupId: null, // not set for existing dimension - tabId: 'datasource', - }); - } - }, - }} - /> + + {typeof accessorConfig !== 'string' && + accessorConfig.triggerIcon === 'color' && + accessorConfig.color && ( + +
+ + )} + {typeof accessorConfig !== 'string' && + accessorConfig.triggerIcon === 'disabled' && ( + + + + )} + {typeof accessorConfig !== 'string' && + accessorConfig.triggerIcon === 'colorBy' && ( + + + + )} + + { + if (popoverState.isOpen) { + setPopoverState(initialPopoverState); + } else { + setPopoverState({ + isOpen: true, + openId: accessor, + addingToGroupId: null, // not set for existing dimension + tabId: 'datasource', + }); + } + }, + }} + /> + + } panel={ ; supportsMoreColumns: boolean; /** If required, a warning will appear if accessors are empty */ required?: boolean; diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx index 8c551c575764e4..adb795e42e6f1a 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx @@ -10,6 +10,7 @@ import { render } from 'react-dom'; import { Position } from '@elastic/charts'; import { I18nProvider } from '@kbn/i18n/react'; import { i18n } from '@kbn/i18n'; +import { EUI_CHARTS_THEME_LIGHT } from '@elastic/eui/dist/eui_charts_theme'; import { getSuggestions } from './xy_suggestions'; import { LayerContextMenu, XyToolbar, DimensionEditor } from './xy_config_panel'; import { Visualization, OperationMetadata, VisualizationType } from '../types'; @@ -160,7 +161,13 @@ export const xyVisualization: Visualization = { }, getConfiguration(props) { - const layer = props.state.layers.find((l) => l.layerId === props.layerId)!; + const layerIndex = props.state.layers.findIndex((l) => l.layerId === props.layerId); + const layer = props.state.layers[layerIndex]; + const chartContainsSplits = props.state.layers.some(({ splitAccessor }) => splitAccessor); + const layerContainsSplits = Boolean(layer.splitAccessor); + const allAccessors = props.state.layers.flatMap((currentLayer) => + currentLayer.accessors.map((currentAccessor) => ({ currentLayer, currentAccessor })) + ); return { groups: [ { @@ -180,7 +187,33 @@ export const xyVisualization: Visualization = { groupLabel: i18n.translate('xpack.lens.xyChart.yAxisLabel', { defaultMessage: 'Y-axis', }), - accessors: layer.accessors, + accessors: layer.accessors.map((accessor) => { + const currentYConfig = layer.yConfig?.find( + (yConfig) => yConfig.forAccessor === accessor + ); + const previousLayerContainsSplit = props.state.layers + .slice(0, layerIndex) + .some(({ splitAccessor }) => splitAccessor); + if (layerContainsSplits || (!currentYConfig?.color && previousLayerContainsSplit)) { + return { + columnId: accessor, + triggerIcon: 'disabled', + }; + } + const accessorIndex = allAccessors.findIndex( + (a) => a.currentAccessor === accessor && a.currentLayer === layer + ); + const seriesColor = + currentYConfig?.color || + EUI_CHARTS_THEME_LIGHT.theme.colors!.vizColors![ + accessorIndex % EUI_CHARTS_THEME_LIGHT.theme.colors!.vizColors!.length + ]; + return { + columnId: accessor, + triggerIcon: 'color', + color: seriesColor, + }; + }), filterOperations: isNumericMetric, supportsMoreColumns: true, required: true, From 1118d0c0b72e4e2cab7f2bd7407185b74adfb055 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 7 Sep 2020 14:20:36 +0200 Subject: [PATCH 02/18] add icon for color by --- .../plugins/lens/public/xy_visualization/xy_visualization.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx index adb795e42e6f1a..6f56c5caec33a9 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx @@ -225,7 +225,9 @@ export const xyVisualization: Visualization = { groupLabel: i18n.translate('xpack.lens.xyChart.splitSeries', { defaultMessage: 'Break down by', }), - accessors: layer.splitAccessor ? [layer.splitAccessor] : [], + accessors: layer.splitAccessor + ? [{ columnId: layer.splitAccessor, triggerIcon: 'colorBy' }] + : [], filterOperations: isBucketed, suggestedPriority: 0, supportsMoreColumns: !layer.splitAccessor, From 66f4bd10afc202af8e64b40a2ec1ed8be4dc88b3 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 7 Sep 2020 15:01:05 +0200 Subject: [PATCH 03/18] fix styling --- .../config_panel/_layer_panel.scss | 4 + .../editor_frame/config_panel/layer_panel.tsx | 123 ++++++++++-------- .../dimension_panel/dimension_panel.tsx | 10 +- x-pack/plugins/lens/public/types.ts | 1 - .../xy_visualization/xy_visualization.tsx | 1 - 5 files changed, 74 insertions(+), 65 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss index 50f95bb22357ec..ec90b45eaba9d4 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss @@ -43,6 +43,10 @@ min-height: $euiSizeXXL; } +.lnsLayerPanel__dimensionLink { + width: 100%; +} + .lnsLayerPanel__styleEditor { width: $euiSize * 30; padding: $euiSizeS; diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index b522b8210733a5..2a0af8fe4c7649 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -15,6 +15,7 @@ import { EuiFormRow, EuiTabbedContent, EuiIcon, + EuiLink, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; @@ -217,6 +218,19 @@ export function LayerPanel( }); } + const togglePopover = () => { + if (popoverState.isOpen) { + setPopoverState(initialPopoverState); + } else { + setPopoverState({ + isOpen: true, + openId: accessor, + addingToGroupId: null, // not set for existing dimension + tabId: 'datasource', + }); + } + }; + return ( - {typeof accessorConfig !== 'string' && - accessorConfig.triggerIcon === 'color' && - accessorConfig.color && ( - -
- - )} - {typeof accessorConfig !== 'string' && - accessorConfig.triggerIcon === 'disabled' && ( - - - - )} - {typeof accessorConfig !== 'string' && - accessorConfig.triggerIcon === 'colorBy' && ( - - - - )} - - { - if (popoverState.isOpen) { - setPopoverState(initialPopoverState); - } else { - setPopoverState({ - isOpen: true, - openId: accessor, - addingToGroupId: null, // not set for existing dimension - tabId: 'datasource', - }); - } - }, - }} - /> - - + { + togglePopover(); + }} + > + + {typeof accessorConfig !== 'string' && + accessorConfig.triggerIcon === 'color' && + accessorConfig.color && ( + +
+ + )} + {typeof accessorConfig !== 'string' && + accessorConfig.triggerIcon === 'disabled' && ( + + + + )} + {typeof accessorConfig !== 'string' && + accessorConfig.triggerIcon === 'colorBy' && ( + + + + )} + + + + + } panel={ { - props.togglePopover(); - }} data-test-subj="lns-dimensionTrigger" aria-label={i18n.translate('xpack.lens.configure.editConfig', { defaultMessage: 'Edit configuration', @@ -251,7 +249,7 @@ export const IndexPatternDimensionTriggerComponent = function IndexPatternDimens })} > {uniqueLabel} - + ); }; diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 5a7c16c680b567..4946fcab6edb95 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -221,7 +221,6 @@ export type DatasourceDimensionEditorProps = DatasourceDimensionPro export type DatasourceDimensionTriggerProps = DatasourceDimensionProps & { dragDropContext: DragContextState; - togglePopover: () => void; }; export interface DatasourceLayerPanelProps { diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx index 6f56c5caec33a9..f4e61479a11185 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx @@ -163,7 +163,6 @@ export const xyVisualization: Visualization = { getConfiguration(props) { const layerIndex = props.state.layers.findIndex((l) => l.layerId === props.layerId); const layer = props.state.layers[layerIndex]; - const chartContainsSplits = props.state.layers.some(({ splitAccessor }) => splitAccessor); const layerContainsSplits = Boolean(layer.splitAccessor); const allAccessors = props.state.layers.flatMap((currentLayer) => currentLayer.accessors.map((currentAccessor) => ({ currentLayer, currentAccessor })) From d5ba1b38a8db07962fb21b2a29c2f512caa7c56f Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 7 Oct 2020 16:26:13 +0200 Subject: [PATCH 04/18] expose active data in some places --- .../public/react_expression_renderer.tsx | 9 +++++++ .../editor_frame/editor_frame.tsx | 1 + .../editor_frame/state_management.ts | 11 +++++++++ .../workspace_panel/workspace_panel.tsx | 17 +++++++++++++ .../editor_frame_service/merge_tables.ts | 15 +++++++++--- .../lens/public/editor_frame_service/types.ts | 12 ++++++++++ x-pack/plugins/lens/public/types.ts | 24 ++++++++++++++++--- .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - 9 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugins/lens/public/editor_frame_service/types.ts diff --git a/src/plugins/expressions/public/react_expression_renderer.tsx b/src/plugins/expressions/public/react_expression_renderer.tsx index 12476c70044b58..4b04d85e9d7e06 100644 --- a/src/plugins/expressions/public/react_expression_renderer.tsx +++ b/src/plugins/expressions/public/react_expression_renderer.tsx @@ -38,6 +38,7 @@ export interface ReactExpressionRendererProps extends IExpressionLoaderParams { renderError?: (error?: string | null) => React.ReactElement | React.ReactElement[]; padding?: 'xs' | 's' | 'm' | 'l' | 'xl'; onEvent?: (event: ExpressionRendererEvent) => void; + onData$?: (data: TData, adapters?: TInspectorAdapters) => void; /** * An observable which can be used to re-run the expression without destroying the component */ @@ -67,6 +68,7 @@ export const ReactExpressionRenderer = ({ renderError, expression, onEvent, + onData$, reload$, ...expressionLoaderOptions }: ReactExpressionRendererProps) => { @@ -114,6 +116,13 @@ export const ReactExpressionRenderer = ({ }) ); } + if (onData$) { + subs.push( + expressionLoaderRef.current.data$.subscribe((newData) => { + onData$(newData, expressionLoaderRef.current?.inspect()); + }) + ); + } subs.push( expressionLoaderRef.current.loading$.subscribe(() => { hasHandledErrorRef.current = false; diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx index 32fd4461dfc8bc..fcdd4bb13eb8ce 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx @@ -99,6 +99,7 @@ export function EditorFrame(props: EditorFrameProps) { const framePublicAPI: FramePublicAPI = { datasourceLayers, + activeData: state.activeData, dateRange: props.dateRange, query: props.query, filters: props.filters, diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts index fc8daaed059ddf..e0101493b27aab 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts @@ -6,6 +6,7 @@ import { EditorFrameProps } from './index'; import { Document } from '../../persistence/saved_object_store'; +import { TableInspectorAdapter } from '../types'; export interface PreviewState { visualization: { @@ -21,6 +22,7 @@ export interface EditorFrameState extends PreviewState { description?: string; stagedPreview?: PreviewState; activeDatasourceId: string | null; + activeData?: TableInspectorAdapter; } export type Action = @@ -32,6 +34,10 @@ export type Action = type: 'UPDATE_TITLE'; title: string; } + | { + type: 'UPDATE_ACTIVE_DATA'; + tables: TableInspectorAdapter; + } | { type: 'UPDATE_STATE'; // Just for diagnostics, so we can determine what action @@ -139,6 +145,11 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta return { ...state, title: action.title }; case 'UPDATE_STATE': return action.updater(state); + case 'UPDATE_ACTIVE_DATA': + return { + ...state, + activeData: action.tables, + }; case 'UPDATE_LAYER': return { ...state, diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx index af44fc28fec153..2fade292829631 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx @@ -40,6 +40,7 @@ import { } from '../../../../../../../src/plugins/data/public'; import { WorkspacePanelWrapper } from './workspace_panel_wrapper'; import { DropIllustration } from '../../../assets/drop_illustration'; +import { LensInspectorAdapters } from '../../types'; export interface WorkspacePanelProps { activeVisualizationId: string | null; @@ -245,6 +246,7 @@ export function WorkspacePanel({ expression={expression} framePublicAPI={framePublicAPI} timefilter={plugins.data.query.timefilter.timefilter} + dispatch={dispatch} onEvent={onEvent} setLocalState={setLocalState} localState={localState} @@ -289,11 +291,13 @@ export const InnerVisualizationWrapper = ({ setLocalState, localState, ExpressionRendererComponent, + dispatch, }: { expression: Ast | null | undefined; framePublicAPI: FramePublicAPI; timefilter: TimefilterContract; onEvent: (event: ExpressionRendererEvent) => void; + dispatch: (action: Action) => void; setLocalState: (dispatch: (prevState: WorkspaceState) => WorkspaceState) => void; localState: WorkspaceState; ExpressionRendererComponent: ReactExpressionRendererType; @@ -317,6 +321,18 @@ export const InnerVisualizationWrapper = ({ ] ); + const onData$ = useCallback( + (data: unknown, inspectorAdapters?: LensInspectorAdapters) => { + if (inspectorAdapters && inspectorAdapters.tables) { + dispatch({ + type: 'UPDATE_ACTIVE_DATA', + tables: inspectorAdapters.tables, + }); + } + }, + [dispatch] + ); + if (localState.expressionBuildError) { return ( @@ -342,6 +358,7 @@ export const InnerVisualizationWrapper = ({ searchContext={context} reload$={autoRefreshFetch$} onEvent={onEvent} + onData$={onData$} renderError={(errorMessage?: string | null) => { return ( diff --git a/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts b/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts index 7c10ee4a57fadc..7f9c97db5c189c 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts @@ -6,6 +6,7 @@ import { i18n } from '@kbn/i18n'; import { + ExecutionContext, ExpressionFunctionDefinition, ExpressionValueSearchContext, KibanaDatatable, @@ -14,6 +15,7 @@ import { search } from '../../../../../src/plugins/data/public'; const { toAbsoluteDates } = search.aggs; import { LensMultiTable } from '../types'; +import { LensInspectorAdapters } from './types'; interface MergeTables { layerIds: string[]; @@ -24,12 +26,14 @@ export const mergeTables: ExpressionFunctionDefinition< 'lens_merge_tables', ExpressionValueSearchContext | null, MergeTables, - LensMultiTable + LensMultiTable, + ExecutionContext > = { name: 'lens_merge_tables', type: 'lens_multitable', help: i18n.translate('xpack.lens.functions.mergeTables.help', { - defaultMessage: 'A helper to merge any number of kibana tables into a single table', + defaultMessage: + 'A helper to merge any number of kibana tables into a single table and expose it via inspector adapter', }), args: { layerIds: { @@ -44,10 +48,15 @@ export const mergeTables: ExpressionFunctionDefinition< }, }, inputTypes: ['kibana_context', 'null'], - fn(input, { layerIds, tables }) { + fn(input, { layerIds, tables }, context) { + if (!context.inspectorAdapters.tables) { + context.inspectorAdapters.tables = {}; + } const resultTables: Record = {}; tables.forEach((table, index) => { resultTables[layerIds[index]] = table; + // adapter is always defined at that point because we make sure by the beginning of the function + context.inspectorAdapters.tables![layerIds[index]] = table; }); return { type: 'lens_multitable', diff --git a/x-pack/plugins/lens/public/editor_frame_service/types.ts b/x-pack/plugins/lens/public/editor_frame_service/types.ts new file mode 100644 index 00000000000000..3d19ca9bc225ff --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_service/types.ts @@ -0,0 +1,12 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { KibanaDatatable } from 'src/plugins/expressions'; + +export type TableInspectorAdapter = Record; +export interface LensInspectorAdapters { + tables?: TableInspectorAdapter; +} diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 2b9ca5a2425f82..2c4e9eb8b800ad 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -171,13 +171,21 @@ export interface Datasource { toExpression: (state: T, layerId: string) => Ast | string | null; - getDatasourceSuggestionsForField: (state: T, field: unknown) => Array>; + getDatasourceSuggestionsForField: ( + state: T, + field: unknown, + activeData?: Record + ) => Array>; getDatasourceSuggestionsForVisualizeField: ( state: T, indexPatternId: string, - fieldName: string + fieldName: string, + activeData?: Record + ) => Array>; + getDatasourceSuggestionsFromCurrentState: ( + state: T, + activeData?: Record ) => Array>; - getDatasourceSuggestionsFromCurrentState: (state: T) => Array>; getPublicAPI: (props: PublicAPIProps) => DatasourcePublicAPI; } @@ -232,17 +240,20 @@ export type DatasourceDimensionEditorProps = DatasourceDimensionPro setState: StateSetter; core: Pick; dateRange: DateRange; + activeData?: Record; }; export type DatasourceDimensionTriggerProps = DatasourceDimensionProps & { dragDropContext: DragContextState; onClick: () => void; + activeData?: Record; }; export interface DatasourceLayerPanelProps { layerId: string; state: T; setState: StateSetter; + activeData?: Record; } export interface DraggedOperation { @@ -266,6 +277,7 @@ export type DatasourceDimensionDropProps = SharedDimensionProps & { columnId: string; state: T; setState: StateSetter; + activeData?: Record; dragDropContext: DragContextState; }; @@ -420,6 +432,12 @@ export interface VisualizationSuggestion { export interface FramePublicAPI { datasourceLayers: Record; + /** + * Data of the chart currently rendered in the preview. + * This data might be not available (e.g. if the chart can't be rendered) or outdated and belonging to another chart. + * If accessing, make sure to check whether expected columns actually exist. + */ + activeData?: Record; dateRange: DateRange; query: Query; diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index f9a0463ff58032..b2133a03fa5286 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -9467,7 +9467,6 @@ "xpack.lens.fittingFunctionsTitle.lookahead": "次へ", "xpack.lens.fittingFunctionsTitle.none": "非表示", "xpack.lens.fittingFunctionsTitle.zero": "ゼロ", - "xpack.lens.functions.mergeTables.help": "いくつかの Kibana 表を 1 つの表に結合するのをアシストします", "xpack.lens.functions.renameColumns.help": "データベースの列の名前の変更をアシストします", "xpack.lens.functions.renameColumns.idMap.help": "キーが古い列 ID で値が対応する新しい列 ID となるように JSON エンコーディングされたオブジェクトです。他の列 ID はすべてのそのままです。", "xpack.lens.includeValueButtonAriaLabel": "{value}を含める", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 6210f565ca3b48..1cf53fbdf51f71 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -9473,7 +9473,6 @@ "xpack.lens.fittingFunctionsTitle.lookahead": "下一", "xpack.lens.fittingFunctionsTitle.none": "隐藏", "xpack.lens.fittingFunctionsTitle.zero": "零", - "xpack.lens.functions.mergeTables.help": "将任何数目的 kibana 表合并成单个表的助手", "xpack.lens.functions.renameColumns.help": "用于重命名数据表列的助手", "xpack.lens.functions.renameColumns.idMap.help": "旧列 ID 为键且相应新列 ID 为值的 JSON 编码对象。所有其他列 ID 都将保留。", "xpack.lens.includeValueButtonAriaLabel": "包括 {value}", From 696f7915753b183e9125af2de159bbaae17fb8e5 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 4 Nov 2020 10:23:57 +0100 Subject: [PATCH 05/18] add active data to all relevant places --- .../editor_frame/config_panel/layer_panel.tsx | 3 +++ .../editor_frame/suggestion_helpers.ts | 6 +++++- .../editor_frame/suggestion_panel.tsx | 1 + .../workspace_panel/chart_switch.tsx | 1 + .../editor_frame_service/merge_tables.ts | 2 +- x-pack/plugins/lens/public/types.ts | 20 +++++++------------ 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index 82f753e3520b05..c9d99bcfb6d8d8 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -93,6 +93,7 @@ export function LayerPanel( state: props.visualizationState, frame: props.framePublicAPI, dateRange: props.framePublicAPI.dateRange, + activeData: props.framePublicAPI.activeData, }; const datasourceId = datasourcePublicAPI.datasourceId; const layerDatasourceState = props.datasourceStates[datasourceId].state; @@ -111,6 +112,7 @@ export function LayerPanel( ...layerDatasourceDropProps, frame: props.framePublicAPI, dateRange: props.framePublicAPI.dateRange, + activeData: props.framePublicAPI.activeData, }; const { groups } = activeVisualization.getConfiguration(layerVisualizationConfigProps); @@ -140,6 +142,7 @@ export function LayerPanel( nativeProps={{ layerId, state: layerDatasourceState, + activeData: props.framePublicAPI.activeData, setState: (updater: unknown) => { const newState = typeof updater === 'function' ? updater(layerDatasourceState) : updater; diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_helpers.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_helpers.ts index c4a92dde6187ce..888ce5f151f7a8 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_helpers.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_helpers.ts @@ -7,6 +7,7 @@ import _ from 'lodash'; import { Ast } from '@kbn/interpreter/common'; import { IconType } from '@elastic/eui/src/components/icon/icon'; +import { Datatable } from 'src/plugins/expressions'; import { VisualizeFieldContext } from '../../../../../../src/plugins/ui_actions/public'; import { Visualization, @@ -49,6 +50,7 @@ export function getSuggestions({ visualizationState, field, visualizeTriggerFieldContext, + activeData, }: { datasourceMap: Record; datasourceStates: Record< @@ -64,6 +66,7 @@ export function getSuggestions({ visualizationState: unknown; field?: unknown; visualizeTriggerFieldContext?: VisualizeFieldContext; + activeData?: Record; }): Suggestion[] { const datasources = Object.entries(datasourceMap).filter( ([datasourceId]) => datasourceStates[datasourceId] && !datasourceStates[datasourceId].isLoading @@ -84,7 +87,8 @@ export function getSuggestions({ dataSourceSuggestions = datasource.getDatasourceSuggestionsForField(datasourceState, field); } else { dataSourceSuggestions = datasource.getDatasourceSuggestionsFromCurrentState( - datasourceState + datasourceState, + activeData ); } return dataSourceSuggestions.map((suggestion) => ({ ...suggestion, datasourceId })); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx index 63ee02ac0404dd..2380612770030b 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx @@ -177,6 +177,7 @@ export function SuggestionPanel({ visualizationMap, activeVisualizationId: currentVisualizationId, visualizationState: currentVisualizationState, + activeData: frame.activeData, }) .map((suggestion) => ({ ...suggestion, diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx index 73ffbf56ff45a3..fd45e94e3741e7 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx @@ -311,6 +311,7 @@ function getTopSuggestion( activeVisualizationId: props.visualizationId, visualizationState: props.visualizationState, subVisualizationId, + activeData: props.framePublicAPI.activeData, }); const suggestions = unfilteredSuggestions.filter((suggestion) => { // don't use extended versions of current data table on switching between visualizations diff --git a/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts b/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts index a3176b5bbebd65..dad3087ca3d4bb 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts @@ -27,7 +27,7 @@ export const mergeTables: ExpressionFunctionDefinition< ExpressionValueSearchContext | null, MergeTables, LensMultiTable, - ExecutionContext + ExecutionContext > = { name: 'lens_merge_tables', type: 'lens_multitable', diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 9a63cae676b70b..e4e4df00887a2c 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -171,20 +171,15 @@ export interface Datasource { toExpression: (state: T, layerId: string) => Ast | string | null; - getDatasourceSuggestionsForField: ( - state: T, - field: unknown, - activeData?: Record - ) => Array>; + getDatasourceSuggestionsForField: (state: T, field: unknown) => Array>; getDatasourceSuggestionsForVisualizeField: ( state: T, indexPatternId: string, - fieldName: string, - activeData?: Record + fieldName: string ) => Array>; getDatasourceSuggestionsFromCurrentState: ( state: T, - activeData?: Record + activeData?: Record ) => Array>; getPublicAPI: (props: PublicAPIProps) => DatasourcePublicAPI; @@ -237,6 +232,7 @@ export type DatasourceDimensionProps = SharedDimensionProps & { columnId: string; onRemove?: (accessor: string) => void; state: T; + activeData?: Record; }; // The only way a visualization has to restrict the query building @@ -244,20 +240,18 @@ export type DatasourceDimensionEditorProps = DatasourceDimensionPro setState: StateSetter; core: Pick; dateRange: DateRange; - activeData?: Record; }; export type DatasourceDimensionTriggerProps = DatasourceDimensionProps & { dragDropContext: DragContextState; onClick: () => void; - activeData?: Record; }; export interface DatasourceLayerPanelProps { layerId: string; state: T; setState: StateSetter; - activeData?: Record; + activeData?: Record; } export interface DraggedOperation { @@ -281,7 +275,7 @@ export type DatasourceDimensionDropProps = SharedDimensionProps & { columnId: string; state: T; setState: StateSetter; - activeData?: Record; + activeData?: Record; // TODO wire up dragDropContext: DragContextState; isReorder?: boolean; }; @@ -442,7 +436,7 @@ export interface FramePublicAPI { * This data might be not available (e.g. if the chart can't be rendered) or outdated and belonging to another chart. * If accessing, make sure to check whether expected columns actually exist. */ - activeData?: Record; + activeData?: Record; // TODO wire up dateRange: DateRange; query: Query; From 1490d3174c10c3f3680ad4321d89f68e0b020cd0 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 4 Nov 2020 10:27:18 +0100 Subject: [PATCH 06/18] Fix bugs --- .../plugins/lens/public/editor_frame_service/merge_tables.ts | 3 +++ x-pack/plugins/lens/public/types.ts | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts b/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts index dad3087ca3d4bb..03ef7cf9cc6374 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/merge_tables.ts @@ -49,6 +49,9 @@ export const mergeTables: ExpressionFunctionDefinition< }, inputTypes: ['kibana_context', 'null'], fn(input, { layerIds, tables }, context) { + if (!context.inspectorAdapters) { + context.inspectorAdapters = {}; + } if (!context.inspectorAdapters.tables) { context.inspectorAdapters.tables = {}; } diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index e4e4df00887a2c..8b417d54cfdaff 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -436,7 +436,7 @@ export interface FramePublicAPI { * This data might be not available (e.g. if the chart can't be rendered) or outdated and belonging to another chart. * If accessing, make sure to check whether expected columns actually exist. */ - activeData?: Record; // TODO wire up + activeData?: Record; dateRange: DateRange; query: Query; From f3ca2652d81d0b58d0743946c3fb6d1418840db7 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 4 Nov 2020 11:47:57 +0100 Subject: [PATCH 07/18] fix types --- x-pack/plugins/lens/public/editor_frame_service/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/types.ts b/x-pack/plugins/lens/public/editor_frame_service/types.ts index 3d19ca9bc225ff..2da95ec2fd66f6 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/types.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/types.ts @@ -4,9 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { KibanaDatatable } from 'src/plugins/expressions'; +import { Datatable } from 'src/plugins/expressions'; -export type TableInspectorAdapter = Record; +export type TableInspectorAdapter = Record; export interface LensInspectorAdapters { tables?: TableInspectorAdapter; } From a78946a35c7eddb924b50428eb82a9974c4f9422 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 4 Nov 2020 12:00:49 +0100 Subject: [PATCH 08/18] add tests --- .../workspace_panel/workspace_panel.test.tsx | 42 ++++++++++++++ .../editor_frame_service/merge_tables.test.ts | 58 +++++++++++-------- 2 files changed, 76 insertions(+), 24 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx index 747a5b4f96d1d7..4feca96c69647b 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx @@ -253,6 +253,48 @@ describe('workspace_panel', () => { expect(trigger.exec).toHaveBeenCalledWith({ data: eventData }); }); + it('should push add current data table to state on data$ emitting value', () => { + const framePublicAPI = createMockFramePublicAPI(); + framePublicAPI.datasourceLayers = { + first: mockDatasource.publicAPIMock, + }; + mockDatasource.toExpression.mockReturnValue('datasource'); + mockDatasource.getLayers.mockReturnValue(['first']); + const dispatch = jest.fn(); + + instance = mount( + 'vis' }, + }} + visualizationState={{}} + dispatch={dispatch} + ExpressionRenderer={expressionRendererMock} + core={coreMock.createSetup()} + plugins={{ uiActions: uiActionsMock, data: dataMock }} + /> + ); + + const onData = expressionRendererMock.mock.calls[0][0].onData$!; + + const tableData = { table1: { columns: [], rows: [] } }; + onData(undefined, { tables: tableData }); + + expect(dispatch).toHaveBeenCalledWith({ type: 'UPDATE_ACTIVE_DATA', tables: tableData }); + }); + it('should include data fetching for each layer in the expression', () => { const mockDatasource2 = createMockDatasource('a'); const framePublicAPI = createMockFramePublicAPI(); diff --git a/x-pack/plugins/lens/public/editor_frame_service/merge_tables.test.ts b/x-pack/plugins/lens/public/editor_frame_service/merge_tables.test.ts index 5afabb9a52367a..07c16665d11b44 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/merge_tables.test.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/merge_tables.test.ts @@ -6,34 +6,35 @@ import moment from 'moment'; import { mergeTables } from './merge_tables'; -import { Datatable } from 'src/plugins/expressions'; +import { Datatable, ExecutionContext } from 'src/plugins/expressions'; +import { LensInspectorAdapters } from './types'; describe('lens_merge_tables', () => { - it('should produce a row with the nested table as defined', () => { - const sampleTable1: Datatable = { - type: 'datatable', - columns: [ - { id: 'bucket', name: 'A', meta: { type: 'string' } }, - { id: 'count', name: 'Count', meta: { type: 'number' } }, - ], - rows: [ - { bucket: 'a', count: 5 }, - { bucket: 'b', count: 10 }, - ], - }; + const sampleTable1: Datatable = { + type: 'datatable', + columns: [ + { id: 'bucket', name: 'A', meta: { type: 'string' } }, + { id: 'count', name: 'Count', meta: { type: 'number' } }, + ], + rows: [ + { bucket: 'a', count: 5 }, + { bucket: 'b', count: 10 }, + ], + }; - const sampleTable2: Datatable = { - type: 'datatable', - columns: [ - { id: 'bucket', name: 'C', meta: { type: 'string' } }, - { id: 'avg', name: 'Average', meta: { type: 'number' } }, - ], - rows: [ - { bucket: 'a', avg: 2.5 }, - { bucket: 'b', avg: 9 }, - ], - }; + const sampleTable2: Datatable = { + type: 'datatable', + columns: [ + { id: 'bucket', name: 'C', meta: { type: 'string' } }, + { id: 'avg', name: 'Average', meta: { type: 'number' } }, + ], + rows: [ + { bucket: 'a', avg: 2.5 }, + { bucket: 'b', avg: 9 }, + ], + }; + it('should produce a row with the nested table as defined', () => { expect( mergeTables.fn( null, @@ -47,6 +48,15 @@ describe('lens_merge_tables', () => { }); }); + it('should store the current tables in the tables inspector', () => { + const adapters: LensInspectorAdapters = { tables: {} }; + mergeTables.fn(null, { layerIds: ['first', 'second'], tables: [sampleTable1, sampleTable2] }, { + inspectorAdapters: adapters, + } as ExecutionContext); + expect(adapters.tables!.first).toBe(sampleTable1); + expect(adapters.tables!.second).toBe(sampleTable2); + }); + it('should pass the date range along', () => { expect( mergeTables.fn( From 8a695d188751b6c241aff4cb0fc52360d8cbf7e3 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 4 Nov 2020 12:16:26 +0100 Subject: [PATCH 09/18] remove leftover --- x-pack/plugins/lens/public/types.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 00a1c213926ec6..e4e85c5886aa31 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -276,7 +276,6 @@ export type DatasourceDimensionDropProps = SharedDimensionProps & { columnId: string; state: T; setState: StateSetter; - activeData?: Record; // TODO wire up dragDropContext: DragContextState; isReorder?: boolean; }; From 3820cfa8392179b755c9d129c0abb54868e81e61 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 4 Nov 2020 12:22:12 +0100 Subject: [PATCH 10/18] add test for expression renderer component --- .../public/react_expression_renderer.test.tsx | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/plugins/expressions/public/react_expression_renderer.test.tsx b/src/plugins/expressions/public/react_expression_renderer.test.tsx index 052c2a9f6a24a1..e52d4d153882ff 100644 --- a/src/plugins/expressions/public/react_expression_renderer.test.tsx +++ b/src/plugins/expressions/public/react_expression_renderer.test.tsx @@ -195,6 +195,38 @@ describe('ExpressionRenderer', () => { expect(instance.find('[data-test-subj="custom-error"]')).toHaveLength(0); }); + it('should call onData$ prop on every data$ observable emission in loader', () => { + const dataSubject = new Subject(); + const data$ = dataSubject.asObservable().pipe(share()); + + const newData = {}; + const inspectData = {}; + const onData$ = jest.fn(); + + (ExpressionLoader as jest.Mock).mockImplementation(() => { + return { + render$: new Subject(), + data$, + loading$: new Subject(), + events$: new Subject(), + update: jest.fn(), + inspect: jest.fn(() => inspectData), + }; + }); + + mount(); + + expect(onData$).toHaveBeenCalledTimes(0); + + act(() => { + dataSubject.next(newData); + }); + + expect(onData$).toHaveBeenCalledTimes(1); + expect(onData$.mock.calls[0][0]).toBe(newData); + expect(onData$.mock.calls[0][1]).toBe(inspectData); + }); + it('should fire onEvent prop on every events$ observable emission in loader', () => { const dataSubject = new Subject(); const data$ = dataSubject.asObservable().pipe(share()); From 9e4fb2e8eea9067b267cc859b28eb2a1734fe588 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 4 Nov 2020 14:57:09 +0100 Subject: [PATCH 11/18] add series and palette indicator --- .../config_panel/layer_panel.scss | 16 ++++++ .../editor_frame/config_panel/layer_panel.tsx | 20 ++++++++ .../pie_visualization/visualization.tsx | 13 ++++- x-pack/plugins/lens/public/types.ts | 14 +++-- .../lens/public/xy_visualization/index.ts | 6 ++- .../public/xy_visualization/visualization.tsx | 51 +++++++++++-------- 6 files changed, 91 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss index 2011fd32a3127d..79ce0f364eb672 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss @@ -52,6 +52,7 @@ align-items: center; overflow: hidden; min-height: $euiSizeXXL; + position: relative; // NativeRenderer is messing this up > div { @@ -119,4 +120,19 @@ .lnsLayerPanel__colorIndicator--solidColor { width: $euiSizeS; height: $euiSizeS; +} + +.lnsLayerPanel__paletteContainer { + position: absolute; + bottom: 0; + left: 0; + right: 0; +} + +.lnsLayerPanel__paletteColor { + height: $euiSizeXS; +} + +.lnsLayerPanel__dimensionLink { + width: 100%; } \ No newline at end of file diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index 014f5b38f5149f..a6d7af8c17c94f 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -396,6 +396,26 @@ export function LayerPanel( ); }} /> + {typeof accessorConfig !== 'string' && + accessorConfig.triggerIcon === 'colorBy' && + accessorConfig.palette && ( + + {accessorConfig.palette.map((color) => ( + + ))} + + )}
); diff --git a/x-pack/plugins/lens/public/pie_visualization/visualization.tsx b/x-pack/plugins/lens/public/pie_visualization/visualization.tsx index 791480162b7fac..bed9befd36deda 100644 --- a/x-pack/plugins/lens/public/pie_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/visualization.tsx @@ -9,7 +9,7 @@ import { render } from 'react-dom'; import { i18n } from '@kbn/i18n'; import { I18nProvider } from '@kbn/i18n/react'; import { PaletteRegistry } from 'src/plugins/charts/public'; -import { Visualization, OperationMetadata } from '../types'; +import { Visualization, OperationMetadata, AccessorConfig } from '../types'; import { toExpression, toPreviewExpression } from './to_expression'; import { LayerState, PieVisualizationState } from './types'; import { suggestions } from './suggestions'; @@ -113,7 +113,16 @@ export const getPieVisualization = ({ .map(({ columnId }) => columnId) .filter((columnId) => columnId !== layer.metric); // When we add a column it could be empty, and therefore have no order - const sortedColumns = Array.from(new Set(originalOrder.concat(layer.groups))); + const sortedColumns: AccessorConfig[] = Array.from(new Set(originalOrder.concat(layer.groups))); + if (sortedColumns.length > 0) { + sortedColumns[0] = { + columnId: sortedColumns[0] as string, + triggerIcon: 'colorBy', + palette: paletteService + .get(state.palette?.name || 'default') + .getColors(10, state.palette?.params), + }; + } if (state.shape === 'treemap') { return { diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 0dd48e24afabc5..2d6e2823233f39 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -343,15 +343,21 @@ export type VisualizationDimensionEditorProps = VisualizationConfig setState: (newState: T) => void; }; +export type AccessorConfig = + | string + | { + columnId: string; + triggerIcon?: 'color' | 'disabled' | 'colorBy' | 'none'; + color?: string; + palette?: string[]; + }; + export type VisualizationDimensionGroupConfig = SharedDimensionProps & { groupLabel: string; /** ID is passed back to visualization. For example, `x` */ groupId: string; - accessors: Array< - | string - | { columnId: string; triggerIcon?: 'color' | 'disabled' | 'colorBy' | 'none'; color?: string } - >; + accessors: AccessorConfig[]; supportsMoreColumns: boolean; /** If required, a warning will appear if accessors are empty */ required?: boolean; diff --git a/x-pack/plugins/lens/public/xy_visualization/index.ts b/x-pack/plugins/lens/public/xy_visualization/index.ts index 4891a51b3124b4..5e5eef2f01c177 100644 --- a/x-pack/plugins/lens/public/xy_visualization/index.ts +++ b/x-pack/plugins/lens/public/xy_visualization/index.ts @@ -10,6 +10,7 @@ import { ExpressionsSetup } from '../../../../../src/plugins/expressions/public' import { UI_SETTINGS } from '../../../../../src/plugins/data/public'; import { EditorFrameSetup, FormatFactory } from '../types'; import { ChartsPluginSetup } from '../../../../../src/plugins/charts/public'; +import { LensPluginStartDependencies } from '../plugin'; export interface XyVisualizationPluginSetupPlugins { expressions: ExpressionsSetup; @@ -31,7 +32,7 @@ export class XyVisualization { constructor() {} setup( - core: CoreSetup, + core: CoreSetup, { expressions, formatFactory, editorFrame, charts }: XyVisualizationPluginSetupPlugins ) { editorFrame.registerVisualization(async () => { @@ -46,6 +47,7 @@ export class XyVisualization { getXyChartRenderer, getXyVisualization, } = await import('../async_services'); + const [, { data }] = await core.getStartServices(); const palettes = await charts.palettes.getPalettes(); expressions.registerFunction(() => legendConfig); expressions.registerFunction(() => yAxisConfig); @@ -64,7 +66,7 @@ export class XyVisualization { histogramBarTarget: core.uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET), }) ); - return getXyVisualization({ paletteService: palettes }); + return getXyVisualization({ paletteService: palettes, data }); }); } } diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx index e0553cc055eb1c..0fe4a1b45b2c00 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx @@ -14,7 +14,7 @@ import { PaletteOutput, PaletteRegistry } from 'src/plugins/charts/public'; import { DataPublicPluginStart } from 'src/plugins/data/public'; import { getSuggestions } from './xy_suggestions'; import { LayerContextMenu, XyToolbar, DimensionEditor } from './xy_config_panel'; -import { Visualization, OperationMetadata, VisualizationType } from '../types'; +import { Visualization, OperationMetadata, VisualizationType, AccessorConfig } from '../types'; import { State, SeriesType, visualizationTypes, LayerConfig } from './types'; import { isHorizontalChart } from './state_helpers'; import { toExpression, toPreviewExpression, getSortedAccessors } from './to_expression'; @@ -173,9 +173,7 @@ export const getXyVisualization = ({ const datasource = frame.datasourceLayers[layer.layerId]; - let sortedAccessors: Array< - { columnId: string; triggerIcon: string; color: string | null } | string - > = getSortedAccessors(datasource, layer); + let sortedAccessors: AccessorConfig[] = getSortedAccessors(datasource, layer); if (frame.activeData) { const colorAssignments = getColorAssignments( @@ -189,27 +187,28 @@ export const getXyVisualization = ({ const currentYConfig = layer.yConfig?.find((yConfig) => yConfig.forAccessor === accessor); if (layerContainsSplits) { return { - columnId: accessor, + columnId: accessor as string, triggerIcon: 'disabled', }; } const rank = colorAssignments[currentPalette.name].getRank(layer, '', accessor as string); + const customColor = + currentYConfig?.color || + paletteService.get(currentPalette.name).getColor( + [ + { + name: '', + rankAtDepth: rank, + totalSeriesAtDepth: totalSeriesCount, + }, + ], + { maxDepth: 1, totalSeries: totalSeriesCount }, + currentPalette.params + ); return { - columnId: accessor, - triggerIcon: 'color', - color: - currentYConfig?.color || - paletteService.get(currentPalette.name).getColor( - [ - { - name: '', - rankAtDepth: rank, - totalSeriesAtDepth: totalSeriesCount, - }, - ], - { maxDepth: 1, totalSeries: totalSeriesCount }, - currentPalette.params - ), + columnId: accessor as string, + triggerIcon: customColor ? 'color' : 'disabled', + color: customColor ? customColor : undefined, }; }); } @@ -254,7 +253,17 @@ export const getXyVisualization = ({ groupLabel: i18n.translate('xpack.lens.xyChart.splitSeries', { defaultMessage: 'Break down by', }), - accessors: layer.splitAccessor ? [layer.splitAccessor] : [], + accessors: layer.splitAccessor + ? [ + { + columnId: layer.splitAccessor, + triggerIcon: 'colorBy', + palette: paletteService + .get(layer.palette?.name || 'default') + .getColors(10, layer.palette?.params), + }, + ] + : [], filterOperations: isBucketed, suggestedPriority: 0, supportsMoreColumns: !layer.splitAccessor, From ec31106f197cb4c6490c4d2cae25b9af3b94c7a8 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 4 Nov 2020 15:44:47 +0100 Subject: [PATCH 12/18] update docs --- ...n-plugins-expressions-public.expressionsservice.md | 4 ++-- ...s-expressions-public.expressionsservice.migrate.md | 2 +- ...sions-public.expressionsservice.migratetolatest.md | 2 +- ...gins-expressions-public.reactexpressionrenderer.md | 2 +- ...expressions-public.reactexpressionrendererprops.md | 1 + ...ons-public.reactexpressionrendererprops.ondata_.md | 11 +++++++++++ src/plugins/expressions/public/public.api.md | 4 +++- 7 files changed, 20 insertions(+), 6 deletions(-) create mode 100644 docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.ondata_.md diff --git a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.md b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.md index 307fc73ec6e9c1..8311a71414abcb 100644 --- a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.md +++ b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.md @@ -39,8 +39,8 @@ export declare class ExpressionsService implements PersistableStateExpressionsServiceStart['getType'] | | | [getTypes](./kibana-plugin-plugins-expressions-public.expressionsservice.gettypes.md) | | () => ReturnType<Executor['getTypes']> | Returns POJO map of all registered expression types, where keys are names of the types and values are ExpressionType instances. | | [inject](./kibana-plugin-plugins-expressions-public.expressionsservice.inject.md) | | (state: ExpressionAstExpression, references: SavedObjectReference[]) => ExpressionAstExpression | Injects saved object references into expression AST | -| [migrate](./kibana-plugin-plugins-expressions-public.expressionsservice.migrate.md) | | (state: SerializableState, version: string) => ExpressionAstExpression | Injects saved object references into expression AST | -| [migrateToLatest](./kibana-plugin-plugins-expressions-public.expressionsservice.migratetolatest.md) | | (state: unknown, version: string) => ExpressionAstExpression | Injects saved object references into expression AST | +| [migrate](./kibana-plugin-plugins-expressions-public.expressionsservice.migrate.md) | | (state: SerializableState, version: string) => ExpressionAstExpression | Runs the migration (if it exists) for specified version. This will run a single migration step (ie from 7.10.0 to 7.10.1) | +| [migrateToLatest](./kibana-plugin-plugins-expressions-public.expressionsservice.migratetolatest.md) | | (state: unknown, version: string) => ExpressionAstExpression | Migrates expression to the latest version | | [registerFunction](./kibana-plugin-plugins-expressions-public.expressionsservice.registerfunction.md) | | (functionDefinition: AnyExpressionFunctionDefinition | (() => AnyExpressionFunctionDefinition)) => void | Register an expression function, which will be possible to execute as part of the expression pipeline.Below we register a function which simply sleeps for given number of milliseconds to delay the execution and outputs its input as-is. ```ts expressions.registerFunction({ diff --git a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.migrate.md b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.migrate.md index 88a6bda4ee3f58..d1f24bfcfc0bbb 100644 --- a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.migrate.md +++ b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.migrate.md @@ -4,7 +4,7 @@ ## ExpressionsService.migrate property -Injects saved object references into expression AST +Runs the migration (if it exists) for specified version. This will run a single migration step (ie from 7.10.0 to 7.10.1) Signature: diff --git a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.migratetolatest.md b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.migratetolatest.md index e6860df19fd3f6..3b8f008546f70f 100644 --- a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.migratetolatest.md +++ b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionsservice.migratetolatest.md @@ -4,7 +4,7 @@ ## ExpressionsService.migrateToLatest property -Injects saved object references into expression AST +Migrates expression to the latest version Signature: diff --git a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrenderer.md b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrenderer.md index 32a7151578658c..8cc32ff698b386 100644 --- a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrenderer.md +++ b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrenderer.md @@ -7,5 +7,5 @@ Signature: ```typescript -ReactExpressionRenderer: ({ className, dataAttrs, padding, renderError, expression, onEvent, reload$, debounce, ...expressionLoaderOptions }: ReactExpressionRendererProps) => JSX.Element +ReactExpressionRenderer: ({ className, dataAttrs, padding, renderError, expression, onEvent, onData$, reload$, debounce, ...expressionLoaderOptions }: ReactExpressionRendererProps) => JSX.Element ``` diff --git a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.md b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.md index e4980ce04b9e27..92ea071b23dfce 100644 --- a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.md +++ b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.md @@ -18,6 +18,7 @@ export interface ReactExpressionRendererProps extends IExpressionLoaderParams | [dataAttrs](./kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.dataattrs.md) | string[] | | | [debounce](./kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.debounce.md) | number | | | [expression](./kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.expression.md) | string | ExpressionAstExpression | | +| [onData$](./kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.ondata_.md) | <TData, TInspectorAdapters>(data: TData, adapters?: TInspectorAdapters) => void | | | [onEvent](./kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.onevent.md) | (event: ExpressionRendererEvent) => void | | | [padding](./kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.padding.md) | 'xs' | 's' | 'm' | 'l' | 'xl' | | | [reload$](./kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.reload_.md) | Observable<unknown> | An observable which can be used to re-run the expression without destroying the component | diff --git a/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.ondata_.md b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.ondata_.md new file mode 100644 index 00000000000000..05ddb0b13a5bee --- /dev/null +++ b/docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.ondata_.md @@ -0,0 +1,11 @@ + + +[Home](./index.md) > [kibana-plugin-plugins-expressions-public](./kibana-plugin-plugins-expressions-public.md) > [ReactExpressionRendererProps](./kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.md) > [onData$](./kibana-plugin-plugins-expressions-public.reactexpressionrendererprops.ondata_.md) + +## ReactExpressionRendererProps.onData$ property + +Signature: + +```typescript +onData$?: (data: TData, adapters?: TInspectorAdapters) => void; +``` diff --git a/src/plugins/expressions/public/public.api.md b/src/plugins/expressions/public/public.api.md index 454c3030aa0727..dc991fbd7117d7 100644 --- a/src/plugins/expressions/public/public.api.md +++ b/src/plugins/expressions/public/public.api.md @@ -1059,7 +1059,7 @@ export interface Range { // Warning: (ae-missing-release-tag) "ReactExpressionRenderer" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) -export const ReactExpressionRenderer: ({ className, dataAttrs, padding, renderError, expression, onEvent, reload$, debounce, ...expressionLoaderOptions }: ReactExpressionRendererProps) => JSX.Element; +export const ReactExpressionRenderer: ({ className, dataAttrs, padding, renderError, expression, onEvent, onData$, reload$, debounce, ...expressionLoaderOptions }: ReactExpressionRendererProps) => JSX.Element; // Warning: (ae-missing-release-tag) "ReactExpressionRendererProps" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // @@ -1074,6 +1074,8 @@ export interface ReactExpressionRendererProps extends IExpressionLoaderParams { // (undocumented) expression: string | ExpressionAstExpression; // (undocumented) + onData$?: (data: TData, adapters?: TInspectorAdapters) => void; + // (undocumented) onEvent?: (event: ExpressionRendererEvent) => void; // (undocumented) padding?: 'xs' | 's' | 'm' | 'l' | 'xl'; From b58abd44fb2a3b3c56dce7d77454ff48e9118e9a Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 4 Nov 2020 17:52:22 +0100 Subject: [PATCH 13/18] put stubs in place for things to test --- .../charts/public/services/palettes/mock.ts | 25 +++- .../xy_visualization/color_assignment.test.ts | 4 + .../xy_visualization/color_assignment.ts | 32 ++--- .../xy_visualization/to_expression.test.ts | 2 + .../xy_visualization/visualization.test.ts | 111 +++++++++++++++++- .../public/xy_visualization/visualization.tsx | 2 + .../xy_visualization/xy_suggestions.test.ts | 2 + 7 files changed, 159 insertions(+), 19 deletions(-) diff --git a/src/plugins/charts/public/services/palettes/mock.ts b/src/plugins/charts/public/services/palettes/mock.ts index a7ec3cc16ce6f5..7a1e94d097b90e 100644 --- a/src/plugins/charts/public/services/palettes/mock.ts +++ b/src/plugins/charts/public/services/palettes/mock.ts @@ -22,7 +22,7 @@ import { PaletteService } from './service'; import { PaletteDefinition, SeriesLayer } from './types'; export const getPaletteRegistry = () => { - const mockPalette: jest.Mocked = { + const mockPalette1: jest.Mocked = { id: 'default', title: 'My Palette', getColor: jest.fn((_: SeriesLayer[]) => 'black'), @@ -41,9 +41,28 @@ export const getPaletteRegistry = () => { })), }; + const mockPalette2: jest.Mocked = { + id: 'mocked', + title: 'Mocked Palette', + getColor: jest.fn((_: SeriesLayer[]) => 'blue'), + getColors: jest.fn((num: number) => ['blue', 'yellow']), + toExpression: jest.fn(() => ({ + type: 'expression', + chain: [ + { + type: 'function', + function: 'system_palette', + arguments: { + name: ['mocked'], + }, + }, + ], + })), + }; + return { - get: (_: string) => mockPalette, - getAll: () => [mockPalette], + get: (name: string) => (name === 'default' ? mockPalette1 : mockPalette2), + getAll: () => [mockPalette1, mockPalette2], }; }; diff --git a/x-pack/plugins/lens/public/xy_visualization/color_assignment.test.ts b/x-pack/plugins/lens/public/xy_visualization/color_assignment.test.ts index b59e09e8c19768..5a24dae429a0f7 100644 --- a/x-pack/plugins/lens/public/xy_visualization/color_assignment.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/color_assignment.test.ts @@ -178,5 +178,9 @@ describe('color_assignment', () => { // 3 series in front of (complex object)/y1 - abc/y1, abc/y2 expect(assignments.palette1.getRank(layers[0], 'formatted', 'y1')).toEqual(2); }); + + it('should handle missing tables and columns', () => { + throw new Error('not implemented'); + }); }); }); diff --git a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts index 1f517fff21f3b6..0d6274b1af9abe 100644 --- a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts +++ b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts @@ -39,18 +39,21 @@ export function getColorAssignments( return { numberOfSeries: layer.accessors.length, splits: [] }; } const splitAccessor = layer.splitAccessor; - const column = data.tables[layer.layerId].columns.find(({ id }) => id === splitAccessor)!; - const splits = uniq( - data.tables[layer.layerId].rows.map((row) => { - let value = row[splitAccessor]; - if (value && !isPrimitive(value)) { - value = formatFactory(column.meta.params).convert(value); - } else { - value = String(value); - } - return value; - }) - ); + const column = data.tables[layer.layerId].columns.find(({ id }) => id === splitAccessor); + const splits = + !column || !data.tables[layer.layerId] + ? [] + : uniq( + data.tables[layer.layerId].rows.map((row) => { + let value = row[splitAccessor]; + if (value && !isPrimitive(value)) { + value = formatFactory(column.meta.params).convert(value); + } else { + value = String(value); + } + return value; + }) + ); return { numberOfSeries: (splits.length || 1) * layer.accessors.length, splits }; }); const totalSeriesCount = seriesPerLayer.reduce( @@ -62,15 +65,14 @@ export function getColorAssignments( getRank(layer: LayerColorConfig, seriesKey: string, yAccessor: string) { const layerIndex = paletteLayers.indexOf(layer); const currentSeriesPerLayer = seriesPerLayer[layerIndex]; + const splitRank = currentSeriesPerLayer.splits.indexOf(seriesKey); return ( (layerIndex === 0 ? 0 : seriesPerLayer .slice(0, layerIndex) .reduce((sum, perLayer) => sum + perLayer.numberOfSeries, 0)) + - (layer.splitAccessor - ? currentSeriesPerLayer.splits.indexOf(seriesKey) * layer.accessors.length - : 0) + + (layer.splitAccessor && splitRank !== -1 ? splitRank * layer.accessors.length : 0) + layer.accessors.indexOf(yAccessor) ); }, diff --git a/x-pack/plugins/lens/public/xy_visualization/to_expression.test.ts b/x-pack/plugins/lens/public/xy_visualization/to_expression.test.ts index 6148824bfec216..2a853d40373f2a 100644 --- a/x-pack/plugins/lens/public/xy_visualization/to_expression.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/to_expression.test.ts @@ -10,10 +10,12 @@ import { chartPluginMock } from '../../../../../src/plugins/charts/public/mocks' import { getXyVisualization } from './xy_visualization'; import { Operation } from '../types'; import { createMockDatasource, createMockFramePublicAPI } from '../editor_frame_service/mocks'; +import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks'; describe('#toExpression', () => { const xyVisualization = getXyVisualization({ paletteService: chartPluginMock.createPaletteRegistry(), + data: dataPluginMock.createStartContract(), }); let mockDatasource: ReturnType; let frame: ReturnType; diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts index 4dde646ab64a5d..61ff77e31781c9 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts @@ -11,6 +11,7 @@ import { State, SeriesType } from './types'; import { createMockDatasource, createMockFramePublicAPI } from '../editor_frame_service/mocks'; import { LensIconChartBar } from '../assets/chart_bar'; import { chartPluginMock } from '../../../../../src/plugins/charts/public/mocks'; +import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks'; function exampleState(): State { return { @@ -27,9 +28,12 @@ function exampleState(): State { ], }; } +const paletteServiceMock = chartPluginMock.createPaletteRegistry(); +const dataMock = dataPluginMock.createStartContract(); const xyVisualization = getXyVisualization({ - paletteService: chartPluginMock.createPaletteRegistry(), + paletteService: paletteServiceMock, + data: dataMock, }); describe('xy_visualization', () => { @@ -305,6 +309,14 @@ describe('xy_visualization', () => { frame.datasourceLayers = { first: mockDatasource.publicAPIMock, }; + + frame.activeData = { + first: { + type: 'datatable', + rows: [], + columns: [], + }, + }; }); it('should return options for 3 dimensions', () => { @@ -406,5 +418,102 @@ describe('xy_visualization', () => { ]; expect(ops.filter(filterOperations).map((x) => x.dataType)).toEqual(['number']); }); + + describe('color assignment', () => { + it('should pass custom y color in accessor config', () => { + const baseState = exampleState(); + const options = xyVisualization.getConfiguration({ + state: { + ...baseState, + layers: [ + { + ...baseState.layers[0], + splitAccessor: undefined, + yConfig: [ + { + forAccessor: 'b', + color: 'red', + }, + ], + }, + ], + }, + frame, + layerId: 'first', + }).groups; + const accessorConfig = options + .find(({ groupId }) => groupId === 'y') + ?.accessors.find((accessor) => typeof accessor !== 'string' && accessor.columnId === 'b'); + if (typeof accessorConfig === 'string') { + throw new Error('could not find accessor'); + } + expect(accessorConfig?.triggerIcon).toEqual('color'); + expect(accessorConfig?.color).toEqual('red'); + }); + + it('should query palette to fill in colors for other dimensions', () => { + const palette = paletteServiceMock.get('default'); + (palette.getColor as jest.Mock).mockClear(); + const baseState = exampleState(); + const options = xyVisualization.getConfiguration({ + state: { + ...baseState, + layers: [ + { + ...baseState.layers[0], + splitAccessor: undefined, + }, + ], + }, + frame, + layerId: 'first', + }).groups; + const accessorConfig = options + .find(({ groupId }) => groupId === 'y') + ?.accessors.find((accessor) => typeof accessor !== 'string' && accessor.columnId === 'c'); + if (typeof accessorConfig === 'string') { + throw new Error('could not find accessor'); + } + expect(accessorConfig?.triggerIcon).toEqual('color'); + // black is the color returned from the palette mock + expect(accessorConfig?.color).toEqual('black'); + expect(palette.getColor).toHaveBeenCalledWith( + [ + { + name: '', + // rank 1 because it's the second y metric + rankAtDepth: 1, + totalSeriesAtDepth: 2, + }, + ], + { maxDepth: 1, totalSeries: 2 }, + undefined + ); + }); + + it('should pass name of current series along', () => { + throw new Error('not implemented'); + }); + + it('should use custom palette if layer contains palette', () => { + throw new Error('not implemented'); + }); + + it('should not show any indicator as long as there is no data', () => { + throw new Error('not implemented'); + }); + + it('should show disable icon for splitted series', () => { + throw new Error('not implemented'); + }); + + it('should show current palette for break down by dimension', () => { + throw new Error('not implemented'); + }); + + it('should show current custom palette for break down by dimension', () => { + throw new Error('not implemented'); + }); + }); }); }); diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx index 0fe4a1b45b2c00..4df03e7b3136ec 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx @@ -191,12 +191,14 @@ export const getXyVisualization = ({ triggerIcon: 'disabled', }; } + // TODO this should be the actual series name (same logic as the chart) const rank = colorAssignments[currentPalette.name].getRank(layer, '', accessor as string); const customColor = currentYConfig?.color || paletteService.get(currentPalette.name).getColor( [ { + // TODO this should be the actual series name (same logic as the chart) name: '', rankAtDepth: rank, totalSeriesAtDepth: totalSeriesCount, diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_suggestions.test.ts b/x-pack/plugins/lens/public/xy_visualization/xy_suggestions.test.ts index ac0039df966d4e..7bca40d07128a5 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_suggestions.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/xy_suggestions.test.ts @@ -15,12 +15,14 @@ import { State, XYState, visualizationTypes } from './types'; import { generateId } from '../id_generator'; import { getXyVisualization } from './xy_visualization'; import { chartPluginMock } from '../../../../../src/plugins/charts/public/mocks'; +import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks'; import { PaletteOutput } from 'src/plugins/charts/public'; jest.mock('../id_generator'); const xyVisualization = getXyVisualization({ paletteService: chartPluginMock.createPaletteRegistry(), + data: dataPluginMock.createStartContract(), }); describe('xy_suggestions', () => { From 9c80ec286bf3ae55a8edd60ee42385d818d3cdfb Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 5 Nov 2020 10:40:50 +0100 Subject: [PATCH 14/18] fix tests and stuff --- .../charts/public/services/palettes/mock.ts | 2 +- .../xy_visualization/color_assignment.test.ts | 50 ++++++- .../xy_visualization/color_assignment.ts | 2 +- .../xy_visualization/expression.test.tsx | 4 +- .../public/xy_visualization/state_helpers.ts | 13 ++ .../public/xy_visualization/to_expression.ts | 13 +- .../xy_visualization/visualization.test.ts | 134 ++++++++++++------ .../public/xy_visualization/visualization.tsx | 20 +-- 8 files changed, 167 insertions(+), 71 deletions(-) diff --git a/src/plugins/charts/public/services/palettes/mock.ts b/src/plugins/charts/public/services/palettes/mock.ts index 7a1e94d097b90e..2e45d93999b8e4 100644 --- a/src/plugins/charts/public/services/palettes/mock.ts +++ b/src/plugins/charts/public/services/palettes/mock.ts @@ -61,7 +61,7 @@ export const getPaletteRegistry = () => { }; return { - get: (name: string) => (name === 'default' ? mockPalette1 : mockPalette2), + get: (name: string) => (name !== 'default' ? mockPalette2 : mockPalette1), getAll: () => [mockPalette1, mockPalette2], }; }; diff --git a/x-pack/plugins/lens/public/xy_visualization/color_assignment.test.ts b/x-pack/plugins/lens/public/xy_visualization/color_assignment.test.ts index 5a24dae429a0f7..666b0d50982185 100644 --- a/x-pack/plugins/lens/public/xy_visualization/color_assignment.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/color_assignment.test.ts @@ -128,6 +128,31 @@ describe('color_assignment', () => { expect(assignments.palette2.totalSeriesCount).toEqual(2 * 3); expect(formatMock).toHaveBeenCalledWith(complexObject); }); + + it('should handle missing tables', () => { + const assignments = getColorAssignments(layers, { ...data, tables: {} }, formatFactory); + // if there is no data, just assume a single split + expect(assignments.palette1.totalSeriesCount).toEqual(2); + }); + + it('should handle missing columns', () => { + const assignments = getColorAssignments( + layers, + { + ...data, + tables: { + ...data.tables, + '1': { + ...data.tables['1'], + columns: [], + }, + }, + }, + formatFactory + ); + // if the split column is missing, just assume a single split + expect(assignments.palette1.totalSeriesCount).toEqual(2); + }); }); describe('getRank', () => { @@ -179,8 +204,29 @@ describe('color_assignment', () => { expect(assignments.palette1.getRank(layers[0], 'formatted', 'y1')).toEqual(2); }); - it('should handle missing tables and columns', () => { - throw new Error('not implemented'); + it('should handle missing tables', () => { + const assignments = getColorAssignments(layers, { ...data, tables: {} }, formatFactory); + // if there is no data, assume it is the first splitted series. One series in front - 0/y1 + expect(assignments.palette1.getRank(layers[0], '2', 'y2')).toEqual(1); + }); + + it('should handle missing columns', () => { + const assignments = getColorAssignments( + layers, + { + ...data, + tables: { + ...data.tables, + '1': { + ...data.tables['1'], + columns: [], + }, + }, + }, + formatFactory + ); + // if the split column is missing, assume it is the first splitted series. One series in front - 0/y1 + expect(assignments.palette1.getRank(layers[0], '2', 'y2')).toEqual(1); }); }); }); diff --git a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts index 0d6274b1af9abe..d0c5575b505f65 100644 --- a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts +++ b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts @@ -39,7 +39,7 @@ export function getColorAssignments( return { numberOfSeries: layer.accessors.length, splits: [] }; } const splitAccessor = layer.splitAccessor; - const column = data.tables[layer.layerId].columns.find(({ id }) => id === splitAccessor); + const column = data.tables[layer.layerId]?.columns.find(({ id }) => id === splitAccessor); const splits = !column || !data.tables[layer.layerId] ? [] diff --git a/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx b/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx index 6c9669dc239ea6..7aafb51a78f741 100644 --- a/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx @@ -1385,13 +1385,13 @@ describe('xy_expression', () => { yAccessor: 'a', seriesKeys: ['a'], }) - ).toEqual('black'); + ).toEqual('blue'); expect( (component.find(LineSeries).at(1).prop('color') as Function)!({ yAccessor: 'c', seriesKeys: ['c'], }) - ).toEqual('black'); + ).toEqual('blue'); }); }); diff --git a/x-pack/plugins/lens/public/xy_visualization/state_helpers.ts b/x-pack/plugins/lens/public/xy_visualization/state_helpers.ts index 41d18e5199e4c2..f0c05a8c5d1ffb 100644 --- a/x-pack/plugins/lens/public/xy_visualization/state_helpers.ts +++ b/x-pack/plugins/lens/public/xy_visualization/state_helpers.ts @@ -5,6 +5,7 @@ */ import { EuiIconType } from '@elastic/eui/src/components/icon/icon'; +import { DatasourcePublicAPI } from '../types'; import { SeriesType, visualizationTypes, LayerConfig, YConfig } from './types'; export function isHorizontalSeries(seriesType: SeriesType) { @@ -37,3 +38,15 @@ export const getSeriesColor = (layer: LayerConfig, accessor: string) => { layer?.yConfig?.find((yConfig: YConfig) => yConfig.forAccessor === accessor)?.color || null ); }; + +export const getColumnToLabelMap = (layer: LayerConfig, datasource: DatasourcePublicAPI) => { + const columnToLabel: Record = {}; + + layer.accessors.concat(layer.splitAccessor ? [layer.splitAccessor] : []).forEach((accessor) => { + const operation = datasource.getOperationForColumnId(accessor); + if (operation?.label) { + columnToLabel[accessor] = operation.label; + } + }); + return columnToLabel; +}; diff --git a/x-pack/plugins/lens/public/xy_visualization/to_expression.ts b/x-pack/plugins/lens/public/xy_visualization/to_expression.ts index 904d1541a85ec6..c75fee2343e896 100644 --- a/x-pack/plugins/lens/public/xy_visualization/to_expression.ts +++ b/x-pack/plugins/lens/public/xy_visualization/to_expression.ts @@ -9,6 +9,7 @@ import { ScaleType } from '@elastic/charts'; import { PaletteRegistry } from 'src/plugins/charts/public'; import { State, LayerConfig } from './types'; import { OperationMetadata, DatasourcePublicAPI } from '../types'; +import { getColumnToLabelMap } from './state_helpers'; interface ValidLayer extends LayerConfig { xAccessor: NonNullable; @@ -198,17 +199,7 @@ export const buildExpression = ( }, ], layers: validLayers.map((layer) => { - const columnToLabel: Record = {}; - - const datasource = datasourceLayers[layer.layerId]; - layer.accessors - .concat(layer.splitAccessor ? [layer.splitAccessor] : []) - .forEach((accessor) => { - const operation = datasource.getOperationForColumnId(accessor); - if (operation?.label) { - columnToLabel[accessor] = operation.label; - } - }); + const columnToLabel = getColumnToLabelMap(layer, datasourceLayers[layer.layerId]); const xAxisOperation = datasourceLayers && diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts index 12ef29c2c3a997..0b35cfec40521d 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts @@ -7,7 +7,7 @@ import { getXyVisualization } from './visualization'; import { Position } from '@elastic/charts'; import { Operation } from '../types'; -import { State, SeriesType } from './types'; +import { State, SeriesType, LayerConfig } from './types'; import { createMockDatasource, createMockFramePublicAPI } from '../editor_frame_service/mocks'; import { LensIconChartBar } from '../assets/chart_bar'; import { chartPluginMock } from '../../../../../src/plugins/charts/public/mocks'; @@ -420,7 +420,7 @@ describe('xy_visualization', () => { }); describe('color assignment', () => { - it('should pass custom y color in accessor config', () => { + function callConfig(layerConfigOverride: Partial) { const baseState = exampleState(); const options = xyVisualization.getConfiguration({ state: { @@ -429,58 +429,64 @@ describe('xy_visualization', () => { { ...baseState.layers[0], splitAccessor: undefined, - yConfig: [ - { - forAccessor: 'b', - color: 'red', - }, - ], + ...layerConfigOverride, }, ], }, frame, layerId: 'first', }).groups; - const accessorConfig = options - .find(({ groupId }) => groupId === 'y') - ?.accessors.find((accessor) => typeof accessor !== 'string' && accessor.columnId === 'b'); - if (typeof accessorConfig === 'string') { + return options; + } + + function callConfigForYConfigs(layerConfigOverride: Partial) { + return callConfig(layerConfigOverride).find(({ groupId }) => groupId === 'y'); + } + + function callConfigForBreakdownConfigs(layerConfigOverride: Partial) { + return callConfig(layerConfigOverride).find(({ groupId }) => groupId === 'breakdown'); + } + + function callConfigAndFindYConfig( + layerConfigOverride: Partial, + assertionAccessor: string + ) { + const accessorConfig = callConfigForYConfigs(layerConfigOverride)?.accessors.find( + (accessor) => typeof accessor !== 'string' && accessor.columnId === assertionAccessor + ); + if (!accessorConfig || typeof accessorConfig === 'string') { throw new Error('could not find accessor'); } - expect(accessorConfig?.triggerIcon).toEqual('color'); - expect(accessorConfig?.color).toEqual('red'); - }); + return accessorConfig; + } - it('should query palette to fill in colors for other dimensions', () => { - const palette = paletteServiceMock.get('default'); - (palette.getColor as jest.Mock).mockClear(); - const baseState = exampleState(); - const options = xyVisualization.getConfiguration({ - state: { - ...baseState, - layers: [ + it('should pass custom y color in accessor config', () => { + const accessorConfig = callConfigAndFindYConfig( + { + yConfig: [ { - ...baseState.layers[0], - splitAccessor: undefined, + forAccessor: 'b', + color: 'red', }, ], }, - frame, - layerId: 'first', - }).groups; - const accessorConfig = options - .find(({ groupId }) => groupId === 'y') - ?.accessors.find((accessor) => typeof accessor !== 'string' && accessor.columnId === 'c'); - if (typeof accessorConfig === 'string') { - throw new Error('could not find accessor'); - } - expect(accessorConfig?.triggerIcon).toEqual('color'); + 'b' + ); + expect(accessorConfig.triggerIcon).toEqual('color'); + expect(accessorConfig.color).toEqual('red'); + }); + + it('should query palette to fill in colors for other dimensions', () => { + const palette = paletteServiceMock.get('default'); + (palette.getColor as jest.Mock).mockClear(); + const accessorConfig = callConfigAndFindYConfig({}, 'c'); + expect(accessorConfig.triggerIcon).toEqual('color'); // black is the color returned from the palette mock - expect(accessorConfig?.color).toEqual('black'); + expect(accessorConfig.color).toEqual('black'); expect(palette.getColor).toHaveBeenCalledWith( [ { - name: '', + name: 'c', // rank 1 because it's the second y metric rankAtDepth: 1, totalSeriesAtDepth: 2, @@ -492,27 +498,63 @@ describe('xy_visualization', () => { }); it('should pass name of current series along', () => { - throw new Error('not implemented'); + (frame.datasourceLayers.first.getOperationForColumnId as jest.Mock).mockReturnValue({ + label: 'Overwritten label', + }); + const palette = paletteServiceMock.get('default'); + (palette.getColor as jest.Mock).mockClear(); + callConfigAndFindYConfig({}, 'c'); + expect(palette.getColor).toHaveBeenCalledWith( + [ + expect.objectContaining({ + name: 'Overwritten label', + }), + ], + expect.anything(), + undefined + ); }); it('should use custom palette if layer contains palette', () => { - throw new Error('not implemented'); + const palette = paletteServiceMock.get('mock'); + callConfigAndFindYConfig( + { + palette: { type: 'palette', name: 'mock', params: {} }, + }, + 'c' + ); + expect(palette.getColor).toHaveBeenCalled(); }); it('should not show any indicator as long as there is no data', () => { - throw new Error('not implemented'); + frame.activeData = undefined; + const yConfigs = callConfigForYConfigs({}); + expect(yConfigs!.accessors.length).toEqual(2); + yConfigs!.accessors.forEach((accessor) => { + expect(typeof accessor).toBe('string'); + }); }); it('should show disable icon for splitted series', () => { - throw new Error('not implemented'); + const accessorConfig = callConfigAndFindYConfig( + { + splitAccessor: 'd', + }, + 'b' + ); + expect(accessorConfig.triggerIcon).toEqual('disabled'); }); it('should show current palette for break down by dimension', () => { - throw new Error('not implemented'); - }); - - it('should show current custom palette for break down by dimension', () => { - throw new Error('not implemented'); + const palette = paletteServiceMock.get('mock'); + const customColors = ['yellow', 'green']; + (palette.getColors as jest.Mock).mockReturnValue(customColors); + const breakdownConfig = callConfigForBreakdownConfigs({ + palette: { type: 'palette', name: 'mock', params: {} }, + splitAccessor: 'd', + }); + const accessorConfig = breakdownConfig!.accessors[0]; + expect(typeof accessorConfig !== 'string' && accessorConfig.palette).toEqual(customColors); }); }); }); diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx index 937b9e708989cd..8ef79e601af150 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx @@ -16,7 +16,7 @@ import { getSuggestions } from './xy_suggestions'; import { LayerContextMenu, XyToolbar, DimensionEditor } from './xy_config_panel'; import { Visualization, OperationMetadata, VisualizationType, AccessorConfig } from '../types'; import { State, SeriesType, visualizationTypes, LayerConfig } from './types'; -import { isHorizontalChart } from './state_helpers'; +import { getColumnToLabelMap, isHorizontalChart } from './state_helpers'; import { toExpression, toPreviewExpression, getSortedAccessors } from './to_expression'; import { LensIconChartBarStacked } from '../assets/chart_bar_stacked'; import { LensIconChartMixedXy } from '../assets/chart_mixed_xy'; @@ -173,7 +173,8 @@ export const getXyVisualization = ({ const datasource = frame.datasourceLayers[layer.layerId]; - let sortedAccessors: AccessorConfig[] = getSortedAccessors(datasource, layer); + const sortedAccessors: string[] = getSortedAccessors(datasource, layer); + let mappedAccessors: AccessorConfig[] = sortedAccessors; if (frame.activeData) { const colorAssignments = getColorAssignments( @@ -183,7 +184,7 @@ export const getXyVisualization = ({ ); const currentPalette: PaletteOutput = layer.palette || { type: 'palette', name: 'default' }; const totalSeriesCount = colorAssignments[currentPalette.name].totalSeriesCount; - sortedAccessors = sortedAccessors.map((accessor) => { + mappedAccessors = sortedAccessors.map((accessor) => { const currentYConfig = layer.yConfig?.find((yConfig) => yConfig.forAccessor === accessor); if (layerContainsSplits) { return { @@ -191,15 +192,18 @@ export const getXyVisualization = ({ triggerIcon: 'disabled', }; } - // TODO this should be the actual series name (same logic as the chart) - const rank = colorAssignments[currentPalette.name].getRank(layer, '', accessor as string); + const columnToLabel = getColumnToLabelMap(layer, frame.datasourceLayers[layer.layerId]); + const rank = colorAssignments[currentPalette.name].getRank( + layer, + columnToLabel[accessor] || accessor, + accessor + ); const customColor = currentYConfig?.color || paletteService.get(currentPalette.name).getColor( [ { - // TODO this should be the actual series name (same logic as the chart) - name: '', + name: columnToLabel[accessor] || accessor, rankAtDepth: rank, totalSeriesAtDepth: totalSeriesCount, }, @@ -230,7 +234,7 @@ export const getXyVisualization = ({ { groupId: 'y', groupLabel: getAxisName('y', { isHorizontal }), - accessors: sortedAccessors, + accessors: mappedAccessors, filterOperations: isNumericMetric, supportsMoreColumns: true, required: true, From 8ed06f89146e8b4f807bceca6a74daf75798694c Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 11 Nov 2020 18:29:46 +0100 Subject: [PATCH 15/18] review comments --- .../visualization.test.tsx | 2 +- .../datatable_visualization/visualization.tsx | 12 +- .../config_panel/color_indicator.tsx | 50 +++++++++ .../config_panel/layer_panel.scss | 5 + .../config_panel/layer_panel.test.tsx | 18 +-- .../editor_frame/config_panel/layer_panel.tsx | 79 +++---------- .../config_panel/palette_indicator.tsx | 32 ++++++ .../metric_visualization/visualization.tsx | 2 +- .../pie_visualization/visualization.tsx | 10 +- x-pack/plugins/lens/public/types.ts | 14 +-- .../xy_visualization/color_assignment.ts | 10 +- .../xy_visualization/visualization.test.ts | 2 +- .../public/xy_visualization/visualization.tsx | 105 +++++++++++------- 13 files changed, 203 insertions(+), 138 deletions(-) create mode 100644 x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx create mode 100644 x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/palette_indicator.tsx diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx index d82c7b092c38af..0af8e01d7290d1 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx @@ -284,7 +284,7 @@ describe('Datatable Visualization', () => { state: { layers: [layer] }, frame, }).groups[1].accessors - ).toEqual(['c', 'b']); + ).toEqual([{ columnId: 'c' }, { columnId: 'b' }]); }); }); diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx index e0f6ae31719caa..8b5d2d7d733484 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx @@ -149,9 +149,9 @@ export const datatableVisualization: Visualization defaultMessage: 'Break down by', }), layerId: state.layers[0].layerId, - accessors: sortedColumns.filter( - (c) => datasource!.getOperationForColumnId(c)?.isBucketed - ), + accessors: sortedColumns + .filter((c) => datasource!.getOperationForColumnId(c)?.isBucketed) + .map((accessor) => ({ columnId: accessor })), supportsMoreColumns: true, filterOperations: (op) => op.isBucketed, dataTestSubj: 'lnsDatatable_column', @@ -162,9 +162,9 @@ export const datatableVisualization: Visualization defaultMessage: 'Metrics', }), layerId: state.layers[0].layerId, - accessors: sortedColumns.filter( - (c) => !datasource!.getOperationForColumnId(c)?.isBucketed - ), + accessors: sortedColumns + .filter((c) => !datasource!.getOperationForColumnId(c)?.isBucketed) + .map((accessor) => ({ columnId: accessor })), supportsMoreColumns: true, filterOperations: (op) => !op.isBucketed, required: true, diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx new file mode 100644 index 00000000000000..c0b077e378139b --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx @@ -0,0 +1,50 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { EuiFlexGroup, EuiFlexItem, EuiIcon } from '@elastic/eui'; +import { AccessorConfig } from '../../../types'; + +export function ColorIndicator({ + accessorConfig, + children, +}: { + accessorConfig: AccessorConfig; + children: React.ReactChild; +}) { + return ( + + {typeof accessorConfig !== 'string' && + accessorConfig.triggerIcon === 'color' && + accessorConfig.color && ( + +
+ + )} + {typeof accessorConfig !== 'string' && accessorConfig.triggerIcon === 'disabled' && ( + + + + )} + {typeof accessorConfig !== 'string' && accessorConfig.triggerIcon === 'colorBy' && ( + + + + )} + {children} + + ); +} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss index 79ce0f364eb672..3176dc48597f12 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss @@ -135,4 +135,9 @@ .lnsLayerPanel__dimensionLink { width: 100%; + + &:focus { + background-color: transparent !important; // sass-lint:disable-line no-important + outline: none !important; // sass-lint:disable-line no-important + } } \ No newline at end of file diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx index c0cd211a49dd9a..2e0f50515d2753 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx @@ -137,7 +137,7 @@ describe('LayerPanel', () => { { groupLabel: 'A', groupId: 'a', - accessors: ['x'], + accessors: [{ columnId: 'x' }], filterOperations: () => true, supportsMoreColumns: false, dataTestSubj: 'lnsGroup', @@ -177,7 +177,7 @@ describe('LayerPanel', () => { { groupLabel: 'A', groupId: 'a', - accessors: ['x'], + accessors: [{ columnId: 'x' }], filterOperations: () => true, supportsMoreColumns: false, dataTestSubj: 'lnsGroup', @@ -209,7 +209,7 @@ describe('LayerPanel', () => { { groupLabel: 'A', groupId: 'a', - accessors: ['newid'], + accessors: [{ columnId: 'newid' }], filterOperations: () => true, supportsMoreColumns: true, dataTestSubj: 'lnsGroup', @@ -257,7 +257,7 @@ describe('LayerPanel', () => { { groupLabel: 'A', groupId: 'a', - accessors: ['newid'], + accessors: [{ columnId: 'newid' }], filterOperations: () => true, supportsMoreColumns: false, dataTestSubj: 'lnsGroup', @@ -302,7 +302,7 @@ describe('LayerPanel', () => { { groupLabel: 'A', groupId: 'a', - accessors: ['newid'], + accessors: [{ columnId: 'newid' }], filterOperations: () => true, supportsMoreColumns: false, dataTestSubj: 'lnsGroup', @@ -377,7 +377,7 @@ describe('LayerPanel', () => { { groupLabel: 'A', groupId: 'a', - accessors: ['a'], + accessors: [{ columnId: 'a' }], filterOperations: () => true, supportsMoreColumns: true, dataTestSubj: 'lnsGroup', @@ -416,7 +416,7 @@ describe('LayerPanel', () => { { groupLabel: 'A', groupId: 'a', - accessors: ['a'], + accessors: [{ columnId: 'a' }], filterOperations: () => true, supportsMoreColumns: false, dataTestSubj: 'lnsGroupA', @@ -424,7 +424,7 @@ describe('LayerPanel', () => { { groupLabel: 'B', groupId: 'b', - accessors: ['b'], + accessors: [{ columnId: 'b' }], filterOperations: () => true, supportsMoreColumns: true, dataTestSubj: 'lnsGroupB', @@ -480,7 +480,7 @@ describe('LayerPanel', () => { { groupLabel: 'A', groupId: 'a', - accessors: ['a', 'b'], + accessors: [{ columnId: 'a' }, { columnId: 'b' }], filterOperations: () => true, supportsMoreColumns: true, dataTestSubj: 'lnsGroup', diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index b4d69429e79b51..8d23be64526d62 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -14,7 +14,6 @@ import { EuiFlexItem, EuiButtonEmpty, EuiFormRow, - EuiIcon, EuiLink, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; @@ -27,6 +26,8 @@ import { trackUiEvent } from '../../../lens_ui_telemetry'; import { generateId } from '../../../id_generator'; import { ConfigPanelWrapperProps, ActiveDimensionState } from './types'; import { DimensionContainer } from './dimension_container'; +import { ColorIndicator } from './color_indicator'; +import { PaletteIndicator } from './palette_indicator'; const initialActiveDimensionState = { isNew: false, @@ -324,51 +325,16 @@ export function LayerPanel( } }} > - - {typeof accessorConfig !== 'string' && - accessorConfig.triggerIcon === 'color' && - accessorConfig.color && ( - -
- - )} - {typeof accessorConfig !== 'string' && - accessorConfig.triggerIcon === 'disabled' && ( - - - - )} - {typeof accessorConfig !== 'string' && - accessorConfig.triggerIcon === 'colorBy' && ( - - - - )} - - - - + + + - {typeof accessorConfig !== 'string' && - accessorConfig.triggerIcon === 'colorBy' && - accessorConfig.palette && ( - - {accessorConfig.palette.map((color) => ( - - ))} - - )} +
); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/palette_indicator.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/palette_indicator.tsx new file mode 100644 index 00000000000000..c615b295a5cfc0 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/palette_indicator.tsx @@ -0,0 +1,32 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { AccessorConfig } from '../../../types'; + +export function PaletteIndicator({ accessorConfig }: { accessorConfig: AccessorConfig }) { + if ( + typeof accessorConfig === 'string' || + accessorConfig.triggerIcon !== 'colorBy' || + !accessorConfig.palette + ) + return null; + return ( + + {accessorConfig.palette.map((color) => ( + + ))} + + ); +} diff --git a/x-pack/plugins/lens/public/metric_visualization/visualization.tsx b/x-pack/plugins/lens/public/metric_visualization/visualization.tsx index b75ac89d7e4d86..d8c475734e67e0 100644 --- a/x-pack/plugins/lens/public/metric_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/metric_visualization/visualization.tsx @@ -96,7 +96,7 @@ export const metricVisualization: Visualization = { groupId: 'metric', groupLabel: i18n.translate('xpack.lens.metric.label', { defaultMessage: 'Metric' }), layerId: props.state.layerId, - accessors: props.state.accessor ? [props.state.accessor] : [], + accessors: props.state.accessor ? [{ columnId: props.state.accessor }] : [], supportsMoreColumns: !props.state.accessor, filterOperations: (op: OperationMetadata) => !op.isBucketed && op.dataType === 'number', }, diff --git a/x-pack/plugins/lens/public/pie_visualization/visualization.tsx b/x-pack/plugins/lens/public/pie_visualization/visualization.tsx index 9b7db28e69afb0..91f0ddb54ad418 100644 --- a/x-pack/plugins/lens/public/pie_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/visualization.tsx @@ -113,10 +113,12 @@ export const getPieVisualization = ({ .map(({ columnId }) => columnId) .filter((columnId) => columnId !== layer.metric); // When we add a column it could be empty, and therefore have no order - const sortedColumns: AccessorConfig[] = Array.from(new Set(originalOrder.concat(layer.groups))); + const sortedColumns: AccessorConfig[] = Array.from( + new Set(originalOrder.concat(layer.groups)) + ).map((accessor) => ({ columnId: accessor })); if (sortedColumns.length > 0) { sortedColumns[0] = { - columnId: sortedColumns[0] as string, + columnId: sortedColumns[0].columnId, triggerIcon: 'colorBy', palette: paletteService .get(state.palette?.name || 'default') @@ -146,7 +148,7 @@ export const getPieVisualization = ({ defaultMessage: 'Size by', }), layerId, - accessors: layer.metric ? [layer.metric] : [], + accessors: layer.metric ? [{ columnId: layer.metric }] : [], supportsMoreColumns: !layer.metric, filterOperations: numberMetricOperations, required: true, @@ -177,7 +179,7 @@ export const getPieVisualization = ({ defaultMessage: 'Size by', }), layerId, - accessors: layer.metric ? [layer.metric] : [], + accessors: layer.metric ? [{ columnId: layer.metric }] : [], supportsMoreColumns: !layer.metric, filterOperations: numberMetricOperations, required: true, diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 46af50c0028065..225fedb987c76a 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -340,14 +340,12 @@ export type VisualizationDimensionEditorProps = VisualizationConfig setState: (newState: T) => void; }; -export type AccessorConfig = - | string - | { - columnId: string; - triggerIcon?: 'color' | 'disabled' | 'colorBy' | 'none'; - color?: string; - palette?: string[]; - }; +export interface AccessorConfig { + columnId: string; + triggerIcon?: 'color' | 'disabled' | 'colorBy' | 'none'; + color?: string; + palette?: string[]; +} export type VisualizationDimensionGroupConfig = SharedDimensionProps & { groupLabel: string; diff --git a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts index d0c5575b505f65..68c47e11acfc0a 100644 --- a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts +++ b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts @@ -18,11 +18,19 @@ interface LayerColorConfig { layerId: string; } +export type ColorAssignments = Record< + string, + { + totalSeriesCount: number; + getRank(layer: LayerColorConfig, seriesKey: string, yAccessor: string): number; + } +>; + export function getColorAssignments( layers: LayerColorConfig[], data: { tables: Record }, formatFactory: FormatFactory -) { +): ColorAssignments { const layersPerPalette: Record = {}; layers.forEach((layer) => { diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts index e6d1ffb7f5e89c..546cf06d4014e4 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts @@ -533,7 +533,7 @@ describe('xy_visualization', () => { const yConfigs = callConfigForYConfigs({}); expect(yConfigs!.accessors.length).toEqual(2); yConfigs!.accessors.forEach((accessor) => { - expect(typeof accessor).toBe('string'); + expect(accessor.triggerIcon).toBeUndefined(); }); }); diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx index f089cb7caa2425..5748e649c181e2 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx @@ -14,14 +14,20 @@ import { PaletteOutput, PaletteRegistry } from 'src/plugins/charts/public'; import { DataPublicPluginStart } from 'src/plugins/data/public'; import { getSuggestions } from './xy_suggestions'; import { LayerContextMenu, XyToolbar, DimensionEditor } from './xy_config_panel'; -import { Visualization, OperationMetadata, VisualizationType, AccessorConfig } from '../types'; +import { + Visualization, + OperationMetadata, + VisualizationType, + AccessorConfig, + FramePublicAPI, +} from '../types'; import { State, SeriesType, visualizationTypes, LayerConfig } from './types'; import { getColumnToLabelMap, isHorizontalChart } from './state_helpers'; import { toExpression, toPreviewExpression, getSortedAccessors } from './to_expression'; import { LensIconChartBarStacked } from '../assets/chart_bar_stacked'; import { LensIconChartMixedXy } from '../assets/chart_mixed_xy'; import { LensIconChartBarHorizontal } from '../assets/chart_bar_horizontal'; -import { getColorAssignments } from './color_assignment'; +import { ColorAssignments, getColorAssignments } from './color_assignment'; const defaultIcon = LensIconChartBarStacked; const defaultSeriesType = 'bar_stacked'; @@ -170,12 +176,12 @@ export const getXyVisualization = ({ return { groups: [] }; } - const layerContainsSplits = Boolean(layer.splitAccessor); - const datasource = frame.datasourceLayers[layer.layerId]; const sortedAccessors: string[] = getSortedAccessors(datasource, layer); - let mappedAccessors: AccessorConfig[] = sortedAccessors; + let mappedAccessors: AccessorConfig[] = sortedAccessors.map((accessor) => ({ + columnId: accessor, + })); if (frame.activeData) { const colorAssignments = getColorAssignments( @@ -183,41 +189,13 @@ export const getXyVisualization = ({ { tables: frame.activeData }, data.fieldFormats.deserialize ); - const currentPalette: PaletteOutput = layer.palette || { type: 'palette', name: 'default' }; - const totalSeriesCount = colorAssignments[currentPalette.name].totalSeriesCount; - mappedAccessors = sortedAccessors.map((accessor) => { - const currentYConfig = layer.yConfig?.find((yConfig) => yConfig.forAccessor === accessor); - if (layerContainsSplits) { - return { - columnId: accessor as string, - triggerIcon: 'disabled', - }; - } - const columnToLabel = getColumnToLabelMap(layer, frame.datasourceLayers[layer.layerId]); - const rank = colorAssignments[currentPalette.name].getRank( - layer, - columnToLabel[accessor] || accessor, - accessor - ); - const customColor = - currentYConfig?.color || - paletteService.get(currentPalette.name).getColor( - [ - { - name: columnToLabel[accessor] || accessor, - rankAtDepth: rank, - totalSeriesAtDepth: totalSeriesCount, - }, - ], - { maxDepth: 1, totalSeries: totalSeriesCount }, - currentPalette.params - ); - return { - columnId: accessor as string, - triggerIcon: customColor ? 'color' : 'disabled', - color: customColor ? customColor : undefined, - }; - }); + mappedAccessors = getAccessorColorConfig( + colorAssignments, + frame, + layer, + sortedAccessors, + paletteService + ); } const isHorizontal = isHorizontalChart(state.layers); @@ -226,7 +204,7 @@ export const getXyVisualization = ({ { groupId: 'x', groupLabel: getAxisName('x', { isHorizontal }), - accessors: layer.xAccessor ? [layer.xAccessor] : [], + accessors: layer.xAccessor ? [{ columnId: layer.xAccessor }] : [], filterOperations: isBucketed, supportsMoreColumns: !layer.xAccessor, dataTestSubj: 'lnsXY_xDimensionPanel', @@ -393,6 +371,51 @@ export const getXyVisualization = ({ }, }); +function getAccessorColorConfig( + colorAssignments: ColorAssignments, + frame: FramePublicAPI, + layer: LayerConfig, + sortedAccessors: string[], + paletteService: PaletteRegistry +): AccessorConfig[] { + const layerContainsSplits = Boolean(layer.splitAccessor); + const currentPalette: PaletteOutput = layer.palette || { type: 'palette', name: 'default' }; + const totalSeriesCount = colorAssignments[currentPalette.name].totalSeriesCount; + return sortedAccessors.map((accessor) => { + const currentYConfig = layer.yConfig?.find((yConfig) => yConfig.forAccessor === accessor); + if (layerContainsSplits) { + return { + columnId: accessor as string, + triggerIcon: 'disabled', + }; + } + const columnToLabel = getColumnToLabelMap(layer, frame.datasourceLayers[layer.layerId]); + const rank = colorAssignments[currentPalette.name].getRank( + layer, + columnToLabel[accessor] || accessor, + accessor + ); + const customColor = + currentYConfig?.color || + paletteService.get(currentPalette.name).getColor( + [ + { + name: columnToLabel[accessor] || accessor, + rankAtDepth: rank, + totalSeriesAtDepth: totalSeriesCount, + }, + ], + { maxDepth: 1, totalSeries: totalSeriesCount }, + currentPalette.params + ); + return { + columnId: accessor as string, + triggerIcon: customColor ? 'color' : 'disabled', + color: customColor ? customColor : undefined, + }; + }); +} + function validateLayersForDimension( dimension: string, layers: LayerConfig[], From 188ed0870d19cdac594d32d54a68b3cc2e3004e1 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 11 Nov 2020 18:32:40 +0100 Subject: [PATCH 16/18] simplify --- .../config_panel/color_indicator.tsx | 26 +++++++++---------- .../config_panel/palette_indicator.tsx | 7 +---- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx index c0b077e378139b..3f91a6b8913bfa 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx @@ -17,19 +17,17 @@ export function ColorIndicator({ }) { return ( - {typeof accessorConfig !== 'string' && - accessorConfig.triggerIcon === 'color' && - accessorConfig.color && ( - -
- - )} - {typeof accessorConfig !== 'string' && accessorConfig.triggerIcon === 'disabled' && ( + {accessorConfig.triggerIcon === 'color' && accessorConfig.color && ( + +
+ + )} + {accessorConfig.triggerIcon === 'disabled' && ( )} - {typeof accessorConfig !== 'string' && accessorConfig.triggerIcon === 'colorBy' && ( + {accessorConfig.triggerIcon === 'colorBy' && ( diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/palette_indicator.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/palette_indicator.tsx index c615b295a5cfc0..7e65fe7025932c 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/palette_indicator.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/palette_indicator.tsx @@ -9,12 +9,7 @@ import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import { AccessorConfig } from '../../../types'; export function PaletteIndicator({ accessorConfig }: { accessorConfig: AccessorConfig }) { - if ( - typeof accessorConfig === 'string' || - accessorConfig.triggerIcon !== 'colorBy' || - !accessorConfig.palette - ) - return null; + if (accessorConfig.triggerIcon !== 'colorBy' || !accessorConfig.palette) return null; return ( {accessorConfig.palette.map((color) => ( From 82580bf9e455cee12a3c414c9400e0c0f80ab92f Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 16 Nov 2020 10:53:44 +0100 Subject: [PATCH 17/18] review fixes --- .../lens/public/drag_drop/drag_drop.scss | 2 +- .../config_panel/color_indicator.tsx | 67 +++++++++++++------ .../config_panel/layer_panel.scss | 21 +++--- .../editor_frame/config_panel/layer_panel.tsx | 13 ++-- .../dimension_panel/dimension_panel.tsx | 14 +--- 5 files changed, 66 insertions(+), 51 deletions(-) diff --git a/x-pack/plugins/lens/public/drag_drop/drag_drop.scss b/x-pack/plugins/lens/public/drag_drop/drag_drop.scss index 8766c9f0acabf6..ded0b4552a4e55 100644 --- a/x-pack/plugins/lens/public/drag_drop/drag_drop.scss +++ b/x-pack/plugins/lens/public/drag_drop/drag_drop.scss @@ -47,7 +47,7 @@ // Drop area will be replacing existing content .lnsDragDrop-isReplacing { &, - .lnsLayerPanel__triggerLink { + .lnsLayerPanel__triggerText { text-decoration: line-through; } } diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx index 3f91a6b8913bfa..5ee1139ff09a2d 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/color_indicator.tsx @@ -6,6 +6,7 @@ import React from 'react'; import { EuiFlexGroup, EuiFlexItem, EuiIcon } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; import { AccessorConfig } from '../../../types'; export function ColorIndicator({ @@ -15,33 +16,55 @@ export function ColorIndicator({ accessorConfig: AccessorConfig; children: React.ReactChild; }) { - return ( - - {accessorConfig.triggerIcon === 'color' && accessorConfig.color && ( - -
+ {accessorConfig.triggerIcon === 'color' && accessorConfig.color && ( + - - )} - {accessorConfig.triggerIcon === 'disabled' && ( - + )} + {accessorConfig.triggerIcon === 'disabled' && ( - - )} - {accessorConfig.triggerIcon === 'colorBy' && ( - - - - )} + )} + {accessorConfig.triggerIcon === 'colorBy' && ( + + )} + + ); + } + + return ( + + {indicatorIcon} {children} ); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss index 3176dc48597f12..da62255b5caf95 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss @@ -81,28 +81,18 @@ margin-right: $euiSizeS; } -.lnsLayerPanel__triggerLink { +.lnsLayerPanel__triggerText { width: 100%; padding: $euiSizeS; min-height: $euiSizeXXL - 2; word-break: break-word; - - &:focus { - background-color: transparent !important; // sass-lint:disable-line no-important - outline: none !important; // sass-lint:disable-line no-important - } - - &:focus .lnsLayerPanel__triggerLinkLabel, - &:focus-within .lnsLayerPanel__triggerLinkLabel { - background-color: transparentize($euiColorVis1, .9); - } } -.lnsLayerPanel__triggerLinkLabel { +.lnsLayerPanel__triggerTextLabel { transition: background-color $euiAnimSpeedFast ease-in-out; } -.lnsLayerPanel__triggerLinkContent { +.lnsLayerPanel__triggerTextContent { // Make EUI button content not centered justify-content: flex-start; padding: 0 !important; // sass-lint:disable-line no-important @@ -140,4 +130,9 @@ background-color: transparent !important; // sass-lint:disable-line no-important outline: none !important; // sass-lint:disable-line no-important } + + &:focus .lnsLayerPanel__triggerTextLabel, + &:focus-within .lnsLayerPanel__triggerTextLabel { + background-color: transparentize($euiColorVis1, .9); + } } \ No newline at end of file diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index 8d23be64526d62..f5b31fb8811672 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -184,6 +184,10 @@ export function LayerPanel( const newId = generateId(); const isMissing = !isEmptyLayer && group.required && group.accessors.length === 0; + const triggerLinkA11yText = i18n.translate('xpack.lens.configure.editConfig', { + defaultMessage: 'Click to edit configuration or drag to move', + }); + return ( {group.accessors.map((accessorConfig) => { - const accessor = - typeof accessorConfig === 'string' ? accessorConfig : accessorConfig.columnId; + const accessor = accessorConfig.columnId; const { dragging } = dragDropContext; const dragType = isDraggedOperation(dragging) && accessor === dragging.columnId @@ -324,6 +327,8 @@ export function LayerPanel( }); } }} + aria-label={triggerLinkA11yText} + title={triggerLinkA11yText} >
{ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx index 4168267c7192e3..94018bd84b5174 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx @@ -66,10 +66,6 @@ export const IndexPatternDimensionTriggerComponent = function IndexPatternDimens } const formattedLabel = wrapOnDot(uniqueLabel); - const triggerLinkA11yText = i18n.translate('xpack.lens.configure.editConfig', { - defaultMessage: 'Click to edit configuration or drag to move', - }); - if (currentFieldIsInvalid) { return ( @@ -110,14 +104,12 @@ export const IndexPatternDimensionTriggerComponent = function IndexPatternDimens - {formattedLabel} + {formattedLabel} From 4e0a21f8028878391c074b16d521a7fc3c9857ff Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 16 Nov 2020 10:54:55 +0100 Subject: [PATCH 18/18] remove unused css class --- .../editor_frame/config_panel/layer_panel.scss | 5 ----- 1 file changed, 5 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss index da62255b5caf95..a1a072be77f810 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss @@ -107,11 +107,6 @@ margin-left: $euiSizeS; } -.lnsLayerPanel__colorIndicator--solidColor { - width: $euiSizeS; - height: $euiSizeS; -} - .lnsLayerPanel__paletteContainer { position: absolute; bottom: 0;