From 2669613dc3b61666b4eeb097c60c2f7f85b6cdad Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Mon, 1 Jun 2020 13:33:23 -0400 Subject: [PATCH] [Lens] Allow visualizations to provide a dimension editor (#67560) * [Lens] Allow visualizations to provide a dimension editor * Update to tab style * Remove table update * Update class name * typecheck fix * Add test * Require each dimension group to enable editor Co-authored-by: Elastic Machine Co-authored-by: Marta Bondyra --- .../config_panel/_layer_panel.scss | 3 + .../config_panel/config_panel.tsx | 3 - .../config_panel/dimension_popover.tsx | 2 +- .../config_panel/layer_panel.test.tsx | 271 ++++++++++++++++++ .../editor_frame/config_panel/layer_panel.tsx | 263 ++++++++++------- .../editor_frame/config_panel/types.ts | 1 + x-pack/plugins/lens/public/types.ts | 21 ++ 7 files changed, 458 insertions(+), 106 deletions(-) create mode 100644 x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss index 3fbc42f9a25a0..924f44a37c459 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss @@ -31,3 +31,6 @@ min-height: $euiSizeXXL; } +.lnsLayerPanel__styleEditor { + width: $euiSize * 28; +} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx index 0d86a051b0faa..e53e465c18950 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx @@ -45,7 +45,6 @@ function LayerPanels( } ) { const { - framePublicAPI, activeVisualization, visualizationState, dispatch, @@ -109,12 +108,10 @@ function LayerPanels( {...props} key={layerId} layerId={layerId} - activeVisualization={activeVisualization} visualizationState={visualizationState} updateVisualization={setVisualizationState} updateDatasource={updateDatasource} updateAll={updateAll} - frame={framePublicAPI} isOnlyLayer={layerIds.length === 1} onRemoveLayer={() => { dispatch({ diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_popover.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_popover.tsx index f89b6ef32d3f7..cc8d97a445016 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_popover.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_popover.tsx @@ -36,7 +36,7 @@ export function DimensionPopover({ (popoverState.openId === accessor || (noMatch && popoverState.addingToGroupId === groupId)) } closePopover={() => { - setPopoverState({ isOpen: false, openId: null, addingToGroupId: null }); + setPopoverState({ isOpen: false, openId: null, addingToGroupId: null, tabId: null }); }} button={trigger} anchorPosition="leftUp" diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx new file mode 100644 index 0000000000000..1f987f86d3950 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx @@ -0,0 +1,271 @@ +/* + * 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 { act } from 'react-dom/test-utils'; +import { + createMockVisualization, + createMockFramePublicAPI, + createMockDatasource, + DatasourceMock, +} from '../../mocks'; +import { EuiFormRow, EuiPopover } from '@elastic/eui'; +import { mount } from 'enzyme'; +import { mountWithIntl } from 'test_utils/enzyme_helpers'; +import { Visualization } from '../../../types'; +import { LayerPanel } from './layer_panel'; +import { coreMock } from 'src/core/public/mocks'; +import { generateId } from '../../../id_generator'; + +jest.mock('../../../id_generator'); + +describe('LayerPanel', () => { + let mockVisualization: jest.Mocked; + let mockDatasource: DatasourceMock; + + function getDefaultProps() { + const frame = createMockFramePublicAPI(); + frame.datasourceLayers = { + first: mockDatasource.publicAPIMock, + }; + return { + layerId: 'first', + activeVisualizationId: 'vis1', + visualizationMap: { + vis1: mockVisualization, + }, + activeDatasourceId: 'ds1', + datasourceMap: { + ds1: mockDatasource, + }, + datasourceStates: { + ds1: { + isLoading: false, + state: 'state', + }, + }, + visualizationState: 'state', + updateVisualization: jest.fn(), + updateDatasource: jest.fn(), + updateAll: jest.fn(), + framePublicAPI: frame, + isOnlyLayer: true, + onRemoveLayer: jest.fn(), + dispatch: jest.fn(), + core: coreMock.createStart(), + }; + } + + beforeEach(() => { + mockVisualization = { + ...createMockVisualization(), + id: 'testVis', + visualizationTypes: [ + { + icon: 'empty', + id: 'testVis', + label: 'TEST1', + }, + ], + }; + + mockVisualization.getLayerIds.mockReturnValue(['first']); + mockDatasource = createMockDatasource('ds1'); + }); + + it('should fail to render if the public API is out of date', () => { + const props = getDefaultProps(); + props.framePublicAPI.datasourceLayers = {}; + const component = mountWithIntl(); + expect(component.isEmptyRender()).toBe(true); + }); + + it('should fail to render if the active visualization is missing', () => { + const component = mountWithIntl( + + ); + expect(component.isEmptyRender()).toBe(true); + }); + + describe('layer reset and remove', () => { + it('should show the reset button when single layer', () => { + const component = mountWithIntl(); + expect(component.find('[data-test-subj="lns_layer_remove"]').first().text()).toContain( + 'Reset layer' + ); + }); + + it('should show the delete button when multiple layers', () => { + const component = mountWithIntl(); + expect(component.find('[data-test-subj="lns_layer_remove"]').first().text()).toContain( + 'Delete layer' + ); + }); + + it('should call the clear callback', () => { + const cb = jest.fn(); + const component = mountWithIntl(); + act(() => { + component.find('[data-test-subj="lns_layer_remove"]').first().simulate('click'); + }); + expect(cb).toHaveBeenCalled(); + }); + }); + + describe('single group', () => { + it('should render the non-editable state', () => { + mockVisualization.getConfiguration.mockReturnValue({ + groups: [ + { + groupLabel: 'A', + groupId: 'a', + accessors: ['x'], + filterOperations: () => true, + supportsMoreColumns: false, + dataTestSubj: 'lnsGroup', + }, + ], + }); + + const component = mountWithIntl(); + + const group = component.find('DragDrop[data-test-subj="lnsGroup"]'); + expect(group).toHaveLength(1); + }); + + it('should render the group with a way to add a new column', () => { + mockVisualization.getConfiguration.mockReturnValue({ + groups: [ + { + groupLabel: 'A', + groupId: 'a', + accessors: [], + filterOperations: () => true, + supportsMoreColumns: true, + dataTestSubj: 'lnsGroup', + }, + ], + }); + + const component = mountWithIntl(); + + const group = component.find('DragDrop[data-test-subj="lnsGroup"]'); + expect(group).toHaveLength(1); + }); + + it('should render the required warning when only one group is configured', () => { + mockVisualization.getConfiguration.mockReturnValue({ + groups: [ + { + groupLabel: 'A', + groupId: 'a', + accessors: ['x'], + filterOperations: () => true, + supportsMoreColumns: false, + dataTestSubj: 'lnsGroup', + }, + { + groupLabel: 'B', + groupId: 'b', + accessors: [], + filterOperations: () => true, + supportsMoreColumns: true, + dataTestSubj: 'lnsGroup', + required: true, + }, + ], + }); + + const component = mountWithIntl(); + + const group = component + .find(EuiFormRow) + .findWhere((e) => e.prop('error') === 'Required dimension'); + expect(group).toHaveLength(1); + }); + + it('should render the datasource and visualization panels inside the dimension popover', () => { + mockVisualization.getConfiguration.mockReturnValueOnce({ + groups: [ + { + groupLabel: 'A', + groupId: 'a', + accessors: ['newid'], + filterOperations: () => true, + supportsMoreColumns: false, + dataTestSubj: 'lnsGroup', + enableDimensionEditor: true, + }, + ], + }); + mockVisualization.renderDimensionEditor = jest.fn(); + + const component = mountWithIntl(); + + const group = component.find('DimensionPopover'); + const panel = mount(group.prop('panel')); + + expect(panel.find('EuiTabbedContent').prop('tabs')).toHaveLength(2); + act(() => { + panel.find('EuiTab#visualization').simulate('click'); + }); + expect(mockVisualization.renderDimensionEditor).toHaveBeenCalledWith( + expect.any(Element), + expect.objectContaining({ + groupId: 'a', + accessor: 'newid', + }) + ); + }); + + it('should keep the popover open when configuring a new dimension', () => { + /** + * The ID generation system for new dimensions has been messy before, so + * this tests that the ID used in the first render is used to keep the popover + * open in future renders + */ + (generateId as jest.Mock).mockReturnValueOnce(`newid`); + (generateId as jest.Mock).mockReturnValueOnce(`bad`); + mockVisualization.getConfiguration.mockReturnValueOnce({ + groups: [ + { + groupLabel: 'A', + groupId: 'a', + accessors: [], + filterOperations: () => true, + supportsMoreColumns: true, + dataTestSubj: 'lnsGroup', + }, + ], + }); + // Normally the configuration would change in response to a state update, + // but this test is updating it directly + mockVisualization.getConfiguration.mockReturnValueOnce({ + groups: [ + { + groupLabel: 'A', + groupId: 'a', + accessors: ['newid'], + filterOperations: () => true, + supportsMoreColumns: false, + dataTestSubj: 'lnsGroup', + }, + ], + }); + + const component = mountWithIntl(); + + const group = component.find('DimensionPopover'); + const triggerButton = mountWithIntl(group.prop('trigger')); + act(() => { + triggerButton.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click'); + }); + component.update(); + + expect(component.find(EuiPopover).prop('isOpen')).toBe(true); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index 814b7fc644c9c..bd501db2b752a 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -13,11 +13,12 @@ import { EuiFlexItem, EuiButtonEmpty, EuiFormRow, + EuiTabbedContent, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { NativeRenderer } from '../../../native_renderer'; -import { Visualization, FramePublicAPI, StateSetter } from '../../../types'; +import { StateSetter } from '../../../types'; import { DragContext, DragDrop, ChildDragDropProvider } from '../../../drag_drop'; import { LayerSettings } from './layer_settings'; import { trackUiEvent } from '../../../lens_ui_telemetry'; @@ -27,11 +28,8 @@ import { DimensionPopover } from './dimension_popover'; export function LayerPanel( props: Exclude & { - frame: FramePublicAPI; layerId: string; isOnlyLayer: boolean; - activeVisualization: Visualization; - visualizationState: unknown; updateVisualization: StateSetter; updateDatasource: (datasourceId: string, newState: unknown) => void; updateAll: ( @@ -47,13 +45,19 @@ export function LayerPanel( isOpen: false, openId: null, addingToGroupId: null, + tabId: null, }); - const { framePublicAPI, layerId, activeVisualization, isOnlyLayer, onRemoveLayer } = props; + const { framePublicAPI, layerId, isOnlyLayer, onRemoveLayer } = props; const datasourcePublicAPI = framePublicAPI.datasourceLayers[layerId]; - if (!datasourcePublicAPI) { + if ( + !datasourcePublicAPI || + !props.activeVisualizationId || + !props.visualizationMap[props.activeVisualizationId] + ) { return null; } + const activeVisualization = props.visualizationMap[props.activeVisualizationId]; const layerVisualizationConfigProps = { layerId, dragDropContext, @@ -158,104 +162,156 @@ export function LayerPanel( } > <> - {group.accessors.map((accessor) => ( - { - layerDatasource.onDrop({ - ...layerDatasourceDropProps, - droppedItem, - columnId: accessor, - filterOperations: group.filterOperations, - }); - }} - > - { - if (popoverState.isOpen) { - setPopoverState({ - isOpen: false, - openId: null, - addingToGroupId: null, - }); - } else { - setPopoverState({ - isOpen: true, - openId: accessor, - addingToGroupId: null, // not set for existing dimension - }); - } - }, - }} - /> - } - panel={ - - } - /> + {group.accessors.map((accessor) => { + const tabs = [ + { + id: 'datasource', + name: i18n.translate('xpack.lens.editorFrame.quickFunctionsLabel', { + defaultMessage: 'Quick functions', + }), + content: ( + <> + + + + ), + }, + ]; - { - trackUiEvent('indexpattern_dimension_removed'); - props.updateAll( - datasourceId, - layerDatasource.removeColumn({ - layerId, - columnId: accessor, - prevState: layerDatasourceState, - }), - props.activeVisualization.removeDimension({ - layerId, - columnId: accessor, - prevState: props.visualizationState, - }) - ); + if (activeVisualization.renderDimensionEditor) { + tabs.push({ + id: 'visualization', + name: i18n.translate('xpack.lens.editorFrame.formatStyleLabel', { + defaultMessage: 'Format & style', + }), + content: ( +
+ + +
+ ), + }); + } + + return ( + { + layerDatasource.onDrop({ + ...layerDatasourceDropProps, + droppedItem, + columnId: accessor, + filterOperations: group.filterOperations, + }); }} - /> - - ))} + > + { + if (popoverState.isOpen) { + setPopoverState({ + isOpen: false, + openId: null, + addingToGroupId: null, + tabId: null, + }); + } else { + setPopoverState({ + isOpen: true, + openId: accessor, + addingToGroupId: null, // not set for existing dimension + tabId: 'datasource', + }); + } + }, + }} + /> + } + panel={ + t.id === popoverState.tabId)} + size="s" + onTabClick={(tab) => { + setPopoverState({ + ...popoverState, + tabId: tab.id as typeof popoverState['tabId'], + }); + }} + /> + } + /> + + { + trackUiEvent('indexpattern_dimension_removed'); + props.updateAll( + datasourceId, + layerDatasource.removeColumn({ + layerId, + columnId: accessor, + prevState: layerDatasourceState, + }), + activeVisualization.removeDimension({ + layerId, + columnId: accessor, + prevState: props.visualizationState, + }) + ); + }} + /> + + ); + })} {group.supportsMoreColumns ? ( = VisualizationConfigProp setState: (newState: T) => void; }; +export type VisualizationDimensionEditorProps = VisualizationConfigProps & { + groupId: string; + accessor: string; + setState: (newState: T) => void; +}; + export type VisualizationDimensionGroupConfig = SharedDimensionProps & { groupLabel: string; @@ -300,6 +306,12 @@ export type VisualizationDimensionGroupConfig = SharedDimensionProps & { /** If required, a warning will appear if accessors are empty */ required?: boolean; dataTestSubj?: string; + + /** + * When the dimension editor is enabled for this group, all dimensions in the group + * will render the extra tab for the dimension editor + */ + enableDimensionEditor?: boolean; }; interface VisualizationDimensionChangeProps { @@ -459,6 +471,15 @@ export interface Visualization { */ removeDimension: (props: VisualizationDimensionChangeProps) => T; + /** + * Additional editor that gets rendered inside the dimension popover. + * This can be used to configure dimension-specific options + */ + renderDimensionEditor?: ( + domElement: Element, + props: VisualizationDimensionEditorProps + ) => void; + /** * The frame will call this function on all visualizations at different times. The * main use cases where visualization suggestions are requested are: