[Lens] Remove extra render when closing flyout with valid column (#84951

) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Dec 4, 2020 · f176e8b · f176e8b
1 parent db70286
commit f176e8b
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 6 deletions.
diff --git a/...k/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx b/...k/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx
@@ -323,6 +323,47 @@ describe('LayerPanel', () => {
       component.update();
       expect(component.find('EuiFlyoutHeader').exists()).toBe(false);
     });
+
+    it('should only update the state on close when needed', () => {
+      const updateDatasource = jest.fn();
+      mockVisualization.getConfiguration.mockReturnValue({
+        groups: [
+          {
+            groupLabel: 'A',
+            groupId: 'a',
+            accessors: [{ columnId: 'a' }],
+            filterOperations: () => true,
+            supportsMoreColumns: false,
+            dataTestSubj: 'lnsGroup',
+          },
+        ],
+      });
+
+      const component = mountWithIntl(
+        <LayerPanel {...getDefaultProps()} updateDatasource={updateDatasource} />
+      );
+
+      // Close without a state update
+      mockDatasource.updateStateOnCloseDimension = jest.fn();
+      component.find('[data-test-subj="lnsLayerPanel-dimensionLink"]').first().simulate('click');
+      act(() => {
+        (component.find('DimensionContainer').first().prop('handleClose') as () => void)();
+      });
+      component.update();
+      expect(mockDatasource.updateStateOnCloseDimension).toHaveBeenCalled();
+      expect(updateDatasource).not.toHaveBeenCalled();
+
+      // Close with a state update
+      mockDatasource.updateStateOnCloseDimension = jest.fn().mockReturnValue({ newState: true });
+
+      component.find('[data-test-subj="lnsLayerPanel-dimensionLink"]').first().simulate('click');
+      act(() => {
+        (component.find('DimensionContainer').first().prop('handleClose') as () => void)();
+      });
+      component.update();
+      expect(mockDatasource.updateStateOnCloseDimension).toHaveBeenCalled();
+      expect(updateDatasource).toHaveBeenCalledWith('ds1', { newState: true });
+    });
   });
 
   // This test is more like an integration test, since the layer panel owns all

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
@@ -321,6 +321,7 @@ export function LayerPanel(
                         <div className="lnsLayerPanel__dimension">
                           <EuiLink
                             className="lnsLayerPanel__dimensionLink"
+                            data-test-subj="lnsLayerPanel-dimensionLink"
                             onClick={() => {
                               if (activeId) {
                                 setActiveDimension(initialActiveDimensionState);
@@ -485,7 +486,9 @@ export function LayerPanel(
                 layerId,
                 columnId: activeId!,
               });
-              props.updateDatasource(datasourceId, newState);
+              if (newState) {
+                props.updateDatasource(datasourceId, newState);
+              }
             }
             setActiveDimension(initialActiveDimensionState);
           }}

diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts
@@ -990,6 +990,38 @@ describe('IndexPattern Data Source', () => {
   });
 
   describe('#updateStateOnCloseDimension', () => {
+    it('should not update when there are no incomplete columns', () => {
+      expect(
+        indexPatternDatasource.updateStateOnCloseDimension!({
+          state: {
+            indexPatternRefs: [],
+            existingFields: {},
+            isFirstExistenceFetch: false,
+            indexPatterns: expectedIndexPatterns,
+            layers: {
+              first: {
+                indexPatternId: '1',
+                columnOrder: ['col1'],
+                columns: {
+                  col1: {
+                    dataType: 'number',
+                    isBucketed: false,
+                    label: 'Foo',
+                    operationType: 'avg',
+                    sourceField: 'bytes',
+                  },
+                },
+                incompleteColumns: {},
+              },
+            },
+            currentIndexPatternId: '1',
+          },
+          layerId: 'first',
+          columnId: 'col1',
+        })
+      ).toBeUndefined();
+    });
+
     it('should clear the incomplete column', () => {
       const state = {
         indexPatternRefs: [],

diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx
@@ -319,12 +319,15 @@ export function getIndexPatternDatasource({
     canHandleDrop,
     onDrop,
 
-    // Reset the temporary invalid state when closing the editor
+    // Reset the temporary invalid state when closing the editor, but don't
+    // update the state if it's not needed
     updateStateOnCloseDimension: ({ state, layerId, columnId }) => {
       const layer = { ...state.layers[layerId] };
-      const newIncomplete: Record<string, IncompleteColumn> = {
-        ...(state.layers[layerId].incompleteColumns || {}),
-      };
+      const current = state.layers[layerId].incompleteColumns || {};
+      if (!Object.values(current).length) {
+        return;
+      }
+      const newIncomplete: Record<string, IncompleteColumn> = { ...current };
       delete newIncomplete[columnId];
       return mergeLayer({
         state,

diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts
@@ -167,7 +167,11 @@ export interface Datasource<T = unknown, P = unknown> {
   renderLayerPanel: (domElement: Element, props: DatasourceLayerPanelProps<T>) => void;
   canHandleDrop: (props: DatasourceDimensionDropProps<T>) => boolean;
   onDrop: (props: DatasourceDimensionDropHandlerProps<T>) => false | true | { deleted: string };
-  updateStateOnCloseDimension?: (props: { layerId: string; columnId: string; state: T }) => T;
+  updateStateOnCloseDimension?: (props: {
+    layerId: string;
+    columnId: string;
+    state: T;
+  }) => T | undefined;
 
   toExpression: (state: T, layerId: string) => Ast | string | null;