From c9699f2dfac7e706b16faf7e1556598d5c88587f Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Fri, 26 Jul 2019 11:03:35 -0400 Subject: [PATCH 01/16] Add global chart switcher --- .../plugins/lens/public/app_plugin/plugin.tsx | 4 +- .../visualization.tsx | 20 +++ .../editor_frame/config_panel_header.tsx | 104 +++++++++++ .../editor_frame/config_panel_wrapper.tsx | 80 +++++---- .../editor_frame/editor_frame.test.tsx | 61 +++++-- .../editor_frame/editor_frame.tsx | 3 +- .../lens/public/editor_frame_plugin/mocks.tsx | 10 ++ .../public/editor_frame_plugin/plugin.tsx | 4 +- x-pack/legacy/plugins/lens/public/types.ts | 16 +- .../public/xy_visualization_plugin/types.ts | 55 ++++++ .../xy_config_panel.test.tsx | 20 ++- .../xy_config_panel.tsx | 163 ++++++++---------- .../xy_visualization_plugin/xy_expression.tsx | 5 +- .../xy_visualization_plugin/xy_suggestions.ts | 7 +- .../xy_visualization.test.ts | 2 + .../xy_visualization.tsx | 62 ++++++- 16 files changed, 453 insertions(+), 163 deletions(-) create mode 100644 x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx diff --git a/x-pack/legacy/plugins/lens/public/app_plugin/plugin.tsx b/x-pack/legacy/plugins/lens/public/app_plugin/plugin.tsx index a559c0a94465b..12cc6f3fb0f47 100644 --- a/x-pack/legacy/plugins/lens/public/app_plugin/plugin.tsx +++ b/x-pack/legacy/plugins/lens/public/app_plugin/plugin.tsx @@ -29,8 +29,8 @@ export class AppPlugin { const editorFrame = editorFrameSetup(); editorFrame.registerDatasource('indexpattern', indexPattern); - editorFrame.registerVisualization('xy', xyVisualization); - editorFrame.registerVisualization('datatable', datatableVisualization); + editorFrame.registerVisualization(xyVisualization); + editorFrame.registerVisualization(datatableVisualization); this.instance = editorFrame.createInstance({}); diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx index 279aa4e7baeac..ae3d28caaed4d 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx @@ -89,6 +89,26 @@ export const datatableVisualization: Visualization< DatatableVisualizationState, DatatableVisualizationState > = { + id: 'lnsDatatable', + + visualizationTypes: [ + { + id: 'lnsDatatable', + icon: 'visTable', + label: i18n.translate('xpack.lens.datatable.label', { + defaultMessage: 'Datatable', + }), + }, + ], + + renderDescription(el) { + render(

Datatable

, el); + }, + + switchVisualizationType(_: string, state: DatatableVisualizationState) { + return state; + }, + initialize(frame, state) { const layerId = Object.keys(frame.datasourceLayers)[0] || frame.addNewLayer(); return ( diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx new file mode 100644 index 0000000000000..8cfe3ac52f5cc --- /dev/null +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx @@ -0,0 +1,104 @@ +/* + * 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, { useState } from 'react'; +import { + EuiIcon, + EuiPopover, + EuiButton, + EuiPopoverTitle, + EuiKeyPadMenu, + EuiKeyPadMenuItemButton, +} from '@elastic/eui'; +import { flatten } from 'lodash'; +import { i18n } from '@kbn/i18n'; +import { NativeRenderer } from '../../native_renderer'; +import { Visualization } from '../../types'; + +interface Props { + visualizations: Visualization[]; + visualizationId: string | null; + visualizationState: unknown; + onChange: (visualizationId: string, subVisualizationId: string) => void; +} + +function VisualizationSummary(props: Props) { + const visualization = props.visualizations.find(v => v.id === props.visualizationId); + + if (!visualization) { + return ( + <> + {i18n.translate('xpack.lens.configPanel.chooseVisualization', { + defaultMessage: 'Choose a visualization', + })} + + ); + } + + return ( + + ); +} + +export function ConfigPanelHeader(props: Props) { + const [state, setState] = useState({ + isFlyoutOpen: false, + }); + const close = () => setState({ isFlyoutOpen: false }); + const visualizationTypes = flatten( + props.visualizations.map(v => + v.visualizationTypes.map(t => ({ + visualizationId: v.id, + ...t, + })) + ) + ); + + return ( + setState({ isFlyoutOpen: !state.isFlyoutOpen })} + data-test-subj="lnsConfigPanelHeaderPopover" + > + + + } + isOpen={state.isFlyoutOpen} + closePopover={close} + anchorPosition="leftUp" + > + + {i18n.translate('xpack.lens.configPanel.chooseVisualization', { + defaultMessage: 'Choose a visualization', + })} + + + {visualizationTypes.map(v => ( + {v.label}} + role="menuitem" + data-test-subj={`lnsConfigPanelHeaderPopover_${v.id}`} + onClick={() => { + props.onChange(v.visualizationId, v.id); + close(); + }} + > + + + ))} + + + ); +} diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx index 2bca6c670c770..77229e439fac1 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx @@ -5,11 +5,11 @@ */ import React, { useMemo, useContext, memo } from 'react'; -import { EuiSelect } from '@elastic/eui'; import { NativeRenderer } from '../../native_renderer'; import { Action } from './state_management'; -import { Visualization, FramePublicAPI, VisualizationSuggestion } from '../../types'; +import { Visualization, FramePublicAPI } from '../../types'; import { DragContext } from '../../drag_drop'; +import { ConfigPanelHeader } from './config_panel_header'; interface ConfigPanelWrapperProps { visualizationState: unknown; @@ -20,33 +20,26 @@ interface ConfigPanelWrapperProps { } function getSuggestedVisualizationState(frame: FramePublicAPI, visualization: Visualization) { - const datasources = Object.entries(frame.datasourceLayers); - - let results: VisualizationSuggestion[] = []; - - datasources.forEach(([layerId, datasource]) => { - const suggestions = visualization.getSuggestions({ - tables: [ - { - datasourceSuggestionId: 0, - isMultiRow: true, - columns: datasource.getTableSpec().map(col => ({ - ...col, - operation: datasource.getOperationForColumnId(col.columnId)!, - })), - layerId, - }, - ], - }); - - results = results.concat(suggestions); + const [[layerId, datasource]] = Object.entries(frame.datasourceLayers); + const suggestions = visualization.getSuggestions({ + tables: [ + { + datasourceSuggestionId: 0, + isMultiRow: true, + columns: datasource.getTableSpec().map(col => ({ + ...col, + operation: datasource.getOperationForColumnId(col.columnId)!, + })), + layerId, + }, + ], }); - if (!results.length) { + if (!suggestions.length) { return visualization.initialize(frame); } - return visualization.initialize(frame, results[0].state); + return visualization.initialize(frame, suggestions[0].state); } export const ConfigPanelWrapper = memo(function ConfigPanelWrapper(props: ConfigPanelWrapperProps) { @@ -63,23 +56,28 @@ export const ConfigPanelWrapper = memo(function ConfigPanelWrapper(props: Config return ( <> - ({ - value: visualizationId, - text: visualizationId, - }))} - value={props.activeVisualizationId || undefined} - onChange={e => { - const newState = getSuggestedVisualizationState( - props.framePublicAPI, - props.visualizationMap[e.target.value] - ); - props.dispatch({ - type: 'SWITCH_VISUALIZATION', - newVisualizationId: e.target.value, - initialState: newState, - }); + { + const visualization = props.visualizationMap[visualizationId]; + const switchVisualizationType = visualization.switchVisualizationType; + + if (visualizationId !== props.activeVisualizationId) { + const newState = getSuggestedVisualizationState(props.framePublicAPI, visualization); + props.dispatch({ + type: 'SWITCH_VISUALIZATION', + newVisualizationId: visualizationId, + initialState: switchVisualizationType + ? switchVisualizationType(subVisualizationId, newState) + : newState, + }); + } else if (switchVisualizationType) { + setVisualizationState( + switchVisualizationType(subVisualizationId, props.visualizationState) + ); + } }} /> {props.activeVisualizationId && props.visualizationState !== null && ( diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx index 8dbf80d9afc74..32ed4c335975c 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx @@ -53,8 +53,28 @@ describe('editor_frame', () => { }; beforeEach(() => { - mockVisualization = createMockVisualization(); - mockVisualization2 = createMockVisualization(); + mockVisualization = { + ...createMockVisualization(), + id: 'testVis', + visualizationTypes: [ + { + icon: 'empty', + id: 'testVis', + label: 'TEST1', + }, + ], + }; + mockVisualization2 = { + ...createMockVisualization(), + id: 'testVis2', + visualizationTypes: [ + { + icon: 'empty', + id: 'testVis2', + label: 'TEST2', + }, + ], + }; mockDatasource = createMockDatasource(); mockDatasource2 = createMockDatasource(); @@ -751,8 +771,27 @@ describe('editor_frame', () => { describe('switching', () => { let instance: ReactWrapper; + function switchTo(subType: string) { + act(() => { + instance + .find('[data-test-subj="lnsConfigPanelHeaderPopover"]') + .last() + .simulate('click'); + }); + + instance.update(); + + act(() => { + instance + .find(`[data-test-subj="lnsConfigPanelHeaderPopover_${subType}"]`) + .last() + .simulate('click'); + }); + } + beforeEach(async () => { mockDatasource.getLayers.mockReturnValue(['first']); + instance = mount( { }); it('should initialize other visualization on switch', async () => { - act(() => { - instance - .find('select[data-test-subj="visualization-switch"]') - .simulate('change', { target: { value: 'testVis2' } }); - }); + switchTo('testVis2'); expect(mockVisualization2.initialize).toHaveBeenCalled(); }); @@ -839,11 +874,7 @@ describe('editor_frame', () => { }, ]); - act(() => { - instance - .find('select[data-test-subj="visualization-switch"]') - .simulate('change', { target: { value: 'testVis2' } }); - }); + switchTo('testVis2'); expect(mockVisualization2.getSuggestions).toHaveBeenCalled(); expect(mockVisualization2.initialize).toHaveBeenCalledWith(expect.anything(), initialState); @@ -856,11 +887,7 @@ describe('editor_frame', () => { it('should fall back when switching visualizations if the visualization has no suggested use', async () => { mockVisualization2.initialize.mockReturnValueOnce({ initial: true }); - act(() => { - instance - .find('select[data-test-subj="visualization-switch"]') - .simulate('change', { target: { value: 'testVis2' } }); - }); + switchTo('testVis2'); expect(mockDatasource.publicAPIMock.getTableSpec).toHaveBeenCalled(); expect(mockVisualization2.getSuggestions).toHaveBeenCalled(); diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index cdc0f9c19e21e..e7eb5523cd712 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -90,7 +90,8 @@ export function EditorFrame(props: EditorFrameProps) { const framePublicAPI: FramePublicAPI = { datasourceLayers, - addNewLayer: () => { + + addNewLayer() { const newLayerId = generateId(); const newState = props.datasourceMap[state.activeDatasourceId!].insertLayer( diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx index 225b9802f113d..cf6958dacdbd2 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx @@ -11,6 +11,16 @@ import { EditorFrameSetupPlugins } from './plugin'; export function createMockVisualization(): jest.Mocked { return { + id: 'TEST_VIS', + visualizationTypes: [ + { + icon: 'empty', + id: 'TEST_VIS', + label: 'TEST', + }, + ], + renderDescription: jest.fn(), + switchVisualizationType: jest.fn((_, x) => x), getPersistableState: jest.fn(_state => ({})), getSuggestions: jest.fn(_options => []), initialize: jest.fn((_frame, _state?) => ({})), diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/plugin.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/plugin.tsx index 359ac72bbd379..e883db8457ea6 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/plugin.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/plugin.tsx @@ -133,8 +133,8 @@ export class EditorFramePlugin { registerDatasource: (name, datasource) => { this.datasources[name] = datasource as Datasource; }, - registerVisualization: (name, visualization) => { - this.visualizations[name] = visualization as Visualization; + registerVisualization: visualization => { + this.visualizations[visualization.id] = visualization as Visualization; }, }; } diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index 3c75377991b3e..abd9c974be75e 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -5,6 +5,7 @@ */ import { Ast } from '@kbn/interpreter/common'; +import { EuiIconType } from '@elastic/eui/src/components/icon/icon'; import { DragContextState } from './drag_drop'; // eslint-disable-next-line @@ -21,7 +22,7 @@ export interface EditorFrameSetup { createInstance: (options: EditorFrameOptions) => EditorFrameInstance; // generic type on the API functions to pull the "unknown vs. specific type" error into the implementation registerDatasource: (name: string, datasource: Datasource) => void; - registerVisualization: (name: string, visualization: Visualization) => void; + registerVisualization: (visualization: Visualization) => void; } // Hints the default nesting to the data source. 0 is the highest priority @@ -182,7 +183,20 @@ export interface FramePublicAPI { removeLayer: (layerId: string) => void; } +export interface VisualizationType { + id: string; + icon?: EuiIconType | string; + label: string; +} + export interface Visualization { + id: string; + + visualizationTypes: VisualizationType[]; + + renderDescription: (domElement: Element, state: T) => void; + switchVisualizationType?: (visualizationTypeId: string, state: T) => T; + // For initializing from saved object initialize: (frame: FramePublicAPI, state?: P) => T; diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts index 90579747e6862..d78b685789789 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts @@ -5,10 +5,12 @@ */ import { Position } from '@elastic/charts'; +import { i18n } from '@kbn/i18n'; import { ExpressionFunction, ArgumentType, } from '../../../../../../src/legacy/core_plugins/interpreter/public'; +import { VisualizationType } from '..'; export interface LegendConfig { isVisible: boolean; @@ -202,9 +204,62 @@ export interface XYArgs { // Persisted parts of the state export interface XYState { + preferredSeriesType: SeriesType; legend: LegendConfig; layers: LayerConfig[]; } export type State = XYState; export type PersistableState = XYState; + +export const visualizationTypes: VisualizationType[] = [ + { + id: 'bar', + icon: 'visBarVertical', + label: i18n.translate('xpack.lens.xyVisualization.verticalBarLabel', { + defaultMessage: 'Vertical Bar', + }), + }, + { + id: 'bar_stacked', + icon: 'visBarVertical', + label: i18n.translate('xpack.lens.xyVisualization.stackedVerticalBarLabel', { + defaultMessage: 'Stacked Vertical Bar', + }), + }, + { + id: 'horizontal_bar', + icon: 'visBarHorizontal', + label: i18n.translate('xpack.lens.xyVisualization.horizontalBarLabel', { + defaultMessage: 'Horizontal Bar', + }), + }, + { + id: 'horizontal_bar_stacked', + icon: 'visBarHorizontal', + label: i18n.translate('xpack.lens.xyVisualization.stackedHorizontalBarLabel', { + defaultMessage: 'Stacked Horizontal Bar', + }), + }, + { + id: 'line', + icon: 'visLine', + label: i18n.translate('xpack.lens.xyVisualization.lineLabel', { + defaultMessage: 'Line', + }), + }, + { + id: 'area', + icon: 'visArea', + label: i18n.translate('xpack.lens.xyVisualization.areaLabel', { + defaultMessage: 'Area', + }), + }, + { + id: 'area_stacked', + icon: 'visArea', + label: i18n.translate('xpack.lens.xyVisualization.stackedAreaLabel', { + defaultMessage: 'Stacked Area', + }), + }, +]; diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx index 15fcec52cbfca..9176f0debc762 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx @@ -26,6 +26,7 @@ describe('XYConfigPanel', () => { function testState(): State { return { legend: { isVisible: true, position: Position.Right }, + preferredSeriesType: 'bar', layers: [ { seriesType: 'bar', @@ -72,19 +73,24 @@ describe('XYConfigPanel', () => { /> ); + component + .find('[data-test-subj="lnsXYSeriesTypePopover"]') + .first() + .simulate('click'); + const options = component .find('[data-test-subj="lnsXY_seriesType"]') .first() .prop('options') as EuiButtonGroupProps['options']; expect(options.map(({ id }) => id)).toEqual([ - 'line', - 'area', 'bar', - 'horizontal_bar', - 'area_stacked', 'bar_stacked', + 'horizontal_bar', 'horizontal_bar_stacked', + 'line', + 'area', + 'area_stacked', ]); expect(options.filter(({ isDisabled }) => isDisabled).map(({ id }) => id)).toEqual([]); @@ -255,6 +261,7 @@ describe('XYConfigPanel', () => { ], }); }); + it('removes layers', () => { const setState = jest.fn(); const state = testState(); @@ -267,6 +274,11 @@ describe('XYConfigPanel', () => { /> ); + component + .find('[data-test-subj="lnsXYSeriesTypePopover"]') + .first() + .simulate('click'); + component .find('[data-test-subj="lnsXY_layer_remove"]') .first() diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index 14d1523df6ec9..5731f8feaecb0 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React from 'react'; +import React, { useState } from 'react'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { Position } from '@elastic/charts'; @@ -14,67 +14,15 @@ import { EuiForm, EuiFormRow, EuiPanel, - IconType, EuiButtonIcon, + EuiPopover, } from '@elastic/eui'; -import { State, SeriesType, LayerConfig } from './types'; +import { State, SeriesType, LayerConfig, visualizationTypes } from './types'; import { VisualizationProps } from '../types'; import { NativeRenderer } from '../native_renderer'; import { MultiColumnEditor } from '../multi_column_editor'; import { generateId } from '../id_generator'; -export const chartTypeIcons: Array<{ id: SeriesType; label: string; iconType: IconType }> = [ - { - id: 'line', - label: i18n.translate('xpack.lens.xyVisualization.lineChartLabel', { - defaultMessage: 'Line', - }), - iconType: 'visLine', - }, - { - id: 'area', - label: i18n.translate('xpack.lens.xyVisualization.areaChartLabel', { - defaultMessage: 'Area', - }), - iconType: 'visArea', - }, - { - id: 'bar', - label: i18n.translate('xpack.lens.xyVisualization.barChartLabel', { - defaultMessage: 'Bar', - }), - iconType: 'visBarVertical', - }, - { - id: 'horizontal_bar', - label: i18n.translate('xpack.lens.xyVisualization.horizontalBarChartLabel', { - defaultMessage: 'Horizontal Bar', - }), - iconType: 'visBarHorizontal', - }, - { - id: 'area_stacked', - label: i18n.translate('xpack.lens.xyVisualization.stackedAreaChartLabel', { - defaultMessage: 'Stacked Area', - }), - iconType: 'visArea', - }, - { - id: 'bar_stacked', - label: i18n.translate('xpack.lens.xyVisualization.stackedBarChartLabel', { - defaultMessage: 'Stacked Bar', - }), - iconType: 'visBarVertical', - }, - { - id: 'horizontal_bar_stacked', - label: i18n.translate('xpack.lens.xyVisualization.stackedHorizontalBarChartLabel', { - defaultMessage: 'Stacked Horizontal Bar', - }), - iconType: 'visBarHorizontal', - }, -]; - type UnwrapArray = T extends Array ? P : T; function updateLayer(state: State, layer: UnwrapArray, index: number): State { @@ -87,11 +35,11 @@ function updateLayer(state: State, layer: UnwrapArray, index: n }; } -function newLayerState(layerId: string): LayerConfig { +function newLayerState(seriesType: SeriesType, layerId: string): LayerConfig { return { layerId, + seriesType, xAccessor: generateId(), - seriesType: 'bar_stacked', accessors: [generateId()], title: '', showGridlines: false, @@ -100,6 +48,66 @@ function newLayerState(layerId: string): LayerConfig { }; } +function LayerSeriesTypeConfig({ + layer, + setSeriesType, + removeLayer, +}: { + layer: LayerConfig; + setSeriesType: (seriesType: SeriesType) => void; + removeLayer: () => void; +}) { + const [isOpen, setIsOpen] = useState(false); + const { icon, label } = visualizationTypes.find(c => c.id === layer.seriesType)!; + + return ( + setIsOpen(!isOpen)} + data-test-subj="lnsXYSeriesTypePopover" + /> + } + isOpen={isOpen} + closePopover={() => setIsOpen(false)} + anchorPosition="leftUp" + > + + ({ + ...t, + iconType: t.icon || 'empty', + }))} + idSelected={layer.seriesType} + onChange={seriesType => setSeriesType(seriesType as SeriesType)} + isIconOnly + /> + + + + {i18n.translate('xpack.lens.xyChart.removeLayer', { + defaultMessage: 'Remove layer', + })} + + + + ); +} + export function XYConfigPanel(props: VisualizationProps) { const { state, setState, frame } = props; @@ -114,16 +122,15 @@ export function XYConfigPanel(props: VisualizationProps) { })} > <> - { + + setState(updateLayer(state, { ...layer, seriesType }, index)) + } + removeLayer={() => { frame.removeLayer(layer.layerId); setState({ ...state, layers: state.layers.filter(l => l !== layer) }); }} - aria-label={i18n.translate('xpack.lens.xyChart.removeLayer', { - defaultMessage: 'Remove layer', - })} /> ) { - - { - setState( - updateLayer(state, { ...layer, seriesType: seriesType as SeriesType }, index) - ); - }} - isIconOnly - /> - - ) { onClick={() => { setState({ ...state, - layers: [...state.layers, newLayerState(frame.addNewLayer())], + layers: [ + ...state.layers, + newLayerState(state.preferredSeriesType, frame.addNewLayer()), + ], }); }} iconType="plusInCircle" diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx index f65c8c990b976..c7a03fd540227 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx @@ -21,9 +21,8 @@ import { ExpressionFunction } from 'src/legacy/core_plugins/interpreter/types'; import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiText, IconType } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; import { LensMultiTable } from '../types'; -import { XYArgs, SeriesType } from './types'; +import { XYArgs, SeriesType, visualizationTypes } from './types'; import { RenderFunction } from '../interpreter_types'; -import { chartTypeIcons } from './xy_config_panel'; export interface XYChartProps { data: LensMultiTable; @@ -92,7 +91,7 @@ export const xyChartRenderer: RenderFunction = { }; function getIconForSeriesType(seriesType: SeriesType): IconType { - return chartTypeIcons.find(chartTypeIcon => chartTypeIcon.id === seriesType)!.iconType; + return visualizationTypes.find(c => c.id === seriesType)!.icon || 'empty'; } export function XYChart({ data, args }: XYChartProps) { diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts index eeffcfd0036fe..86ed05708e83a 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts @@ -12,7 +12,7 @@ import { TableSuggestionColumn, TableSuggestion, } from '../types'; -import { State } from './types'; +import { State, SeriesType } from './types'; import { generateId } from '../id_generator'; import { buildExpression } from './to_expression'; @@ -101,14 +101,17 @@ function getSuggestion( // TODO: Localize the title, label, etc const preposition = isDate ? 'over' : 'of'; const title = `${yTitle} ${preposition} ${xTitle}`; + const seriesType: SeriesType = + (currentState && currentState.preferredSeriesType) || (splitBy && isDate ? 'line' : 'bar'); const state: State = { legend: currentState ? currentState.legend : { isVisible: true, position: Position.Right }, + preferredSeriesType: seriesType, layers: [ ...(currentState ? currentState.layers.filter(layer => layer.layerId !== layerId) : []), { layerId, + seriesType, xAccessor: xValue.columnId, - seriesType: splitBy && isDate ? 'line' : 'bar', splitAccessor: splitBy && isDate ? splitBy.columnId : generateId(), accessors: yValues.map(col => col.columnId), position: Position.Left, diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts index 060f645717ee4..846c8442bf770 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts @@ -17,6 +17,7 @@ jest.mock('../id_generator'); function exampleState(): State { return { legend: { position: Position.Bottom, isVisible: true }, + preferredSeriesType: 'bar', layers: [ { layerId: 'first', @@ -67,6 +68,7 @@ describe('xy_visualization', () => { "isVisible": true, "position": "right", }, + "preferredSeriesType": "bar", "title": "Empty XY Chart", } `); diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx index 35825dbc228db..f6cce7522eed1 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx @@ -8,14 +8,71 @@ import React from 'react'; import { render } from 'react-dom'; import { Position } from '@elastic/charts'; import { I18nProvider } from '@kbn/i18n/react'; +import { i18n } from '@kbn/i18n'; +import { EuiIcon } from '@elastic/eui'; import { getSuggestions } from './xy_suggestions'; import { XYConfigPanel } from './xy_config_panel'; import { Visualization } from '../types'; -import { State, PersistableState } from './types'; +import { State, PersistableState, SeriesType, visualizationTypes } from './types'; import { toExpression } from './to_expression'; import { generateId } from '../id_generator'; +const defaultIcon = 'visBarVertical'; +const defaultSeriesType = 'bar'; + +function getDescription(state?: State) { + if (!state) { + return { + icon: defaultIcon, + label: i18n.translate('xpack.lens.xyVisualization.xyLabel', { + defaultMessage: 'XY Chart', + }), + }; + } + + if (!state.layers.length) { + return visualizationTypes.find(v => v.id === state.preferredSeriesType)!; + } + + const visualizationType = visualizationTypes.find(t => t.id === state.layers[0].seriesType)!; + const seriesTypes = _.unique(state.layers.map(l => l.seriesType)); + + return { + icon: visualizationType.icon, + label: + seriesTypes.length === 1 + ? visualizationType.label + : i18n.translate('xpack.lens.xyVisualization.mixedLabel', { + defaultMessage: 'Mixed XY Chart', + }), + }; +} + export const xyVisualization: Visualization = { + id: 'lnsXY', + + visualizationTypes, + + renderDescription(el, state) { + const { icon, label } = getDescription(state); + + render( + <> + + {label} + , + el + ); + }, + + switchVisualizationType(seriesType: string, state: State) { + return { + ...state, + preferredSeriesType: seriesType as SeriesType, + layers: state.layers.map(layer => ({ ...layer, seriesType: seriesType as SeriesType })), + }; + }, + getSuggestions, initialize(frame, state) { @@ -23,12 +80,13 @@ export const xyVisualization: Visualization = { state || { title: 'Empty XY Chart', legend: { isVisible: true, position: Position.Right }, + preferredSeriesType: defaultSeriesType, layers: [ { layerId: frame.addNewLayer(), accessors: [generateId()], position: Position.Top, - seriesType: 'bar', + seriesType: defaultSeriesType, showGridlines: false, splitAccessor: generateId(), title: '', From ed234e084d9c49af7085108b4f79e2e5c11d7e09 Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Fri, 26 Jul 2019 11:49:04 -0400 Subject: [PATCH 02/16] Remove horizontal bars --- .../lens/public/xy_visualization_plugin/types.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts index d1e282fc05273..0d69601cf0810 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts @@ -203,20 +203,6 @@ export const visualizationTypes: VisualizationType[] = [ defaultMessage: 'Stacked Vertical Bar', }), }, - { - id: 'horizontal_bar', - icon: 'visBarHorizontal', - label: i18n.translate('xpack.lens.xyVisualization.horizontalBarLabel', { - defaultMessage: 'Horizontal Bar', - }), - }, - { - id: 'horizontal_bar_stacked', - icon: 'visBarHorizontal', - label: i18n.translate('xpack.lens.xyVisualization.stackedHorizontalBarLabel', { - defaultMessage: 'Stacked Horizontal Bar', - }), - }, { id: 'line', icon: 'visLine', From 9549e2c583864bbb187178ea7653d7e15d49a1f5 Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Fri, 26 Jul 2019 11:55:54 -0400 Subject: [PATCH 03/16] Fix tests --- .../xy_config_panel.test.tsx | 7 ------- .../xy_visualization_plugin/xy_config_panel.tsx | 17 +++++------------ 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx index 249235b1dc659..e77395f21920b 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx @@ -122,8 +122,6 @@ describe('XYConfigPanel', () => { expect(options.map(({ id }) => id)).toEqual([ 'bar', 'bar_stacked', - 'horizontal_bar', - 'horizontal_bar_stacked', 'line', 'area', 'area_stacked', @@ -314,11 +312,6 @@ describe('XYConfigPanel', () => { openComponentPopover(component, 'first'); - component - .find('[data-test-subj="lnsXYSeriesTypePopover"]') - .first() - .simulate('click'); - component .find('[data-test-subj="lnsXY_layer_remove"]') .first() diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index 613c114eca0c6..ed493848bf979 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -71,7 +71,7 @@ function LayerSeriesTypeConfig({ defaultMessage: 'Edit layer settings', })} onClick={() => setIsOpen(!isOpen)} - data-test-subj="lnsXYSeriesTypePopover" + data-test-subj="lnsXY_layer_advanced" /> } isOpen={isOpen} @@ -117,20 +117,15 @@ function LayerSeriesTypeConfig({ export function XYConfigPanel(props: VisualizationProps) { const { state, setState, frame } = props; - const [localState, setLocalState] = useState({ - isChartOptionsOpen: false, - openLayerId: null, - }); + const [isChartOptionsOpen, setIsChartOptionsOpen] = useState(false); return ( { - setLocalState({ ...localState, isChartOptionsOpen: false }); - }} + isOpen={isChartOptionsOpen} + closePopover={() => setIsChartOptionsOpen(false)} button={ <> @@ -138,9 +133,7 @@ export function XYConfigPanel(props: VisualizationProps) { iconType="gear" size="s" data-test-subj="lnsXY_chart_settings" - onClick={() => { - setLocalState({ ...localState, isChartOptionsOpen: true }); - }} + onClick={() => setIsChartOptionsOpen(true)} > Date: Fri, 26 Jul 2019 14:50:59 -0400 Subject: [PATCH 04/16] Add chart switching confirmation dialog --- .../visualization.tsx | 2 +- .../editor_frame/config_panel_header.tsx | 180 +++++++++++++----- .../editor_frame/config_panel_wrapper.tsx | 21 +- 3 files changed, 154 insertions(+), 49 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx index ae3d28caaed4d..f45fcce780a81 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx @@ -155,7 +155,7 @@ export const datatableVisualization: Visualization< {props.state.layers.map(layer => ( - + ))} , diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx index 8cfe3ac52f5cc..65230341e20ba 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx @@ -12,17 +12,30 @@ import { EuiPopoverTitle, EuiKeyPadMenu, EuiKeyPadMenuItemButton, + EuiOverlayMask, + EuiConfirmModal, } from '@elastic/eui'; import { flatten } from 'lodash'; import { i18n } from '@kbn/i18n'; import { NativeRenderer } from '../../native_renderer'; import { Visualization } from '../../types'; +interface VisualizationSelection { + visualizationId: string; + subVisualizationId: string; +} + +interface State { + isFlyoutOpen: boolean; + confirmVisualization?: VisualizationSelection; +} + interface Props { visualizations: Visualization[]; + requireConfirmation: (visualizationId: string) => boolean; visualizationId: string | null; visualizationState: unknown; - onChange: (visualizationId: string, subVisualizationId: string) => void; + onChange: (selection: VisualizationSelection) => void; } function VisualizationSummary(props: Props) { @@ -46,11 +59,81 @@ function VisualizationSummary(props: Props) { ); } +function ConfirmationModal({ + onCancel, + onConfirm, + visualizationLabel, +}: { + visualizationLabel: string; + onCancel: () => void; + onConfirm: () => void; +}) { + return ( + + +

+ {i18n.translate('xpack.lens.configPanel.dropLayersDescription', { + defaultMessage: 'Switching to {visualizationLabel} will drop all but the first layers.', + values: { visualizationLabel }, + })} +

+

+ {i18n.translate('xpack.lens.configPanel.dropLayersPrompt', { + defaultMessage: 'Are you sure you want to drop layers?', + })} +

+
+
+ ); +} + +function selectedVisualizationType(selection: VisualizationSelection, props: Props) { + const visualization = props.visualizations.find(v => v.id === selection.visualizationId); + return visualization!.visualizationTypes.find(v => v.id === selection.subVisualizationId)!; +} + export function ConfigPanelHeader(props: Props) { - const [state, setState] = useState({ - isFlyoutOpen: false, - }); - const close = () => setState({ isFlyoutOpen: false }); + const [state, setState] = useState({ isFlyoutOpen: false }); + + const hideFlyout = (force: boolean = false) => { + if (force || !state.confirmVisualization) { + setState({ isFlyoutOpen: false }); + } + }; + + const requestConfirmation = (confirmVisualization: VisualizationSelection) => { + setState({ + ...state, + confirmVisualization, + }); + }; + + const commitSelection = (selection: VisualizationSelection) => { + hideFlyout(true); + props.onChange(selection); + }; + + const onSelect = (selection: VisualizationSelection) => { + if (props.requireConfirmation(selection.visualizationId)) { + requestConfirmation(selection); + } else { + commitSelection(selection); + } + }; + const visualizationTypes = flatten( props.visualizations.map(v => v.visualizationTypes.map(t => ({ @@ -61,44 +144,55 @@ export function ConfigPanelHeader(props: Props) { ); return ( - setState({ isFlyoutOpen: !state.isFlyoutOpen })} - data-test-subj="lnsConfigPanelHeaderPopover" - > - - - } - isOpen={state.isFlyoutOpen} - closePopover={close} - anchorPosition="leftUp" - > - - {i18n.translate('xpack.lens.configPanel.chooseVisualization', { - defaultMessage: 'Choose a visualization', - })} - - - {visualizationTypes.map(v => ( - {v.label}} - role="menuitem" - data-test-subj={`lnsConfigPanelHeaderPopover_${v.id}`} - onClick={() => { - props.onChange(v.visualizationId, v.id); - close(); - }} + <> + {state.confirmVisualization && ( + setState({ ...state, confirmVisualization: undefined })} + onConfirm={() => commitSelection(state.confirmVisualization!)} + /> + )} + setState({ isFlyoutOpen: !state.isFlyoutOpen })} + data-test-subj="lnsConfigPanelHeaderPopover" > - - - ))} - - + + + } + isOpen={state.isFlyoutOpen} + closePopover={() => hideFlyout()} + anchorPosition="leftUp" + > + + {i18n.translate('xpack.lens.configPanel.chooseVisualization', { + defaultMessage: 'Choose a visualization', + })} + + + {visualizationTypes.map(v => ( + {v.label}} + role="menuitem" + data-test-subj={`lnsConfigPanelHeaderPopover_${v.id}`} + onClick={() => + onSelect({ + visualizationId: v.visualizationId, + subVisualizationId: v.id, + }) + } + > + + + ))} + +
+ ); } diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx index 3d0229b3cc05f..12182c1d6e217 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx @@ -35,11 +35,18 @@ function getSuggestedVisualizationState(frame: FramePublicAPI, visualization: Vi ], }); - if (!suggestions.length) { - return visualization.initialize(frame); - } + const suggestion = suggestions.length ? suggestions[0] : undefined; - return visualization.initialize(frame, suggestions[0].state); + // Remove any layers that are not used by the new visualization. If we don't do this, + // we get orphaned objects, and weird edge cases such as prompting the user that + // layers are going to be dropped, when the user is unaware of any extraneous layers. + Object.keys(frame.datasourceLayers).forEach(id => { + if (!suggestion || id !== layerId) { + frame.removeLayer(id); + } + }); + + return visualization.initialize(frame, suggestion && suggestion.state); } export const ConfigPanelWrapper = memo(function ConfigPanelWrapper(props: ConfigPanelWrapperProps) { @@ -60,7 +67,11 @@ export const ConfigPanelWrapper = memo(function ConfigPanelWrapper(props: Config visualizations={Object.values(props.visualizationMap)} visualizationId={props.activeVisualizationId} visualizationState={props.visualizationState} - onChange={(visualizationId, subVisualizationId) => { + requireConfirmation={visualizationId => + props.activeVisualizationId !== visualizationId && + Object.keys(props.framePublicAPI.datasourceLayers).length > 1 + } + onChange={({ visualizationId, subVisualizationId }) => { const visualization = props.visualizationMap[visualizationId]; const switchVisualizationType = visualization.switchVisualizationType; From a24956f3a0b87a8adaac97a387088a86142cb669 Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Fri, 26 Jul 2019 14:52:41 -0400 Subject: [PATCH 05/16] Fix tests --- .../editor_frame_plugin/editor_frame/editor_frame.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx index e4e07ddab3323..73db4ba22b19d 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx @@ -936,7 +936,8 @@ describe('editor_frame', () => { expect(mockDatasource.publicAPIMock.getTableSpec).toHaveBeenCalled(); expect(mockVisualization2.getSuggestions).toHaveBeenCalled(); expect(mockVisualization2.initialize).toHaveBeenCalledWith( - expect.objectContaining({ datasourceLayers: { first: mockDatasource.publicAPIMock } }) + expect.objectContaining({ datasourceLayers: { first: mockDatasource.publicAPIMock } }), + undefined ); expect(mockVisualization2.renderConfigPanel).toHaveBeenCalledWith( expect.any(Element), From 14c3a60af2f125fad1aa418ba255315ba641396c Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Fri, 26 Jul 2019 15:15:54 -0400 Subject: [PATCH 06/16] Minor tweaks --- .../visualization.tsx | 13 +++++++++---- .../xy_visualization_plugin/xy_config_panel.tsx | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx index f45fcce780a81..aa43d79aacdce 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx @@ -102,12 +102,17 @@ export const datatableVisualization: Visualization< ], renderDescription(el) { - render(

Datatable

, el); + render( + <> + {i18n.translate('xpack.lens.datatable.label', { + defaultMessage: 'Datatable', + })} + , + el + ); }, - switchVisualizationType(_: string, state: DatatableVisualizationState) { - return state; - }, + switchVisualizationType: (_, state) => state, initialize(frame, state) { const layerId = Object.keys(frame.datasourceLayers)[0] || frame.addNewLayer(); diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index ed493848bf979..2fa12d4b2bbbc 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -48,7 +48,7 @@ function newLayerState(seriesType: SeriesType, layerId: string): LayerConfig { }; } -function LayerSeriesTypeConfig({ +function LayerSettings({ layer, setSeriesType, removeLayer, @@ -170,7 +170,7 @@ export function XYConfigPanel(props: VisualizationProps) { - setState(updateLayer(state, { ...layer, seriesType }, index)) From b9bf6c123b783ec6018d05f083dfd96a1a21e9a3 Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Fri, 26 Jul 2019 17:33:09 -0400 Subject: [PATCH 07/16] Add visualization switcher tests --- .../editor_frame/config_panel_header.test.tsx | 289 ++++++++++++++++++ .../editor_frame/config_panel_header.tsx | 149 +++++++-- .../editor_frame/config_panel_wrapper.tsx | 58 +--- 3 files changed, 414 insertions(+), 82 deletions(-) create mode 100644 x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx new file mode 100644 index 0000000000000..661c925590702 --- /dev/null +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx @@ -0,0 +1,289 @@ +/* + * 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 { createMockVisualization, createMockFramePublicAPI } from '../mocks'; +import { mountWithIntl as mount } from 'test_utils/enzyme_helpers'; +import { ReactWrapper } from 'enzyme'; +import { ConfigPanelHeader } from './config_panel_header'; +import { Visualization } from '../../types'; + +describe('config_panel_header', () => { + function generateVisualization(id: string): jest.Mocked { + return { + ...createMockVisualization(), + id, + visualizationTypes: [ + { + icon: 'empty', + id: `sub${id}`, + label: `Label ${id}`, + }, + ], + initialize: jest.fn((_frame, state?: unknown) => { + return state || `${id} initial state`; + }), + getSuggestions: jest.fn(_options => { + return [ + { + score: 1, + title: '', + state: `suggestion ${id}`, + datasourceSuggestionId: 1, + previewIcon: 'empty', + }, + ]; + }), + }; + } + + function mockVisualizations() { + return { + visA: generateVisualization('visA'), + visB: generateVisualization('visB'), + visC: { + ...generateVisualization('visC'), + visualizationTypes: [ + { + icon: 'empty', + id: 'subvisC1', + label: 'C1', + }, + { + icon: 'empty', + id: 'subvisC2', + label: 'C2', + }, + ], + }, + }; + } + + function mockFrame(layers: string[]) { + return { + ...createMockFramePublicAPI(), + datasourceLayers: layers.reduce( + jest.fn((acc, layerId) => ({ + ...acc, + [layerId]: { + getTableSpec() { + return [{ columnId: 2 }]; + }, + getOperationForColumnId() { + return {}; + }, + }, + })), + {} + ), + }; + } + + function showFlyout(component: ReactWrapper) { + component + .find('[data-test-subj="lnsConfigPanelHeaderPopover"]') + .first() + .simulate('click'); + } + + function switchTo(subType: string, component: ReactWrapper) { + showFlyout(component); + component + .find(`[data-test-subj="lnsConfigPanelHeaderPopover_${subType}"]`) + .first() + .simulate('click'); + } + + function confirm(component: ReactWrapper) { + component + .find('[data-test-subj="confirmModalConfirmButton"]') + .first() + .simulate('click'); + } + + function deny(component: ReactWrapper) { + component + .find('[data-test-subj="confirmModalCancelButton"]') + .first() + .simulate('click'); + } + + function isModalVisible(component: ReactWrapper) { + return component.find('[data-test-subj="lnsConfirmDropLayer"]').length > 0; + } + + it('should not prompt for confirmation if there is only one layer', () => { + const dispatch = jest.fn(); + const visualizations = mockVisualizations(); + const component = mount( + + ); + + switchTo('subvisB', component); + + expect(dispatch).toHaveBeenCalledWith({ + initialState: 'suggestion visB', + newVisualizationId: 'visB', + type: 'SWITCH_VISUALIZATION', + }); + }); + + it('should prompt for confirmation if there is more than one layer', () => { + const dispatch = jest.fn(); + const component = mount( + + ); + + switchTo('subvisB', component); + + expect(dispatch).not.toHaveBeenCalled(); + + expect(isModalVisible(component)).toBeTruthy(); + confirm(component); + + expect(isModalVisible(component)).toBeFalsy(); + + expect(dispatch).toHaveBeenCalledWith({ + initialState: 'suggestion visB', + newVisualizationId: 'visB', + type: 'SWITCH_VISUALIZATION', + }); + }); + + it('should remove unused layers', () => { + const removeLayer = jest.fn(); + const frame = { + ...mockFrame(['a', 'b', 'c']), + removeLayer, + }; + const component = mount( + + ); + + switchTo('subvisB', component); + confirm(component); + + expect(removeLayer).toHaveBeenCalledTimes(2); + expect(removeLayer).toHaveBeenCalledWith('b'); + expect(removeLayer).toHaveBeenCalledWith('c'); + }); + + it('should not prompt for confirmation if the visualization is not changing', () => { + const dispatch = jest.fn(); + const visualizations = mockVisualizations(); + const switchVisualizationType = jest.fn(() => 'therebedragons'); + + visualizations.visC.switchVisualizationType = switchVisualizationType; + + const component = mount( + + ); + + switchTo('subvisC2', component); + expect(isModalVisible(component)).toBeFalsy(); + expect(switchVisualizationType).toHaveBeenCalledWith('subvisC2', 'therebegriffins'); + expect(dispatch).toHaveBeenCalledWith({ + type: 'UPDATE_VISUALIZATION_STATE', + newState: 'therebedragons', + }); + }); + + it('should ensure the new visualization has the proper subtype', () => { + const dispatch = jest.fn(); + const visualizations = mockVisualizations(); + const switchVisualizationType = jest.fn( + (visualizationType, state) => `${state} ${visualizationType}` + ); + + visualizations.visB.switchVisualizationType = switchVisualizationType; + + const component = mount( + + ); + + switchTo('subvisB', component); + + expect(dispatch).toHaveBeenCalledWith({ + initialState: 'suggestion visB subvisB', + newVisualizationId: 'visB', + type: 'SWITCH_VISUALIZATION', + }); + }); + + it('should not process the change, if cancelled', () => { + const dispatch = jest.fn(); + const component = mount( + + ); + + switchTo('subvisB', component); + + expect(isModalVisible(component)).toBeTruthy(); + expect(dispatch).not.toHaveBeenCalled(); + + deny(component); + + expect(isModalVisible(component)).toBeFalsy(); + expect(dispatch).not.toHaveBeenCalled(); + }); + + it('should show all visualization types', () => { + const component = mount( + + ); + + showFlyout(component); + + const allDisplayed = ['subvisA', 'subvisB', 'subvisC1', 'subvisC2'].every( + subType => + component.find(`[data-test-subj="lnsConfigPanelHeaderPopover_${subType}"]`).length > 0 + ); + + expect(allDisplayed).toBeTruthy(); + }); +}); diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx index 65230341e20ba..a1d303ef2a469 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useState } from 'react'; +import React, { useState, useMemo } from 'react'; import { EuiIcon, EuiPopover, @@ -18,7 +18,8 @@ import { import { flatten } from 'lodash'; import { i18n } from '@kbn/i18n'; import { NativeRenderer } from '../../native_renderer'; -import { Visualization } from '../../types'; +import { Visualization, FramePublicAPI } from '../../types'; +import { Action } from './state_management'; interface VisualizationSelection { visualizationId: string; @@ -31,15 +32,67 @@ interface State { } interface Props { - visualizations: Visualization[]; - requireConfirmation: (visualizationId: string) => boolean; + dispatch: (action: Action) => void; + visualizationMap: Record; visualizationId: string | null; visualizationState: unknown; - onChange: (selection: VisualizationSelection) => void; + framePublicAPI: FramePublicAPI; +} + +function getSuggestion(frame: FramePublicAPI, visualization: Visualization) { + const layers = Object.entries(frame.datasourceLayers); + + if (!layers.length) { + return { + suggestion: undefined, + layerId: undefined, + }; + } + + const [[layerId, datasource]] = layers; + const suggestions = visualization.getSuggestions({ + tables: [ + { + datasourceSuggestionId: 0, + isMultiRow: true, + columns: datasource.getTableSpec().map(col => ({ + ...col, + operation: datasource.getOperationForColumnId(col.columnId)!, + })), + layerId, + }, + ], + }); + + const suggestion = suggestions.length ? suggestions[0] : undefined; + return { layerId, suggestion }; +} + +function dropUnusedLayers(frame: FramePublicAPI, suggestion: unknown, layerId?: string) { + // Remove any layers that are not used by the new visualization. If we don't do this, + // we get orphaned objects, and weird edge cases such as prompting the user that + // layers are going to be dropped, when the user is unaware of any extraneous layers. + Object.keys(frame.datasourceLayers).forEach(id => { + if (!suggestion || id !== layerId) { + frame.removeLayer(id); + } + }); +} + +function willLoseAllLayers(frame: FramePublicAPI, visualization: Visualization) { + return !getSuggestion(frame, visualization).layerId; +} + +function getNewVisualizationState(frame: FramePublicAPI, visualization: Visualization) { + const { suggestion, layerId } = getSuggestion(frame, visualization); + + dropUnusedLayers(frame, suggestion, layerId); + + return visualization.initialize(frame, suggestion && suggestion.state); } function VisualizationSummary(props: Props) { - const visualization = props.visualizations.find(v => v.id === props.visualizationId); + const visualization = props.visualizationMap[props.visualizationId || '']; if (!visualization) { return ( @@ -62,12 +115,22 @@ function VisualizationSummary(props: Props) { function ConfirmationModal({ onCancel, onConfirm, - visualizationLabel, + selection, + visualizationMap, + frame, }: { - visualizationLabel: string; + frame: FramePublicAPI; + selection: VisualizationSelection; + visualizationMap: Record; onCancel: () => void; onConfirm: () => void; }) { + const visualization = visualizationMap[selection.visualizationId]; + const dropAll = willLoseAllLayers(frame, visualization); + const { label: visualizationLabel } = visualization.visualizationTypes.find( + v => v.id === selection.subVisualizationId + )!; + return (

- {i18n.translate('xpack.lens.configPanel.dropLayersDescription', { - defaultMessage: 'Switching to {visualizationLabel} will drop all but the first layers.', - values: { visualizationLabel }, - })} + {dropAll + ? i18n.translate('xpack.lens.configPanel.dropAllLayersDescription', { + defaultMessage: 'Switching to {visualizationLabel} will drop all layers.', + values: { visualizationLabel }, + }) + : i18n.translate('xpack.lens.configPanel.dropAllButLAstLayersDescription', { + defaultMessage: + 'Switching to {visualizationLabel} will drop all but the last layers.', + values: { visualizationLabel }, + })}

{i18n.translate('xpack.lens.configPanel.dropLayersPrompt', { @@ -100,11 +170,6 @@ function ConfirmationModal({ ); } -function selectedVisualizationType(selection: VisualizationSelection, props: Props) { - const visualization = props.visualizations.find(v => v.id === selection.visualizationId); - return visualization!.visualizationTypes.find(v => v.id === selection.subVisualizationId)!; -} - export function ConfigPanelHeader(props: Props) { const [state, setState] = useState({ isFlyoutOpen: false }); @@ -121,33 +186,61 @@ export function ConfigPanelHeader(props: Props) { }); }; - const commitSelection = (selection: VisualizationSelection) => { + const commitSelection = ({ visualizationId, subVisualizationId }: VisualizationSelection) => { hideFlyout(true); - props.onChange(selection); + + const visualization = props.visualizationMap[visualizationId]; + const switchVisualizationType = visualization.switchVisualizationType; + + if (visualizationId !== props.visualizationId) { + const newState = getNewVisualizationState(props.framePublicAPI, visualization); + props.dispatch({ + type: 'SWITCH_VISUALIZATION', + newVisualizationId: visualizationId, + initialState: switchVisualizationType + ? switchVisualizationType(subVisualizationId, newState) + : newState, + }); + } else if (switchVisualizationType) { + props.dispatch({ + type: 'UPDATE_VISUALIZATION_STATE', + newState: switchVisualizationType(subVisualizationId, props.visualizationState), + }); + } }; const onSelect = (selection: VisualizationSelection) => { - if (props.requireConfirmation(selection.visualizationId)) { + const numLayers = Object.keys(props.framePublicAPI.datasourceLayers).length; + const shouldRequestConfirmation = + props.visualizationId !== selection.visualizationId && numLayers > 1; + + if (shouldRequestConfirmation) { requestConfirmation(selection); } else { commitSelection(selection); } }; - const visualizationTypes = flatten( - props.visualizations.map(v => - v.visualizationTypes.map(t => ({ - visualizationId: v.id, - ...t, - })) - ) + const visualizationTypes = useMemo( + () => + flatten( + Object.values(props.visualizationMap).map(v => + v.visualizationTypes.map(t => ({ + visualizationId: v.id, + ...t, + })) + ) + ), + [props.visualizationMap] ); return ( <> {state.confirmVisualization && ( setState({ ...state, confirmVisualization: undefined })} onConfirm={() => commitSelection(state.confirmVisualization!)} /> diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx index 12182c1d6e217..658673bc3071d 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx @@ -19,36 +19,6 @@ interface ConfigPanelWrapperProps { framePublicAPI: FramePublicAPI; } -function getSuggestedVisualizationState(frame: FramePublicAPI, visualization: Visualization) { - const [[layerId, datasource]] = Object.entries(frame.datasourceLayers); - const suggestions = visualization.getSuggestions({ - tables: [ - { - datasourceSuggestionId: 0, - isMultiRow: true, - columns: datasource.getTableSpec().map(col => ({ - ...col, - operation: datasource.getOperationForColumnId(col.columnId)!, - })), - layerId, - }, - ], - }); - - const suggestion = suggestions.length ? suggestions[0] : undefined; - - // Remove any layers that are not used by the new visualization. If we don't do this, - // we get orphaned objects, and weird edge cases such as prompting the user that - // layers are going to be dropped, when the user is unaware of any extraneous layers. - Object.keys(frame.datasourceLayers).forEach(id => { - if (!suggestion || id !== layerId) { - frame.removeLayer(id); - } - }); - - return visualization.initialize(frame, suggestion && suggestion.state); -} - export const ConfigPanelWrapper = memo(function ConfigPanelWrapper(props: ConfigPanelWrapperProps) { const context = useContext(DragContext); const setVisualizationState = useMemo( @@ -64,32 +34,12 @@ export const ConfigPanelWrapper = memo(function ConfigPanelWrapper(props: Config return ( <> - props.activeVisualizationId !== visualizationId && - Object.keys(props.framePublicAPI.datasourceLayers).length > 1 - } - onChange={({ visualizationId, subVisualizationId }) => { - const visualization = props.visualizationMap[visualizationId]; - const switchVisualizationType = visualization.switchVisualizationType; - - if (visualizationId !== props.activeVisualizationId) { - const newState = getSuggestedVisualizationState(props.framePublicAPI, visualization); - props.dispatch({ - type: 'SWITCH_VISUALIZATION', - newVisualizationId: visualizationId, - initialState: switchVisualizationType - ? switchVisualizationType(subVisualizationId, newState) - : newState, - }); - } else if (switchVisualizationType) { - setVisualizationState( - switchVisualizationType(subVisualizationId, props.visualizationState) - ); - } - }} + dispatch={props.dispatch} + framePublicAPI={props.framePublicAPI} /> {props.activeVisualizationId && props.visualizationState !== null && (

From f7a3ef4eeb53bf110a9f37355a7973413ef2640d Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 29 Jul 2019 15:53:40 +0200 Subject: [PATCH 08/16] remove duplicated imports --- .../lens/public/xy_visualization_plugin/xy_config_panel.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index a3182149ecc2e..9cc20f6439c58 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -20,8 +20,6 @@ import { EuiSwitch, } from '@elastic/eui'; import { State, SeriesType, LayerConfig, visualizationTypes } from './types'; -import { VisualizationProps } from '../types'; -import { State, SeriesType, LayerConfig } from './types'; import { VisualizationProps, OperationMetadata } from '../types'; import { NativeRenderer } from '../native_renderer'; import { MultiColumnEditor } from '../multi_column_editor'; From 87c143fd15cc740a56621fd810381370d5fc2b6c Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 29 Jul 2019 18:50:34 +0200 Subject: [PATCH 09/16] cleanup WIP --- .../editor_frame/config_panel_header.tsx | 19 ++++--------------- x-pack/legacy/plugins/lens/public/types.ts | 6 +++++- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx index a1d303ef2a469..39970a9f7750f 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx @@ -79,10 +79,6 @@ function dropUnusedLayers(frame: FramePublicAPI, suggestion: unknown, layerId?: }); } -function willLoseAllLayers(frame: FramePublicAPI, visualization: Visualization) { - return !getSuggestion(frame, visualization).layerId; -} - function getNewVisualizationState(frame: FramePublicAPI, visualization: Visualization) { const { suggestion, layerId } = getSuggestion(frame, visualization); @@ -126,7 +122,6 @@ function ConfirmationModal({ onConfirm: () => void; }) { const visualization = visualizationMap[selection.visualizationId]; - const dropAll = willLoseAllLayers(frame, visualization); const { label: visualizationLabel } = visualization.visualizationTypes.find( v => v.id === selection.subVisualizationId )!; @@ -149,16 +144,10 @@ function ConfirmationModal({ data-test-subj="lnsConfirmDropLayer" >

- {dropAll - ? i18n.translate('xpack.lens.configPanel.dropAllLayersDescription', { - defaultMessage: 'Switching to {visualizationLabel} will drop all layers.', - values: { visualizationLabel }, - }) - : i18n.translate('xpack.lens.configPanel.dropAllButLAstLayersDescription', { - defaultMessage: - 'Switching to {visualizationLabel} will drop all but the last layers.', - values: { visualizationLabel }, - })} + {i18n.translate('xpack.lens.configPanel.dropAllButLAstLayersDescription', { + defaultMessage: 'Switching to {visualizationLabel} will drop all but the last layers.', + values: { visualizationLabel }, + })}

{i18n.translate('xpack.lens.configPanel.dropLayersPrompt', { diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index 953aedb870ce9..a13ed55c9e66e 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -201,7 +201,11 @@ export interface Visualization { visualizationTypes: VisualizationType[]; - renderDescription: (domElement: Element, state: T) => void; + getDescription: (state: T) => { + icon?: EuiIconType | string; + label: string; + }; + switchVisualizationType?: (visualizationTypeId: string, state: T) => T; // For initializing from saved object From 2195b0e1105408480a3f67bee78b59054f684cd8 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 30 Jul 2019 09:47:46 +0200 Subject: [PATCH 10/16] cleanup #2 --- .../visualization.test.tsx | 8 ++-- .../visualization.tsx | 16 ++++---- .../editor_frame/config_panel_header.test.tsx | 9 ++--- .../editor_frame/config_panel_header.tsx | 18 ++++----- .../editor_frame/editor_frame.test.tsx | 2 +- .../editor_frame/editor_frame.tsx | 6 +-- .../editor_frame/save.test.ts | 2 +- .../lens/public/editor_frame_plugin/mocks.tsx | 6 +-- .../indexpattern_plugin/indexpattern.test.tsx | 38 ++++++++++++++++++- .../indexpattern_plugin/indexpattern.tsx | 7 +++- .../date_histogram.test.tsx | 1 - x-pack/legacy/plugins/lens/public/types.ts | 8 ++-- .../xy_config_panel.test.tsx | 2 +- .../xy_config_panel.tsx | 2 +- .../xy_visualization.tsx | 15 +++----- 15 files changed, 85 insertions(+), 55 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx index 65b52b8d7fa87..f94e46aa39ca7 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx @@ -20,7 +20,7 @@ jest.mock('../id_generator'); function mockFrame(): FramePublicAPI { return { addNewLayer: () => 'aaa', - removeLayer: () => {}, + removeLayers: () => {}, datasourceLayers: {}, }; } @@ -77,7 +77,7 @@ describe('Datatable Visualization', () => { dragDropContext={{ dragging: undefined, setDragging: () => {} }} frame={{ addNewLayer: jest.fn(), - removeLayer: jest.fn(), + removeLayers: jest.fn(), datasourceLayers: { a: datasource.publicAPIMock }, }} layer={layer} @@ -115,7 +115,7 @@ describe('Datatable Visualization', () => { dragDropContext={{ dragging: undefined, setDragging: () => {} }} frame={{ addNewLayer: jest.fn(), - removeLayer: jest.fn(), + removeLayers: jest.fn(), datasourceLayers: { a: datasource.publicAPIMock }, }} layer={layer} @@ -151,7 +151,7 @@ describe('Datatable Visualization', () => { dragDropContext={{ dragging: undefined, setDragging: () => {} }} frame={{ addNewLayer: jest.fn(), - removeLayer: jest.fn(), + removeLayers: jest.fn(), datasourceLayers: { a: datasource.publicAPIMock }, }} layer={layer} diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx index cd5308851f889..b81f3bc0a44f9 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx @@ -103,15 +103,13 @@ export const datatableVisualization: Visualization< }, ], - renderDescription(el) { - render( - <> - {i18n.translate('xpack.lens.datatable.label', { - defaultMessage: 'Datatable', - })} - , - el - ); + getDescription(state) { + return { + icon: 'empty', + label: i18n.translate('xpack.lens.datatable.label', { + defaultMessage: 'Datatable', + }), + }; }, switchVisualizationType: (_, state) => state, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx index 661c925590702..98adaf0988b41 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx @@ -166,10 +166,10 @@ describe('config_panel_header', () => { }); it('should remove unused layers', () => { - const removeLayer = jest.fn(); + const removeLayers = jest.fn(); const frame = { ...mockFrame(['a', 'b', 'c']), - removeLayer, + removeLayers, }; const component = mount( { switchTo('subvisB', component); confirm(component); - expect(removeLayer).toHaveBeenCalledTimes(2); - expect(removeLayer).toHaveBeenCalledWith('b'); - expect(removeLayer).toHaveBeenCalledWith('c'); + expect(removeLayers).toHaveBeenCalledTimes(1); + expect(removeLayers).toHaveBeenCalledWith(['b', 'c']); }); it('should not prompt for confirmation if the visualization is not changing', () => { diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx index 39970a9f7750f..ab9a4c00dfd70 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx @@ -17,7 +17,6 @@ import { } from '@elastic/eui'; import { flatten } from 'lodash'; import { i18n } from '@kbn/i18n'; -import { NativeRenderer } from '../../native_renderer'; import { Visualization, FramePublicAPI } from '../../types'; import { Action } from './state_management'; @@ -72,11 +71,10 @@ function dropUnusedLayers(frame: FramePublicAPI, suggestion: unknown, layerId?: // Remove any layers that are not used by the new visualization. If we don't do this, // we get orphaned objects, and weird edge cases such as prompting the user that // layers are going to be dropped, when the user is unaware of any extraneous layers. - Object.keys(frame.datasourceLayers).forEach(id => { - if (!suggestion || id !== layerId) { - frame.removeLayer(id); - } + const layerIds = Object.keys(frame.datasourceLayers).filter(id => { + return !suggestion || id !== layerId; }); + frame.removeLayers(layerIds); } function getNewVisualizationState(frame: FramePublicAPI, visualization: Visualization) { @@ -100,11 +98,13 @@ function VisualizationSummary(props: Props) { ); } + const description = visualization.getDescription(props.visualizationState); + return ( - + <> + {description.icon && } + {description.label} + ); } diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx index 73db4ba22b19d..d2022f4585b6b 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx @@ -243,7 +243,7 @@ describe('editor_frame', () => { expect(mockVisualization.initialize).toHaveBeenCalledWith({ datasourceLayers: {}, addNewLayer: expect.any(Function), - removeLayer: expect.any(Function), + removeLayers: expect.any(Function), }); }); diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index ae344bfc8ace1..5bd2cff5bb67e 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -107,10 +107,10 @@ export function EditorFrame(props: EditorFrameProps) { return newLayerId; }, - removeLayer: (layerId: string) => { - const newState = props.datasourceMap[state.activeDatasourceId!].removeLayer( + removeLayers: (layerIds: string[]) => { + const newState = props.datasourceMap[state.activeDatasourceId!].removeLayers( state.datasourceStates[state.activeDatasourceId!].state, - layerId + layerIds ); dispatch({ diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.test.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.test.ts index fe829c70cc063..67e5ef1f192ed 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.test.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.test.ts @@ -35,7 +35,7 @@ describe('save editor frame state', () => { activeDatasourceId: 'indexpattern', framePublicAPI: { addNewLayer: jest.fn(), - removeLayer: jest.fn(), + removeLayers: jest.fn(), datasourceLayers: { first: mockDatasource.publicAPIMock, }, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx index c1afd9168320f..fd0c7e40708d2 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx @@ -19,7 +19,7 @@ export function createMockVisualization(): jest.Mocked { label: 'TEST', }, ], - renderDescription: jest.fn(), + getDescription: jest.fn(_state => ({ label: '' })), switchVisualizationType: jest.fn((_, x) => x), getPersistableState: jest.fn(_state => _state), getSuggestions: jest.fn(_options => []), @@ -53,7 +53,7 @@ export function createMockDatasource(): DatasourceMock { renderDataPanel: jest.fn(), toExpression: jest.fn((_frame, _state) => null), insertLayer: jest.fn((_state, _newLayerId) => {}), - removeLayer: jest.fn((_state, _layerId) => {}), + removeLayers: jest.fn((_state, _layerIds) => {}), getLayers: jest.fn(_state => []), getMetaData: jest.fn(_state => ({ filterableIndexPatterns: [] })), @@ -69,7 +69,7 @@ export function createMockFramePublicAPI(): FrameMock { return { datasourceLayers: {}, addNewLayer: jest.fn(() => ''), - removeLayer: jest.fn(), + removeLayers: jest.fn(), }; } diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx index 9ea076b3795a0..e68ee8b1dfab5 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx @@ -780,7 +780,7 @@ describe('IndexPattern Data Source', () => { }); }); - describe('#removeLayer', () => { + describe('#removeLayers', () => { it('should remove a layer', () => { const state = { indexPatterns: expectedIndexPatterns, @@ -798,7 +798,41 @@ describe('IndexPattern Data Source', () => { }, currentIndexPatternId: '1', }; - expect(indexPatternDatasource.removeLayer(state, 'first')).toEqual({ + expect(indexPatternDatasource.removeLayers(state, ['first'])).toEqual({ + ...state, + layers: { + second: { + indexPatternId: '2', + columnOrder: [], + columns: {}, + }, + }, + }); + }); + + it('should remove multiple layers', () => { + const state = { + indexPatterns: expectedIndexPatterns, + layers: { + first: { + indexPatternId: '1', + columnOrder: [], + columns: {}, + }, + second: { + indexPatternId: '2', + columnOrder: [], + columns: {}, + }, + third: { + indexPatternId: '2', + columnOrder: [], + columns: {}, + }, + }, + currentIndexPatternId: '1', + }; + expect(indexPatternDatasource.removeLayers(state, ['first', 'third'])).toEqual({ ...state, layers: { second: { diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx index 913d51acc763d..e976570bb9e87 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx @@ -267,9 +267,12 @@ export function getIndexPatternDatasource({ }; }, - removeLayer(state: IndexPatternPrivateState, layerId: string) { + removeLayers(state: IndexPatternPrivateState, layerIds: string[]) { const newLayers = { ...state.layers }; - delete newLayers[layerId]; + layerIds.forEach(layerId => { + delete newLayers[layerId]; + }); + return { ...state, layers: newLayers, diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx index 0aff04c38f643..3a7d1814bb9fe 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx @@ -70,7 +70,6 @@ describe('date_histogram', () => { columnOrder: ['col2'], columns: { col2: { - operationId: 'op2', label: 'Value of timestamp', dataType: 'date', isBucketed: true, diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index a13ed55c9e66e..8b287423bb634 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -62,7 +62,7 @@ export interface Datasource { getPersistableState: (state: T) => P; insertLayer: (state: T, newLayerId: string) => T; - removeLayer: (state: T, layerId: string) => T; + removeLayers: (state: T, layerIds: string[]) => T; getLayers: (state: T) => string[]; renderDataPanel: (domElement: Element, props: DatasourceDataPanelProps) => void; @@ -187,7 +187,7 @@ export interface FramePublicAPI { datasourceLayers: Record; // Adds a new layer. This has a side effect of updating the datasource state addNewLayer: () => string; - removeLayer: (layerId: string) => void; + removeLayers: (layerIds: string[]) => void; } export interface VisualizationType { @@ -201,7 +201,9 @@ export interface Visualization { visualizationTypes: VisualizationType[]; - getDescription: (state: T) => { + getDescription: ( + state: T + ) => { icon?: EuiIconType | string; label: string; }; diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx index 1619b42f94c6b..2c23d69c6bf9d 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx @@ -315,7 +315,7 @@ describe('XYConfigPanel', () => { .first() .simulate('click'); - expect(frame.removeLayer).toHaveBeenCalled(); + expect(frame.removeLayers).toHaveBeenCalled(); expect(setState).toHaveBeenCalledTimes(1); expect(setState.mock.calls[0][0]).toMatchObject({ layers: [], diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index 9cc20f6439c58..65a9526cfc394 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -179,7 +179,7 @@ export function XYConfigPanel(props: VisualizationProps) { setState(updateLayer(state, { ...layer, seriesType }, index)) } removeLayer={() => { - frame.removeLayer(layer.layerId); + frame.removeLayers([layer.layerId]); setState({ ...state, layers: state.layers.filter(l => l !== layer) }); }} /> diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx index 0ebae6bd6ab8a..f0b3c1a79ed2b 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx @@ -10,7 +10,6 @@ import { render } from 'react-dom'; import { Position } from '@elastic/charts'; import { I18nProvider } from '@kbn/i18n/react'; import { i18n } from '@kbn/i18n'; -import { EuiIcon } from '@elastic/eui'; import { getSuggestions } from './xy_suggestions'; import { XYConfigPanel } from './xy_config_panel'; import { Visualization } from '../types'; @@ -54,16 +53,12 @@ export const xyVisualization: Visualization = { visualizationTypes, - renderDescription(el, state) { + getDescription(state) { const { icon, label } = getDescription(state); - - render( - <> - - {label} - , - el - ); + return { + icon: icon || defaultIcon, + label, + }; }, switchVisualizationType(seriesType: string, state: State) { From c519aed948dcd37a132b8b6b70b020879747db6c Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 30 Jul 2019 15:41:27 +0200 Subject: [PATCH 11/16] fix tests and add choose behavior --- .../editor_frame/config_panel_header.test.tsx | 146 +++++++-- .../editor_frame/config_panel_header.tsx | 288 ++++++++++++------ .../editor_frame/editor_frame.test.tsx | 3 +- .../editor_frame/editor_frame.tsx | 30 +- .../editor_frame/state_management.ts | 38 +++ .../lens/public/editor_frame_plugin/mocks.tsx | 2 +- .../indexpattern_plugin/indexpattern.test.tsx | 38 +-- .../indexpattern_plugin/indexpattern.tsx | 12 +- x-pack/legacy/plugins/lens/public/types.ts | 2 +- .../xy_visualization_plugin/xy_suggestions.ts | 9 +- 10 files changed, 390 insertions(+), 178 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx index 98adaf0988b41..3fb783512a445 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx @@ -9,7 +9,7 @@ import { createMockVisualization, createMockFramePublicAPI } from '../mocks'; import { mountWithIntl as mount } from 'test_utils/enzyme_helpers'; import { ReactWrapper } from 'enzyme'; import { ConfigPanelHeader } from './config_panel_header'; -import { Visualization } from '../../types'; +import { Visualization, FramePublicAPI, DatasourcePublicAPI } from '../../types'; describe('config_panel_header', () => { function generateVisualization(id: string): jest.Mocked { @@ -26,13 +26,13 @@ describe('config_panel_header', () => { initialize: jest.fn((_frame, state?: unknown) => { return state || `${id} initial state`; }), - getSuggestions: jest.fn(_options => { + getSuggestions: jest.fn(options => { return [ { score: 1, title: '', state: `suggestion ${id}`, - datasourceSuggestionId: 1, + datasourceSuggestionId: options.tables[0].datasourceSuggestionId, previewIcon: 'empty', }, ]; @@ -66,20 +66,20 @@ describe('config_panel_header', () => { return { ...createMockFramePublicAPI(), datasourceLayers: layers.reduce( - jest.fn((acc, layerId) => ({ + (acc, layerId) => ({ ...acc, - [layerId]: { - getTableSpec() { + [layerId]: ({ + getTableSpec: jest.fn(() => { return [{ columnId: 2 }]; - }, + }), getOperationForColumnId() { return {}; }, - }, - })), - {} + } as unknown) as DatasourcePublicAPI, + }), + {} as Record ), - }; + } as FramePublicAPI; } function showFlyout(component: ReactWrapper) { @@ -104,6 +104,13 @@ describe('config_panel_header', () => { .simulate('click'); } + function chooseOption(component: ReactWrapper, option: number) { + component + .find(`[data-test-subj="lnsLayerOption-${option}"]`) + .last() + .simulate('click'); + } + function deny(component: ReactWrapper) { component .find('[data-test-subj="confirmModalCancelButton"]') @@ -115,7 +122,11 @@ describe('config_panel_header', () => { return component.find('[data-test-subj="lnsConfirmDropLayer"]').length > 0; } - it('should not prompt for confirmation if there is only one layer', () => { + function isChooseModalVisible(component: ReactWrapper) { + return component.find('[data-test-subj="lnsChooseOption"]').length > 0; + } + + it('should not prompt for confirmation if there is a suggestion from the target visualization', () => { const dispatch = jest.fn(); const visualizations = mockVisualizations(); const component = mount( @@ -137,15 +148,43 @@ describe('config_panel_header', () => { }); }); - it('should prompt for confirmation if there is more than one layer', () => { + it('should not prompt for confirmation if there is no data in the layers', () => { const dispatch = jest.fn(); + const visualizations = mockVisualizations(); + visualizations.visB.getSuggestions.mockReturnValueOnce([]); + const frame = mockFrame(['a']); + (frame.datasourceLayers.a.getTableSpec as jest.Mock).mockReturnValue([]); + const component = mount( + ); + + switchTo('subvisB', component); + + expect(dispatch).toHaveBeenCalledWith({ + initialState: 'visB initial state', + newVisualizationId: 'visB', + type: 'SWITCH_VISUALIZATION', + }); + }); + + it('should prompt for confirmation if there is no suggestion from the target visualization', () => { + const dispatch = jest.fn(); + const visualizations = mockVisualizations(); + visualizations.visB.getSuggestions.mockReturnValueOnce([]); + const component = mount( + ); @@ -159,7 +198,53 @@ describe('config_panel_header', () => { expect(isModalVisible(component)).toBeFalsy(); expect(dispatch).toHaveBeenCalledWith({ - initialState: 'suggestion visB', + initialState: 'visB initial state', + newVisualizationId: 'visB', + type: 'SWITCH_VISUALIZATION', + }); + }); + + it('should prompt for confirmation if there is more than one layer', () => { + const dispatch = jest.fn(); + const visualizations = mockVisualizations(); + visualizations.visB.getSuggestions.mockReturnValue([ + { + score: 1, + title: '', + state: `suggestion visb 1`, + datasourceSuggestionId: 0, + previewIcon: 'empty', + }, + { + score: 1, + title: '', + state: `suggestion visb 2`, + datasourceSuggestionId: 1, + previewIcon: 'empty', + }, + ]); + + const component = mount( + + ); + + switchTo('subvisB', component); + + expect(dispatch).not.toHaveBeenCalled(); + + expect(isChooseModalVisible(component)).toBeTruthy(); + chooseOption(component, 1); + + expect(isChooseModalVisible(component)).toBeFalsy(); + + expect(dispatch).toHaveBeenCalledWith({ + initialState: 'suggestion visb 2', newVisualizationId: 'visB', type: 'SWITCH_VISUALIZATION', }); @@ -182,7 +267,7 @@ describe('config_panel_header', () => { ); switchTo('subvisB', component); - confirm(component); + chooseOption(component, 0); expect(removeLayers).toHaveBeenCalledTimes(1); expect(removeLayers).toHaveBeenCalledWith(['b', 'c']); @@ -244,11 +329,13 @@ describe('config_panel_header', () => { it('should not process the change, if cancelled', () => { const dispatch = jest.fn(); + const visualizations = mockVisualizations(); + visualizations.visB.getSuggestions.mockReturnValueOnce([]); const component = mount( @@ -265,6 +352,29 @@ describe('config_panel_header', () => { expect(dispatch).not.toHaveBeenCalled(); }); + it('should not process the change if there are options, if cancelled', () => { + const dispatch = jest.fn(); + const component = mount( + + ); + + switchTo('subvisB', component); + + expect(isChooseModalVisible(component)).toBeTruthy(); + expect(dispatch).not.toHaveBeenCalled(); + + deny(component); + + expect(isChooseModalVisible(component)).toBeFalsy(); + expect(dispatch).not.toHaveBeenCalled(); + }); + it('should show all visualization types', () => { const component = mount( unknown; + title: string; + icon: string; + keptLayerId: string; + }>; + dataLoss: 'nothing' | 'layers' | 'everything'; } interface State { @@ -38,53 +53,16 @@ interface Props { framePublicAPI: FramePublicAPI; } -function getSuggestion(frame: FramePublicAPI, visualization: Visualization) { - const layers = Object.entries(frame.datasourceLayers); - - if (!layers.length) { - return { - suggestion: undefined, - layerId: undefined, - }; - } - - const [[layerId, datasource]] = layers; - const suggestions = visualization.getSuggestions({ - tables: [ - { - datasourceSuggestionId: 0, - isMultiRow: true, - columns: datasource.getTableSpec().map(col => ({ - ...col, - operation: datasource.getOperationForColumnId(col.columnId)!, - })), - layerId, - }, - ], - }); - - const suggestion = suggestions.length ? suggestions[0] : undefined; - return { layerId, suggestion }; -} - -function dropUnusedLayers(frame: FramePublicAPI, suggestion: unknown, layerId?: string) { +function dropUnusedLayers(frame: FramePublicAPI, layerId: string) { // Remove any layers that are not used by the new visualization. If we don't do this, // we get orphaned objects, and weird edge cases such as prompting the user that // layers are going to be dropped, when the user is unaware of any extraneous layers. const layerIds = Object.keys(frame.datasourceLayers).filter(id => { - return !suggestion || id !== layerId; + return id !== layerId; }); frame.removeLayers(layerIds); } -function getNewVisualizationState(frame: FramePublicAPI, visualization: Visualization) { - const { suggestion, layerId } = getSuggestion(frame, visualization); - - dropUnusedLayers(frame, suggestion, layerId); - - return visualization.initialize(frame, suggestion && suggestion.state); -} - function VisualizationSummary(props: Props) { const visualization = props.visualizationMap[props.visualizationId || '']; @@ -119,44 +97,89 @@ function ConfirmationModal({ selection: VisualizationSelection; visualizationMap: Record; onCancel: () => void; - onConfirm: () => void; + onConfirm: (optionIndex: number) => void; }) { const visualization = visualizationMap[selection.visualizationId]; const { label: visualizationLabel } = visualization.visualizationTypes.find( v => v.id === selection.subVisualizationId )!; - return ( - - -

- {i18n.translate('xpack.lens.configPanel.dropAllButLAstLayersDescription', { - defaultMessage: 'Switching to {visualizationLabel} will drop all but the last layers.', - values: { visualizationLabel }, + if (selection.dataLoss === 'everything') { + return ( + + -

- {i18n.translate('xpack.lens.configPanel.dropLayersPrompt', { - defaultMessage: 'Are you sure you want to drop layers?', + onCancel={onCancel} + onConfirm={() => onConfirm(0)} + cancelButtonText={i18n.translate('xpack.lens.configPanel.dropLayersCancelLabel', { + defaultMessage: `No, keep current chart`, })} -

- - - ); + confirmButtonText={i18n.translate('xpack.lens.configPanel.dropLayersConfirmLabel', { + defaultMessage: `Yes, start over`, + })} + defaultFocusedButton="cancel" + data-test-subj="lnsConfirmDropLayer" + > +

+ {i18n.translate('xpack.lens.configPanel.dropAllLayersDescription', { + defaultMessage: + '{visualizationLabel} cannot use your current data and you will have to start over.', + values: { visualizationLabel }, + })} +

+

+ {i18n.translate('xpack.lens.configPanel.dropLayersPrompt', { + defaultMessage: 'Are you sure you want to switch charts?', + })} +

+ + + ); + } else { + return ( + + + + + {i18n.translate('xpack.lens.configPanel.lossySwitchLabel', { + defaultMessage: 'Data loss due to chart switching', + })} + + + +

+ {i18n.translate('xpack.lens.configPanel.dropAllButOneLayerDescription', { + defaultMessage: + 'By switching to {visualizationLabel} you will loose a part of your configuration. Please pick your preferred chart', + values: { visualizationLabel }, + })} +

+ + {selection.options.map((option, index) => ( + onConfirm(index)} + /> + ))} + +
+ + + {i18n.translate('xpack.lens.configPanel.dropAllButOneLayerDescriptionCancelLabel', { + defaultMessage: `Keep current chart`, + })} + + +
+
+ ); + } } export function ConfigPanelHeader(props: Props) { @@ -175,38 +198,122 @@ export function ConfigPanelHeader(props: Props) { }); }; - const commitSelection = ({ visualizationId, subVisualizationId }: VisualizationSelection) => { + const commitSelection = (selection: VisualizationSelection, optionIndex: number) => { hideFlyout(true); - const visualization = props.visualizationMap[visualizationId]; - const switchVisualizationType = visualization.switchVisualizationType; + if (selection.dataLoss === 'everything' || selection.dataLoss === 'layers') { + dropUnusedLayers(props.framePublicAPI, selection.options[optionIndex].keptLayerId); + } - if (visualizationId !== props.visualizationId) { - const newState = getNewVisualizationState(props.framePublicAPI, visualization); + if (selection.visualizationId !== props.visualizationId) { props.dispatch({ type: 'SWITCH_VISUALIZATION', - newVisualizationId: visualizationId, - initialState: switchVisualizationType - ? switchVisualizationType(subVisualizationId, newState) - : newState, + newVisualizationId: selection.visualizationId, + initialState: selection.options[optionIndex].getVisualizationState(), }); - } else if (switchVisualizationType) { + } else { props.dispatch({ type: 'UPDATE_VISUALIZATION_STATE', - newState: switchVisualizationType(subVisualizationId, props.visualizationState), + newState: selection.options[optionIndex].getVisualizationState(), }); } }; - const onSelect = (selection: VisualizationSelection) => { - const numLayers = Object.keys(props.framePublicAPI.datasourceLayers).length; - const shouldRequestConfirmation = - props.visualizationId !== selection.visualizationId && numLayers > 1; + function getSelection( + visualizationId: string, + subVisualizationId: string + ): VisualizationSelection { + const newVisualization = props.visualizationMap[visualizationId]; + const switchVisType = + props.visualizationMap[visualizationId].switchVisualizationType || + ((_type: string, initialState: unknown) => initialState); + if (props.visualizationId === visualizationId) { + return { + visualizationId, + subVisualizationId, + dataLoss: 'nothing', + options: [ + { + keptLayerId: '', + getVisualizationState: () => + switchVisType(subVisualizationId, props.visualizationState), + title: '', + icon: '', + }, + ], + }; + } + + const layers = Object.entries(props.framePublicAPI.datasourceLayers); + const containsData = layers.some( + ([_layerId, datasource]) => datasource.getTableSpec().length > 0 + ); + + // get ranked suggestions for all layers of current state + const suggestions = newVisualization + .getSuggestions({ + tables: layers.map(([layerId, datasource], index) => ({ + datasourceSuggestionId: index, + isMultiRow: true, + columns: datasource.getTableSpec().map(col => ({ + ...col, + operation: datasource.getOperationForColumnId(col.columnId)!, + })), + layerId, + })), + }) + .map(suggestion => ({ suggestion, layerId: layers[suggestion.datasourceSuggestionId][0] })) + .sort( + ({ suggestion: { score: scoreA } }, { suggestion: { score: scoreB } }) => scoreB - scoreA + ); + + let dataLoss: VisualizationSelection['dataLoss'] = 'nothing'; + + if (!suggestions.length && containsData) { + dataLoss = 'everything'; + } else if (layers.length > 1 && containsData) { + dataLoss = 'layers'; + } + + return { + visualizationId, + subVisualizationId, + dataLoss, + options: suggestions.length + ? suggestions.map(suggestion => ({ + getVisualizationState: () => + switchVisType( + subVisualizationId, + newVisualization.initialize(props.framePublicAPI, suggestion.suggestion.state) + ), + title: suggestion.suggestion.title, + icon: suggestion.suggestion.previewIcon, + keptLayerId: suggestion.layerId, + })) + : [ + { + getVisualizationState: () => { + return switchVisType( + subVisualizationId, + newVisualization.initialize(props.framePublicAPI) + ); + }, + icon: '', + keptLayerId: '', + title: '', + }, + ], + }; + } + + const onSelect = (visualizationId: string, subVisualizationId: string) => { + const selection = getSelection(visualizationId, subVisualizationId); + const shouldRequestConfirmation = selection.dataLoss !== 'nothing'; if (shouldRequestConfirmation) { requestConfirmation(selection); } else { - commitSelection(selection); + commitSelection(selection, 0); } }; @@ -231,7 +338,9 @@ export function ConfigPanelHeader(props: Props) { selection={state.confirmVisualization} visualizationMap={props.visualizationMap} onCancel={() => setState({ ...state, confirmVisualization: undefined })} - onConfirm={() => commitSelection(state.confirmVisualization!)} + onConfirm={(optionIndex: number) => + commitSelection(state.confirmVisualization!, optionIndex) + } /> )} {v.label}} role="menuitem" data-test-subj={`lnsConfigPanelHeaderPopover_${v.id}`} - onClick={() => - onSelect({ - visualizationId: v.visualizationId, - subVisualizationId: v.id, - }) - } + onClick={() => onSelect(v.visualizationId, v.id)} > diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx index d2022f4585b6b..424371d308da3 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx @@ -936,8 +936,7 @@ describe('editor_frame', () => { expect(mockDatasource.publicAPIMock.getTableSpec).toHaveBeenCalled(); expect(mockVisualization2.getSuggestions).toHaveBeenCalled(); expect(mockVisualization2.initialize).toHaveBeenCalledWith( - expect.objectContaining({ datasourceLayers: { first: mockDatasource.publicAPIMock } }), - undefined + expect.objectContaining({ datasourceLayers: { first: mockDatasource.publicAPIMock } }) ); expect(mockVisualization2.renderConfigPanel).toHaveBeenCalledWith( expect.any(Element), diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index 5bd2cff5bb67e..3ebf489e15474 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -94,29 +94,27 @@ export function EditorFrame(props: EditorFrameProps) { addNewLayer() { const newLayerId = generateId(); - const newState = props.datasourceMap[state.activeDatasourceId!].insertLayer( - state.datasourceStates[state.activeDatasourceId!].state, - newLayerId - ); - dispatch({ - type: 'UPDATE_DATASOURCE_STATE', + type: 'INSERT_LAYER', datasourceId: state.activeDatasourceId!, - newState, + layerId: newLayerId, + reducer: props.datasourceMap[state.activeDatasourceId!].insertLayer, }); return newLayerId; }, removeLayers: (layerIds: string[]) => { - const newState = props.datasourceMap[state.activeDatasourceId!].removeLayers( - state.datasourceStates[state.activeDatasourceId!].state, - layerIds - ); - - dispatch({ - type: 'UPDATE_DATASOURCE_STATE', - datasourceId: state.activeDatasourceId!, - newState, + layerIds.forEach(layerId => { + const layerDatasourceId = Object.entries(props.datasourceMap).find( + ([datasourceId, datasource]) => + datasource.getLayers(state.datasourceStates[datasourceId].state) + )![0]; + dispatch({ + type: 'REMOVE_LAYER', + layerId, + datasourceId: layerDatasourceId, + reducer: props.datasourceMap[layerDatasourceId].removeLayer, + }); }); }, }; diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts index 004bbc45194a1..0cf7334e1d8d5 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts @@ -46,6 +46,18 @@ export type Action = type: 'UPDATE_VISUALIZATION_STATE'; newState: unknown; } + | { + type: 'REMOVE_LAYER'; + layerId: string; + datasourceId: string; + reducer: (state: unknown, layerId: string) => unknown; + } + | { + type: 'INSERT_LAYER'; + layerId: string; + datasourceId: string; + reducer: (state: unknown, layerId: string) => unknown; + } | { type: 'VISUALIZATION_LOADED'; doc: Document; @@ -97,6 +109,32 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta return { ...state, persistedId: action.id }; case 'UPDATE_TITLE': return { ...state, title: action.title }; + case 'INSERT_LAYER': + return { + ...state, + datasourceStates: { + ...state.datasourceStates, + [action.datasourceId]: { + ...state.datasourceStates[action.datasourceId], + state: action.reducer( + state.datasourceStates[action.datasourceId].state, + action.layerId + ), + }, + }, + }; + case 'REMOVE_LAYER': + const layerDatasourceId = action.datasourceId; + return { + ...state, + datasourceStates: { + ...state.datasourceStates, + [layerDatasourceId]: { + ...state.datasourceStates[layerDatasourceId], + state: action.reducer(state.datasourceStates[layerDatasourceId].state, action.layerId), + }, + }, + }; case 'VISUALIZATION_LOADED': return { ...state, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx index fd0c7e40708d2..cdd4f1bf07898 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx @@ -53,7 +53,7 @@ export function createMockDatasource(): DatasourceMock { renderDataPanel: jest.fn(), toExpression: jest.fn((_frame, _state) => null), insertLayer: jest.fn((_state, _newLayerId) => {}), - removeLayers: jest.fn((_state, _layerIds) => {}), + removeLayer: jest.fn((_state, _layerId) => {}), getLayers: jest.fn(_state => []), getMetaData: jest.fn(_state => ({ filterableIndexPatterns: [] })), diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx index e68ee8b1dfab5..9ea076b3795a0 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx @@ -780,7 +780,7 @@ describe('IndexPattern Data Source', () => { }); }); - describe('#removeLayers', () => { + describe('#removeLayer', () => { it('should remove a layer', () => { const state = { indexPatterns: expectedIndexPatterns, @@ -798,41 +798,7 @@ describe('IndexPattern Data Source', () => { }, currentIndexPatternId: '1', }; - expect(indexPatternDatasource.removeLayers(state, ['first'])).toEqual({ - ...state, - layers: { - second: { - indexPatternId: '2', - columnOrder: [], - columns: {}, - }, - }, - }); - }); - - it('should remove multiple layers', () => { - const state = { - indexPatterns: expectedIndexPatterns, - layers: { - first: { - indexPatternId: '1', - columnOrder: [], - columns: {}, - }, - second: { - indexPatternId: '2', - columnOrder: [], - columns: {}, - }, - third: { - indexPatternId: '2', - columnOrder: [], - columns: {}, - }, - }, - currentIndexPatternId: '1', - }; - expect(indexPatternDatasource.removeLayers(state, ['first', 'third'])).toEqual({ + expect(indexPatternDatasource.removeLayer(state, 'first')).toEqual({ ...state, layers: { second: { diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx index e976570bb9e87..9f5c15e550e18 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx @@ -267,11 +267,9 @@ export function getIndexPatternDatasource({ }; }, - removeLayers(state: IndexPatternPrivateState, layerIds: string[]) { + removeLayer(state: IndexPatternPrivateState, layerId: string) { const newLayers = { ...state.layers }; - layerIds.forEach(layerId => { - delete newLayers[layerId]; - }); + delete newLayers[layerId]; return { ...state, @@ -311,11 +309,9 @@ export function getIndexPatternDatasource({ return state.layers[layerId].columnOrder.map(colId => ({ columnId: colId })); }, getOperationForColumnId: (columnId: string) => { - const layer = Object.values(state.layers).find(l => - l.columnOrder.find(id => id === columnId) - ); + const layer = state.layers[layerId]; - if (layer) { + if (layer && layer.columns[columnId]) { return columnToOperation(layer.columns[columnId]); } return null; diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index 8b287423bb634..387acd6612810 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -62,7 +62,7 @@ export interface Datasource { getPersistableState: (state: T) => P; insertLayer: (state: T, newLayerId: string) => T; - removeLayers: (state: T, layerIds: string[]) => T; + removeLayer: (state: T, layerId: string) => T; getLayers: (state: T) => string[]; renderDataPanel: (domElement: Element, props: DatasourceDataPanelProps) => void; diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts index cff0583391c12..0140785e16b8d 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts @@ -42,19 +42,20 @@ export function getSuggestions( columns.some(col => col.operation.dataType === 'number') && !columns.some(col => !columnSortOrder.hasOwnProperty(col.operation.dataType)) ) - .map(table => getSuggestionForColumns(table, opts.state)); + .map(table => getSuggestionForColumns(table, opts.state)) + .filter((suggestion): suggestion is VisualizationSuggestion => suggestion !== undefined); } function getSuggestionForColumns( table: TableSuggestion, currentState?: State -): VisualizationSuggestion { +): VisualizationSuggestion | undefined { const [buckets, values] = partition( prioritizeColumns(table.columns), col => col.operation.isBucketed ); - if (buckets.length >= 1) { + if (buckets.length === 1 || buckets.length === 2) { const [x, splitBy] = buckets; return getSuggestion( table.datasourceSuggestionId, @@ -64,7 +65,7 @@ function getSuggestionForColumns( splitBy, currentState ); - } else { + } else if (buckets.length === 0) { const [x, ...yValues] = values; return getSuggestion( table.datasourceSuggestionId, From 2f5fdec3b99d961b765ba95a56a78cf0e61e0c73 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 30 Jul 2019 18:05:33 +0200 Subject: [PATCH 12/16] change user flow --- .../editor_frame/config_panel_header.test.tsx | 188 +++++-------- .../editor_frame/config_panel_header.tsx | 262 ++++-------------- .../editor_frame/editor_frame.test.tsx | 31 +++ .../editor_frame/editor_frame.tsx | 7 +- .../editor_frame/state_management.test.ts | 30 ++ .../editor_frame/state_management.ts | 22 +- 6 files changed, 192 insertions(+), 348 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx index 3fb783512a445..d7e7a73d2941a 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx @@ -10,6 +10,7 @@ import { mountWithIntl as mount } from 'test_utils/enzyme_helpers'; import { ReactWrapper } from 'enzyme'; import { ConfigPanelHeader } from './config_panel_header'; import { Visualization, FramePublicAPI, DatasourcePublicAPI } from '../../types'; +import { EuiKeyPadMenuItemButton } from '@elastic/eui'; describe('config_panel_header', () => { function generateVisualization(id: string): jest.Mocked { @@ -97,36 +98,15 @@ describe('config_panel_header', () => { .simulate('click'); } - function confirm(component: ReactWrapper) { - component - .find('[data-test-subj="confirmModalConfirmButton"]') - .first() - .simulate('click'); - } - - function chooseOption(component: ReactWrapper, option: number) { - component - .find(`[data-test-subj="lnsLayerOption-${option}"]`) - .last() - .simulate('click'); - } - - function deny(component: ReactWrapper) { - component - .find('[data-test-subj="confirmModalCancelButton"]') - .first() - .simulate('click'); - } - - function isModalVisible(component: ReactWrapper) { - return component.find('[data-test-subj="lnsConfirmDropLayer"]').length > 0; - } - - function isChooseModalVisible(component: ReactWrapper) { - return component.find('[data-test-subj="lnsChooseOption"]').length > 0; + function getMenuItem(subType: string, component: ReactWrapper) { + showFlyout(component); + return component + .find(EuiKeyPadMenuItemButton) + .find(`[data-test-subj="lnsConfigPanelHeaderPopover_${subType}"]`) + .first(); } - it('should not prompt for confirmation if there is a suggestion from the target visualization', () => { + it('should use suggested state if there is a suggestion from the target visualization', () => { const dispatch = jest.fn(); const visualizations = mockVisualizations(); const component = mount( @@ -148,7 +128,7 @@ describe('config_panel_header', () => { }); }); - it('should not prompt for confirmation if there is no data in the layers', () => { + it('should use initial state if there is no suggestion from the target visualization', () => { const dispatch = jest.fn(); const visualizations = mockVisualizations(); visualizations.visB.getSuggestions.mockReturnValueOnce([]); @@ -174,55 +154,49 @@ describe('config_panel_header', () => { }); }); - it('should prompt for confirmation if there is no suggestion from the target visualization', () => { + it('should indicate data loss if not all layers will be used', () => { const dispatch = jest.fn(); const visualizations = mockVisualizations(); - visualizations.visB.getSuggestions.mockReturnValueOnce([]); + const frame = mockFrame(['a', 'b']); + const component = mount( ); - switchTo('subvisB', component); - - expect(dispatch).not.toHaveBeenCalled(); + expect(getMenuItem('subvisB', component).prop('betaBadgeIconType')).toEqual('bolt'); + }); - expect(isModalVisible(component)).toBeTruthy(); - confirm(component); + it('should indicate data loss if no data will be used', () => { + const dispatch = jest.fn(); + const visualizations = mockVisualizations(); + visualizations.visB.getSuggestions.mockReturnValueOnce([]); + const frame = mockFrame(['a']); - expect(isModalVisible(component)).toBeFalsy(); + const component = mount( + + ); - expect(dispatch).toHaveBeenCalledWith({ - initialState: 'visB initial state', - newVisualizationId: 'visB', - type: 'SWITCH_VISUALIZATION', - }); + expect(getMenuItem('subvisB', component).prop('betaBadgeIconType')).toEqual('bolt'); }); - it('should prompt for confirmation if there is more than one layer', () => { + it('should not indicate data loss if there is no data', () => { const dispatch = jest.fn(); const visualizations = mockVisualizations(); - visualizations.visB.getSuggestions.mockReturnValue([ - { - score: 1, - title: '', - state: `suggestion visb 1`, - datasourceSuggestionId: 0, - previewIcon: 'empty', - }, - { - score: 1, - title: '', - state: `suggestion visb 2`, - datasourceSuggestionId: 1, - previewIcon: 'empty', - }, - ]); + visualizations.visB.getSuggestions.mockReturnValueOnce([]); + const frame = mockFrame(['a']); + (frame.datasourceLayers.a.getTableSpec as jest.Mock).mockReturnValue([]); const component = mount( { visualizationState={{}} visualizationMap={visualizations} dispatch={dispatch} - framePublicAPI={mockFrame(['a', 'b'])} + framePublicAPI={frame} /> ); - switchTo('subvisB', component); + expect(getMenuItem('subvisB', component).prop('betaBadgeIconType')).toBeUndefined(); + }); - expect(dispatch).not.toHaveBeenCalled(); + it('should not indicate data loss if visualization is not changed', () => { + const dispatch = jest.fn(); + const removeLayers = jest.fn(); + const frame = { + ...mockFrame(['a', 'b', 'c']), + removeLayers, + }; + const visualizations = mockVisualizations(); + const switchVisualizationType = jest.fn(() => 'therebedragons'); - expect(isChooseModalVisible(component)).toBeTruthy(); - chooseOption(component, 1); + visualizations.visC.switchVisualizationType = switchVisualizationType; - expect(isChooseModalVisible(component)).toBeFalsy(); + const component = mount( + + ); - expect(dispatch).toHaveBeenCalledWith({ - initialState: 'suggestion visb 2', - newVisualizationId: 'visB', - type: 'SWITCH_VISUALIZATION', - }); + expect(getMenuItem('subvisC2', component).prop('betaBadgeIconType')).toBeUndefined(); }); it('should remove unused layers', () => { @@ -267,14 +253,18 @@ describe('config_panel_header', () => { ); switchTo('subvisB', component); - chooseOption(component, 0); expect(removeLayers).toHaveBeenCalledTimes(1); expect(removeLayers).toHaveBeenCalledWith(['b', 'c']); }); - it('should not prompt for confirmation if the visualization is not changing', () => { + it('should not remove layers if the visualization is not changing', () => { const dispatch = jest.fn(); + const removeLayers = jest.fn(); + const frame = { + ...mockFrame(['a', 'b', 'c']), + removeLayers, + }; const visualizations = mockVisualizations(); const switchVisualizationType = jest.fn(() => 'therebedragons'); @@ -286,12 +276,12 @@ describe('config_panel_header', () => { visualizationState={'therebegriffins'} visualizationMap={visualizations} dispatch={dispatch} - framePublicAPI={mockFrame(['a', 'b'])} + framePublicAPI={frame} /> ); switchTo('subvisC2', component); - expect(isModalVisible(component)).toBeFalsy(); + expect(removeLayers).not.toHaveBeenCalled(); expect(switchVisualizationType).toHaveBeenCalledWith('subvisC2', 'therebegriffins'); expect(dispatch).toHaveBeenCalledWith({ type: 'UPDATE_VISUALIZATION_STATE', @@ -327,54 +317,6 @@ describe('config_panel_header', () => { }); }); - it('should not process the change, if cancelled', () => { - const dispatch = jest.fn(); - const visualizations = mockVisualizations(); - visualizations.visB.getSuggestions.mockReturnValueOnce([]); - const component = mount( - - ); - - switchTo('subvisB', component); - - expect(isModalVisible(component)).toBeTruthy(); - expect(dispatch).not.toHaveBeenCalled(); - - deny(component); - - expect(isModalVisible(component)).toBeFalsy(); - expect(dispatch).not.toHaveBeenCalled(); - }); - - it('should not process the change if there are options, if cancelled', () => { - const dispatch = jest.fn(); - const component = mount( - - ); - - switchTo('subvisB', component); - - expect(isChooseModalVisible(component)).toBeTruthy(); - expect(dispatch).not.toHaveBeenCalled(); - - deny(component); - - expect(isChooseModalVisible(component)).toBeFalsy(); - expect(dispatch).not.toHaveBeenCalled(); - }); - it('should show all visualization types', () => { const component = mount( unknown; - title: string; - icon: string; - keptLayerId: string; - }>; + getVisualizationState: () => unknown; + keptLayerId: string; dataLoss: 'nothing' | 'layers' | 'everything'; } -interface State { - isFlyoutOpen: boolean; - confirmVisualization?: VisualizationSelection; -} - interface Props { dispatch: (action: Action) => void; visualizationMap: Record; @@ -86,135 +67,26 @@ function VisualizationSummary(props: Props) { ); } -function ConfirmationModal({ - onCancel, - onConfirm, - selection, - visualizationMap, - frame, -}: { - frame: FramePublicAPI; - selection: VisualizationSelection; - visualizationMap: Record; - onCancel: () => void; - onConfirm: (optionIndex: number) => void; -}) { - const visualization = visualizationMap[selection.visualizationId]; - const { label: visualizationLabel } = visualization.visualizationTypes.find( - v => v.id === selection.subVisualizationId - )!; - - if (selection.dataLoss === 'everything') { - return ( - - onConfirm(0)} - cancelButtonText={i18n.translate('xpack.lens.configPanel.dropLayersCancelLabel', { - defaultMessage: `No, keep current chart`, - })} - confirmButtonText={i18n.translate('xpack.lens.configPanel.dropLayersConfirmLabel', { - defaultMessage: `Yes, start over`, - })} - defaultFocusedButton="cancel" - data-test-subj="lnsConfirmDropLayer" - > -

- {i18n.translate('xpack.lens.configPanel.dropAllLayersDescription', { - defaultMessage: - '{visualizationLabel} cannot use your current data and you will have to start over.', - values: { visualizationLabel }, - })} -

-

- {i18n.translate('xpack.lens.configPanel.dropLayersPrompt', { - defaultMessage: 'Are you sure you want to switch charts?', - })} -

-
-
- ); - } else { - return ( - - - - - {i18n.translate('xpack.lens.configPanel.lossySwitchLabel', { - defaultMessage: 'Data loss due to chart switching', - })} - - - -

- {i18n.translate('xpack.lens.configPanel.dropAllButOneLayerDescription', { - defaultMessage: - 'By switching to {visualizationLabel} you will loose a part of your configuration. Please pick your preferred chart', - values: { visualizationLabel }, - })} -

- - {selection.options.map((option, index) => ( - onConfirm(index)} - /> - ))} - -
- - - {i18n.translate('xpack.lens.configPanel.dropAllButOneLayerDescriptionCancelLabel', { - defaultMessage: `Keep current chart`, - })} - - -
-
- ); - } -} - export function ConfigPanelHeader(props: Props) { - const [state, setState] = useState({ isFlyoutOpen: false }); + const [flyoutOpen, setFlyoutOpen] = useState(false); - const hideFlyout = (force: boolean = false) => { - if (force || !state.confirmVisualization) { - setState({ isFlyoutOpen: false }); - } - }; - - const requestConfirmation = (confirmVisualization: VisualizationSelection) => { - setState({ - ...state, - confirmVisualization, - }); - }; - - const commitSelection = (selection: VisualizationSelection, optionIndex: number) => { - hideFlyout(true); + const commitSelection = (selection: VisualizationSelection) => { + setFlyoutOpen(false); if (selection.dataLoss === 'everything' || selection.dataLoss === 'layers') { - dropUnusedLayers(props.framePublicAPI, selection.options[optionIndex].keptLayerId); + dropUnusedLayers(props.framePublicAPI, selection.keptLayerId); } if (selection.visualizationId !== props.visualizationId) { props.dispatch({ type: 'SWITCH_VISUALIZATION', newVisualizationId: selection.visualizationId, - initialState: selection.options[optionIndex].getVisualizationState(), + initialState: selection.getVisualizationState(), }); } else { props.dispatch({ type: 'UPDATE_VISUALIZATION_STATE', - newState: selection.options[optionIndex].getVisualizationState(), + newState: selection.getVisualizationState(), }); } }; @@ -232,15 +104,8 @@ export function ConfigPanelHeader(props: Props) { visualizationId, subVisualizationId, dataLoss: 'nothing', - options: [ - { - keptLayerId: '', - getVisualizationState: () => - switchVisType(subVisualizationId, props.visualizationState), - title: '', - icon: '', - }, - ], + keptLayerId: '', + getVisualizationState: () => switchVisType(subVisualizationId, props.visualizationState), }; } @@ -249,8 +114,8 @@ export function ConfigPanelHeader(props: Props) { ([_layerId, datasource]) => datasource.getTableSpec().length > 0 ); - // get ranked suggestions for all layers of current state - const suggestions = newVisualization + // get top ranked suggestion for all layers of current state + const topSuggestion = newVisualization .getSuggestions({ tables: layers.map(([layerId, datasource], index) => ({ datasourceSuggestionId: index, @@ -265,11 +130,11 @@ export function ConfigPanelHeader(props: Props) { .map(suggestion => ({ suggestion, layerId: layers[suggestion.datasourceSuggestionId][0] })) .sort( ({ suggestion: { score: scoreA } }, { suggestion: { score: scoreB } }) => scoreB - scoreA - ); + )[0]; let dataLoss: VisualizationSelection['dataLoss'] = 'nothing'; - if (!suggestions.length && containsData) { + if (!topSuggestion && containsData) { dataLoss = 'everything'; } else if (layers.length > 1 && containsData) { dataLoss = 'layers'; @@ -279,46 +144,25 @@ export function ConfigPanelHeader(props: Props) { visualizationId, subVisualizationId, dataLoss, - options: suggestions.length - ? suggestions.map(suggestion => ({ - getVisualizationState: () => - switchVisType( - subVisualizationId, - newVisualization.initialize(props.framePublicAPI, suggestion.suggestion.state) - ), - title: suggestion.suggestion.title, - icon: suggestion.suggestion.previewIcon, - keptLayerId: suggestion.layerId, - })) - : [ - { - getVisualizationState: () => { - return switchVisType( - subVisualizationId, - newVisualization.initialize(props.framePublicAPI) - ); - }, - icon: '', - keptLayerId: '', - title: '', - }, - ], + getVisualizationState: topSuggestion + ? () => + switchVisType( + subVisualizationId, + newVisualization.initialize(props.framePublicAPI, topSuggestion.suggestion.state) + ) + : () => { + return switchVisType( + subVisualizationId, + newVisualization.initialize(props.framePublicAPI) + ); + }, + keptLayerId: topSuggestion ? topSuggestion.layerId : '', }; } - const onSelect = (visualizationId: string, subVisualizationId: string) => { - const selection = getSelection(visualizationId, subVisualizationId); - const shouldRequestConfirmation = selection.dataLoss !== 'nothing'; - - if (shouldRequestConfirmation) { - requestConfirmation(selection); - } else { - commitSelection(selection, 0); - } - }; - const visualizationTypes = useMemo( () => + flyoutOpen && flatten( Object.values(props.visualizationMap).map(v => v.visualizationTypes.map(t => ({ @@ -326,23 +170,21 @@ export function ConfigPanelHeader(props: Props) { ...t, })) ) - ), - [props.visualizationMap] + ).map(visualizationType => ({ + ...visualizationType, + selection: getSelection(visualizationType.visualizationId, visualizationType.id), + })), + [ + flyoutOpen, + props.visualizationMap, + props.framePublicAPI, + props.visualizationId, + props.visualizationState, + ] ); return ( <> - {state.confirmVisualization && ( - setState({ ...state, confirmVisualization: undefined })} - onConfirm={(optionIndex: number) => - commitSelection(state.confirmVisualization!, optionIndex) - } - /> - )} setState({ isFlyoutOpen: !state.isFlyoutOpen })} + onClick={() => setFlyoutOpen(!flyoutOpen)} data-test-subj="lnsConfigPanelHeaderPopover" > } - isOpen={state.isFlyoutOpen} - closePopover={() => hideFlyout()} + isOpen={flyoutOpen} + closePopover={() => setFlyoutOpen(false)} anchorPosition="leftUp" > @@ -366,13 +208,29 @@ export function ConfigPanelHeader(props: Props) { })} - {visualizationTypes.map(v => ( + {(visualizationTypes || []).map(v => ( {v.label}} role="menuitem" data-test-subj={`lnsConfigPanelHeaderPopover_${v.id}`} - onClick={() => onSelect(v.visualizationId, v.id)} + onClick={() => commitSelection(v.selection)} + betaBadgeLabel={ + v.selection.dataLoss !== 'nothing' + ? i18n.translate('xpack.lens.configPanel.dataLossLabel', { + defaultMessage: 'Data loss', + }) + : undefined + } + betaBadgeTooltipContent={ + v.selection.dataLoss !== 'nothing' + ? i18n.translate('xpack.lens.configPanel.dataLossDescription', { + defaultMessage: + 'Switching to this chart will loose some of the configuration', + }) + : undefined + } + betaBadgeIconType={v.selection.dataLoss !== 'nothing' ? 'bolt' : undefined} > diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx index 424371d308da3..81fc0748d5682 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx @@ -275,6 +275,37 @@ describe('editor_frame', () => { expect(mockDatasource2.insertLayer).toHaveBeenCalledWith(initialState, expect.anything()); }); + it('should remove layer on active datasource on frame api call', async () => { + const initialState = { datasource2: '' }; + mockDatasource2.initialize.mockReturnValue(Promise.resolve(initialState)); + mockDatasource2.getLayers.mockReturnValue(['abc', 'def']); + mockDatasource2.removeLayer.mockReturnValue({ removed: true }); + act(() => { + mount( + + ); + }); + + await waitForPromises(); + + mockVisualization.initialize.mock.calls[0][0].removeLayers(['abc', 'def']); + + expect(mockDatasource2.removeLayer).toHaveBeenCalledWith(initialState, 'abc'); + expect(mockDatasource2.removeLayer).toHaveBeenCalledWith({ removed: true }, 'def'); + }); + it('should render data panel after initialization is complete', async () => { const initialState = {}; let databaseInitialized: ({}) => void; diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index 3ebf489e15474..3e89398137c4a 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -95,7 +95,7 @@ export function EditorFrame(props: EditorFrameProps) { const newLayerId = generateId(); dispatch({ - type: 'INSERT_LAYER', + type: 'UPDATE_LAYER', datasourceId: state.activeDatasourceId!, layerId: newLayerId, reducer: props.datasourceMap[state.activeDatasourceId!].insertLayer, @@ -107,10 +107,11 @@ export function EditorFrame(props: EditorFrameProps) { layerIds.forEach(layerId => { const layerDatasourceId = Object.entries(props.datasourceMap).find( ([datasourceId, datasource]) => - datasource.getLayers(state.datasourceStates[datasourceId].state) + state.datasourceStates[datasourceId] && + datasource.getLayers(state.datasourceStates[datasourceId].state).includes(layerId) )![0]; dispatch({ - type: 'REMOVE_LAYER', + type: 'UPDATE_LAYER', layerId, datasourceId: layerDatasourceId, reducer: props.datasourceMap[layerDatasourceId].removeLayer, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts index e02614ffb4be5..0fdfaa57efead 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts @@ -153,6 +153,36 @@ describe('editor_frame state management', () => { expect(newState.datasourceStates.testDatasource.state).toBe(newDatasourceState); }); + it('should update the datasource state with passed in reducer', () => { + const layerReducer = jest.fn((_state, layerId) => ({ inserted: layerId })); + const newState = reducer( + { + datasourceStates: { + testDatasource: { + state: {}, + isLoading: false, + }, + }, + activeDatasourceId: 'testDatasource', + saving: false, + title: 'bbb', + visualization: { + activeId: 'testVis', + state: {}, + }, + }, + { + type: 'UPDATE_LAYER', + layerId: 'abc', + reducer: layerReducer, + datasourceId: 'testDatasource', + } + ); + + expect(newState.datasourceStates.testDatasource.state).toEqual({ inserted: 'abc' }); + expect(layerReducer).toHaveBeenCalledTimes(1); + }); + it('should should switch active visualization', () => { const testVisState = {}; const newVisState = {}; diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts index 0cf7334e1d8d5..3695842728fee 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts @@ -47,13 +47,7 @@ export type Action = newState: unknown; } | { - type: 'REMOVE_LAYER'; - layerId: string; - datasourceId: string; - reducer: (state: unknown, layerId: string) => unknown; - } - | { - type: 'INSERT_LAYER'; + type: 'UPDATE_LAYER'; layerId: string; datasourceId: string; reducer: (state: unknown, layerId: string) => unknown; @@ -109,7 +103,7 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta return { ...state, persistedId: action.id }; case 'UPDATE_TITLE': return { ...state, title: action.title }; - case 'INSERT_LAYER': + case 'UPDATE_LAYER': return { ...state, datasourceStates: { @@ -123,18 +117,6 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta }, }, }; - case 'REMOVE_LAYER': - const layerDatasourceId = action.datasourceId; - return { - ...state, - datasourceStates: { - ...state.datasourceStates, - [layerDatasourceId]: { - ...state.datasourceStates[layerDatasourceId], - state: action.reducer(state.datasourceStates[layerDatasourceId].state, action.layerId), - }, - }, - }; case 'VISUALIZATION_LOADED': return { ...state, From 0f1a3243f39e0956322146e3865c2fb86ab7e62b Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 31 Jul 2019 09:35:40 +0200 Subject: [PATCH 13/16] fix review comments and rename component --- ..._header.test.tsx => chart_switch.test.tsx} | 33 +++++++++---------- ...nfig_panel_header.tsx => chart_switch.tsx} | 15 ++++----- .../editor_frame/config_panel_wrapper.tsx | 6 ++-- .../editor_frame/editor_frame.test.tsx | 4 +-- .../editor_frame/editor_frame.tsx | 4 +-- .../editor_frame/state_management.test.ts | 2 +- .../editor_frame/state_management.ts | 4 +-- .../public/xy_visualization_plugin/types.ts | 8 ++--- 8 files changed, 37 insertions(+), 39 deletions(-) rename x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/{config_panel_header.test.tsx => chart_switch.test.tsx} (93%) rename x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/{config_panel_header.tsx => chart_switch.tsx} (93%) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.test.tsx similarity index 93% rename from x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx rename to x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.test.tsx index d7e7a73d2941a..1d5fb3563d744 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.test.tsx @@ -8,11 +8,11 @@ import React from 'react'; import { createMockVisualization, createMockFramePublicAPI } from '../mocks'; import { mountWithIntl as mount } from 'test_utils/enzyme_helpers'; import { ReactWrapper } from 'enzyme'; -import { ConfigPanelHeader } from './config_panel_header'; +import { ChartSwitch } from './chart_switch'; import { Visualization, FramePublicAPI, DatasourcePublicAPI } from '../../types'; import { EuiKeyPadMenuItemButton } from '@elastic/eui'; -describe('config_panel_header', () => { +describe('chart_switch', () => { function generateVisualization(id: string): jest.Mocked { return { ...createMockVisualization(), @@ -85,7 +85,7 @@ describe('config_panel_header', () => { function showFlyout(component: ReactWrapper) { component - .find('[data-test-subj="lnsConfigPanelHeaderPopover"]') + .find('[data-test-subj="lnsChartSwitchPopover"]') .first() .simulate('click'); } @@ -93,7 +93,7 @@ describe('config_panel_header', () => { function switchTo(subType: string, component: ReactWrapper) { showFlyout(component); component - .find(`[data-test-subj="lnsConfigPanelHeaderPopover_${subType}"]`) + .find(`[data-test-subj="lnsChartSwitchPopover_${subType}"]`) .first() .simulate('click'); } @@ -102,7 +102,7 @@ describe('config_panel_header', () => { showFlyout(component); return component .find(EuiKeyPadMenuItemButton) - .find(`[data-test-subj="lnsConfigPanelHeaderPopover_${subType}"]`) + .find(`[data-test-subj="lnsChartSwitchPopover_${subType}"]`) .first(); } @@ -110,7 +110,7 @@ describe('config_panel_header', () => { const dispatch = jest.fn(); const visualizations = mockVisualizations(); const component = mount( - { (frame.datasourceLayers.a.getTableSpec as jest.Mock).mockReturnValue([]); const component = mount( - { const frame = mockFrame(['a', 'b']); const component = mount( - { const frame = mockFrame(['a']); const component = mount( - { (frame.datasourceLayers.a.getTableSpec as jest.Mock).mockReturnValue([]); const component = mount( - { visualizations.visC.switchVisualizationType = switchVisualizationType; const component = mount( - { removeLayers, }; const component = mount( - { visualizations.visC.switchVisualizationType = switchVisualizationType; const component = mount( - { visualizations.visB.switchVisualizationType = switchVisualizationType; const component = mount( - { it('should show all visualization types', () => { const component = mount( - { showFlyout(component); const allDisplayed = ['subvisA', 'subvisB', 'subvisC1', 'subvisC2'].every( - subType => - component.find(`[data-test-subj="lnsConfigPanelHeaderPopover_${subType}"]`).length > 0 + subType => component.find(`[data-test-subj="lnsChartSwitchPopover_${subType}"]`).length > 0 ); expect(allDisplayed).toBeTruthy(); diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.tsx similarity index 93% rename from x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx rename to x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.tsx index 81e7d6282fdfd..5fe5c7e2e2921 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_header.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.tsx @@ -67,7 +67,7 @@ function VisualizationSummary(props: Props) { ); } -export function ConfigPanelHeader(props: Props) { +export function ChartSwitch(props: Props) { const [flyoutOpen, setFlyoutOpen] = useState(false); const commitSelection = (selection: VisualizationSelection) => { @@ -186,14 +186,14 @@ export function ConfigPanelHeader(props: Props) { return ( <> setFlyoutOpen(!flyoutOpen)} - data-test-subj="lnsConfigPanelHeaderPopover" + data-test-subj="lnsChartSwitchPopover" > @@ -213,20 +213,19 @@ export function ConfigPanelHeader(props: Props) { key={`${v.visualizationId}:${v.id}`} label={{v.label}} role="menuitem" - data-test-subj={`lnsConfigPanelHeaderPopover_${v.id}`} + data-test-subj={`lnsChartSwitchPopover_${v.id}`} onClick={() => commitSelection(v.selection)} betaBadgeLabel={ v.selection.dataLoss !== 'nothing' - ? i18n.translate('xpack.lens.configPanel.dataLossLabel', { + ? i18n.translate('xpack.lens.chartSwitch.dataLossLabel', { defaultMessage: 'Data loss', }) : undefined } betaBadgeTooltipContent={ v.selection.dataLoss !== 'nothing' - ? i18n.translate('xpack.lens.configPanel.dataLossDescription', { - defaultMessage: - 'Switching to this chart will loose some of the configuration', + ? i18n.translate('xpack.lens.chartSwitch.dataLossDescription', { + defaultMessage: 'Switching to this chart will lose some of the configuration', }) : undefined } diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx index 658673bc3071d..7c7fc0906717b 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx @@ -9,7 +9,7 @@ import { NativeRenderer } from '../../native_renderer'; import { Action } from './state_management'; import { Visualization, FramePublicAPI } from '../../types'; import { DragContext } from '../../drag_drop'; -import { ConfigPanelHeader } from './config_panel_header'; +import { ChartSwitch } from './chart_switch'; interface ConfigPanelWrapperProps { visualizationState: unknown; @@ -33,8 +33,8 @@ export const ConfigPanelWrapper = memo(function ConfigPanelWrapper(props: Config return ( <> - { function switchTo(subType: string) { act(() => { instance - .find('[data-test-subj="lnsConfigPanelHeaderPopover"]') + .find('[data-test-subj="lnsChartSwitchPopover"]') .last() .simulate('click'); }); @@ -858,7 +858,7 @@ describe('editor_frame', () => { act(() => { instance - .find(`[data-test-subj="lnsConfigPanelHeaderPopover_${subType}"]`) + .find(`[data-test-subj="lnsChartSwitchPopover_${subType}"]`) .last() .simulate('click'); }); diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index 3e89398137c4a..09b2884b29b92 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -98,7 +98,7 @@ export function EditorFrame(props: EditorFrameProps) { type: 'UPDATE_LAYER', datasourceId: state.activeDatasourceId!, layerId: newLayerId, - reducer: props.datasourceMap[state.activeDatasourceId!].insertLayer, + updater: props.datasourceMap[state.activeDatasourceId!].insertLayer, }); return newLayerId; @@ -114,7 +114,7 @@ export function EditorFrame(props: EditorFrameProps) { type: 'UPDATE_LAYER', layerId, datasourceId: layerDatasourceId, - reducer: props.datasourceMap[layerDatasourceId].removeLayer, + updater: props.datasourceMap[layerDatasourceId].removeLayer, }); }); }, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts index 0fdfaa57efead..eccca61814303 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts @@ -174,7 +174,7 @@ describe('editor_frame state management', () => { { type: 'UPDATE_LAYER', layerId: 'abc', - reducer: layerReducer, + updater: layerReducer, datasourceId: 'testDatasource', } ); diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts index 3695842728fee..bfa52341b9495 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts @@ -50,7 +50,7 @@ export type Action = type: 'UPDATE_LAYER'; layerId: string; datasourceId: string; - reducer: (state: unknown, layerId: string) => unknown; + updater: (state: unknown, layerId: string) => unknown; } | { type: 'VISUALIZATION_LOADED'; @@ -110,7 +110,7 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta ...state.datasourceStates, [action.datasourceId]: { ...state.datasourceStates[action.datasourceId], - state: action.reducer( + state: action.updater( state.datasourceStates[action.datasourceId].state, action.layerId ), diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts index 0d69601cf0810..2287ec3c3d1ef 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/types.ts @@ -192,15 +192,15 @@ export const visualizationTypes: VisualizationType[] = [ { id: 'bar', icon: 'visBarVertical', - label: i18n.translate('xpack.lens.xyVisualization.verticalBarLabel', { - defaultMessage: 'Vertical Bar', + label: i18n.translate('xpack.lens.xyVisualization.barLabel', { + defaultMessage: 'Bar', }), }, { id: 'bar_stacked', icon: 'visBarVertical', - label: i18n.translate('xpack.lens.xyVisualization.stackedVerticalBarLabel', { - defaultMessage: 'Stacked Vertical Bar', + label: i18n.translate('xpack.lens.xyVisualization.stackedBarLabel', { + defaultMessage: 'Stacked Bar', }), }, { From 54545037b794d445fd031e38feec40bd2c51ba84 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 1 Aug 2019 11:33:14 +0200 Subject: [PATCH 14/16] initially focus chart switch popover --- .../public/editor_frame_plugin/editor_frame/chart_switch.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.tsx index 5fe5c7e2e2921..08d5e8650c374 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.tsx @@ -188,6 +188,8 @@ export function ChartSwitch(props: Props) { Date: Fri, 2 Aug 2019 11:19:52 +0200 Subject: [PATCH 15/16] improve default series type --- .../xy_config_panel.test.tsx | 104 +++++++++++++++++- .../xy_config_panel.tsx | 7 +- 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx index 2c23d69c6bf9d..64ceddac2021d 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx @@ -10,7 +10,7 @@ import { mountWithIntl as mount } from 'test_utils/enzyme_helpers'; import { EuiButtonGroupProps } from '@elastic/eui'; import { XYConfigPanel } from './xy_config_panel'; import { DatasourceDimensionPanelProps, Operation, FramePublicAPI } from '../types'; -import { State } from './types'; +import { State, XYState } from './types'; import { Position } from '@elastic/charts'; import { NativeRendererProps } from '../native_renderer'; import { generateId } from '../id_generator'; @@ -296,6 +296,108 @@ describe('XYConfigPanel', () => { }); }); + it('should use series type of existing layers if they all have the same', () => { + frame.addNewLayer = jest.fn().mockReturnValue('newLayerId'); + frame.datasourceLayers.second = createMockDatasource().publicAPIMock; + (generateId as jest.Mock).mockReturnValue('accessor'); + const setState = jest.fn(); + const state: XYState = { + ...testState(), + preferredSeriesType: 'bar', + layers: [ + { + seriesType: 'line', + layerId: 'first', + splitAccessor: 'baz', + xAccessor: 'foo', + title: 'X', + accessors: ['bar'], + }, + { + seriesType: 'line', + layerId: 'second', + splitAccessor: 'baz', + xAccessor: 'foo', + title: 'Y', + accessors: ['bar'], + }, + ], + }; + const component = mount( + + ); + + component + .find('[data-test-subj="lnsXY_layer_add"]') + .first() + .simulate('click'); + + expect(setState.mock.calls[0][0]).toMatchObject({ + layers: [ + ...state.layers, + expect.objectContaining({ + seriesType: 'line', + }), + ], + }); + }); + + it('should use preffered series type if there are already various different layers', () => { + frame.addNewLayer = jest.fn().mockReturnValue('newLayerId'); + frame.datasourceLayers.second = createMockDatasource().publicAPIMock; + (generateId as jest.Mock).mockReturnValue('accessor'); + const setState = jest.fn(); + const state: XYState = { + ...testState(), + preferredSeriesType: 'bar', + layers: [ + { + seriesType: 'area', + layerId: 'first', + splitAccessor: 'baz', + xAccessor: 'foo', + title: 'X', + accessors: ['bar'], + }, + { + seriesType: 'line', + layerId: 'second', + splitAccessor: 'baz', + xAccessor: 'foo', + title: 'Y', + accessors: ['bar'], + }, + ], + }; + const component = mount( + + ); + + component + .find('[data-test-subj="lnsXY_layer_add"]') + .first() + .simulate('click'); + + expect(setState.mock.calls[0][0]).toMatchObject({ + layers: [ + ...state.layers, + expect.objectContaining({ + seriesType: 'bar', + }), + ], + }); + }); + it('removes layers', () => { const setState = jest.fn(); const state = testState(); diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index 65a9526cfc394..03892abc84c3e 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import _ from 'lodash'; import React, { useState } from 'react'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; @@ -276,11 +277,15 @@ export function XYConfigPanel(props: VisualizationProps) { size="s" data-test-subj={`lnsXY_layer_add`} onClick={() => { + const usedSeriesTypes = _.uniq(state.layers.map(layer => layer.seriesType)); setState({ ...state, layers: [ ...state.layers, - newLayerState(state.preferredSeriesType, frame.addNewLayer()), + newLayerState( + usedSeriesTypes.length === 1 ? usedSeriesTypes[0] : state.preferredSeriesType, + frame.addNewLayer() + ), ], }); }} From 6610ea75df2ef685c1b9f628628ee2b08117ce24 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 2 Aug 2019 12:53:09 +0200 Subject: [PATCH 16/16] fix type bug --- .../lens/public/xy_visualization_plugin/xy_suggestions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts index a21789fec7043..a7eb5f30b7128 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts @@ -114,7 +114,6 @@ function getSuggestion( layerId, seriesType, xAccessor: xValue.columnId, - seriesType: splitBy && isDate ? 'line' : 'bar', splitAccessor: splitBy ? splitBy.columnId : generateId(), accessors: yValues.map(col => col.columnId), title: yTitle,