[7962] Allow consumer changes to display settings after user changes have been made

cee-chen · cee-chen · commit 9cf4a4da1ff4 · 2024-10-15T22:02:30.000-07:00
- requires updating how reset button behaves, and storing initial display settings in refs on mount

+ reorder/organize code with comment blocks / by concern

- modify deep equal to potentially not cost as much / re-run isEqual as often if the object *is* correctly memoized
diff --git a/packages/eui/src/components/datagrid/controls/display_selector.test.tsx b/packages/eui/src/components/datagrid/controls/display_selector.test.tsx
@@ -545,7 +545,6 @@ describe('useDataGridDisplaySelector', () => {
 
     describe('reset button', () => {
       it('renders a reset button only when the user changes from the current settings', async () => {
-        // const component = mount(<MockComponent gridStyles={startingStyles} />);
         const { container, baseElement, getByTestSubject } = render(
           <MockComponent gridStyles={startingStyles} />
         );
@@ -582,7 +581,32 @@ describe('useDataGridDisplaySelector', () => {
         ).not.toBeInTheDocument();
       });
 
-      it('hides the reset button even after changes if allowResetButton is false', async () => {
+      it('hides the reset button if the user changes display settings back to the initial settings', async () => {
+        const { container, baseElement, getByTestSubject } = render(
+          <MockComponent gridStyles={startingStyles} />
+        );
+        openPopover(container);
+
+        await waitFor(() => {
+          userEvent.click(getByTestSubject('expanded'));
+          userEvent.click(getByTestSubject('auto'));
+        });
+
+        expect(
+          baseElement.querySelector('[data-test-subj="resetDisplaySelector"]')
+        ).toBeInTheDocument();
+
+        await waitFor(() => {
+          userEvent.click(getByTestSubject('normal'));
+          userEvent.click(getByTestSubject('undefined'));
+        });
+
+        expect(
+          container.querySelector('[data-test-subj="resetDisplaySelector"]')
+        ).not.toBeInTheDocument();
+      });
+
+      it('does not render the reset button if allowResetButton is false', async () => {
         const { container, baseElement, getByTestSubject } = render(
           <MockComponent
             showDisplaySelector={{
@@ -603,16 +627,6 @@ describe('useDataGridDisplaySelector', () => {
         expect(
           baseElement.querySelector('[data-test-subj="resetDisplaySelector"]')
         ).not.toBeInTheDocument();
-
-        // Should hide the reset button again after the popover was reopened
-        closePopover(container);
-        await waitForEuiPopoverClose();
-        openPopover(container);
-        await waitForEuiPopoverOpen();
-
-        expect(
-          baseElement.querySelector('[data-test-subj="resetDisplaySelector"]')
-        ).not.toBeInTheDocument();
       });
     });
 
@@ -658,6 +672,17 @@ describe('useDataGridDisplaySelector', () => {
               }
             `);
     });
+
+    it('updates gridStyles when consumers pass in new settings', () => {
+      const { result, rerender } = renderHook(
+        ({ gridStyles }) => useDataGridDisplaySelector(true, gridStyles),
+        { initialProps: { gridStyles: startingStyles } }
+      );
+      expect(result.current[1].border).toEqual('all');
+
+      rerender({ gridStyles: { ...startingStyles, border: 'none' } });
+      expect(result.current[1].border).toEqual('none');
+    });
   });
 
   describe('rowHeightsOptions', () => {
@@ -687,6 +712,7 @@ describe('useDataGridDisplaySelector', () => {
     const getOutput = () => {
       return JSON.parse(screen.getByTestSubject('output').textContent!);
     };
+
     describe('returns an object of rowHeightsOptions with user overrides', () => {
       it('overrides `rowHeights` and `defaultHeight`', () => {
         render(
@@ -703,6 +729,7 @@ describe('useDataGridDisplaySelector', () => {
           defaultHeight: undefined,
         });
       });
+
       it('does not override other rowHeightsOptions properties', () => {
         render(
           <MockComponent initialRowHeightsOptions={{ lineHeight: '2em' }} />
@@ -714,6 +741,21 @@ describe('useDataGridDisplaySelector', () => {
           rowHeights: {},
         });
       });
+
+      it('updates rowHeightsOptions when consumers pass in new settings', () => {
+        const initialRowHeightsOptions: EuiDataGridRowHeightsOptions = {
+          defaultHeight: 'auto',
+        };
+        const { result, rerender } = renderHook(
+          ({ rowHeightsOptions }) =>
+            useDataGridDisplaySelector(true, startingStyles, rowHeightsOptions),
+          { initialProps: { rowHeightsOptions: initialRowHeightsOptions } }
+        );
+        expect(result.current[2].defaultHeight).toEqual('auto');
+
+        rerender({ rowHeightsOptions: { defaultHeight: { lineCount: 2 } } });
+        expect(result.current[2].defaultHeight).toEqual({ lineCount: 2 });
+      });
     });
 
     it('handles undefined initialRowHeightsOptions', () => {
diff --git a/packages/eui/src/components/datagrid/controls/display_selector.tsx b/packages/eui/src/components/datagrid/controls/display_selector.tsx
@@ -12,10 +12,11 @@ import React, {
   useMemo,
   useCallback,
   useEffect,
+  useRef,
 } from 'react';
 
 import { logicalStyle, mathWithUnits } from '../../../global_styling';
-import { useUpdateEffect, useEuiTheme } from '../../../services';
+import { useUpdateEffect, useDeepEqual, useEuiTheme } from '../../../services';
 import { EuiI18n, useEuiI18n } from '../../i18n';
 import { EuiPopover, EuiPopoverFooter } from '../../popover';
 import { EuiButtonIcon, EuiButtonGroup, EuiButtonEmpty } from '../../button';
@@ -41,6 +42,7 @@ export const startingStyles: EuiDataGridStyle = {
   footer: 'overline',
   stickyFooter: true,
 };
+const emptyRowHeightsOptions: EuiDataGridRowHeightsOptions = {};
 
 /**
  * Cell density
@@ -281,8 +283,8 @@ const RowHeightControl = ({
 
 export const useDataGridDisplaySelector = (
   showDisplaySelector: EuiDataGridToolBarVisibilityOptions['showDisplaySelector'],
-  initialStyles: EuiDataGridStyle,
-  initialRowHeightsOptions?: EuiDataGridRowHeightsOptions
+  passedGridStyles: EuiDataGridStyle,
+  passedRowHeightsOptions: EuiDataGridRowHeightsOptions = emptyRowHeightsOptions
 ): [ReactNode, EuiDataGridStyle, EuiDataGridRowHeightsOptions] => {
   const [isOpen, setIsOpen] = useState(false);
 
@@ -296,79 +298,110 @@ export const useDataGridDisplaySelector = (
       ? undefined
       : showDisplaySelector?.customRender;
 
-  // Track styles specified by the user at run time
-  const [userGridStyles, setUserGridStyles] = useState({});
-  const [userRowHeightsOptions, setUserRowHeightsOptions] = useState({});
-
-  // Merge the developer-specified configurations with user overrides
-  const gridStyles = useMemo(() => {
-    return {
-      ...initialStyles,
-      ...userGridStyles,
-    };
-  }, [initialStyles, userGridStyles]);
-
-  const rowHeightsOptions = useMemo(() => {
-    return {
-      ...initialRowHeightsOptions,
-      ...userRowHeightsOptions,
-    };
-  }, [initialRowHeightsOptions, userRowHeightsOptions]);
-
-  // Invoke onChange callbacks (removing the callback value itself, so that only configuration values are returned)
-  useUpdateEffect(() => {
-    const { onChange, ...currentGridStyles } = gridStyles;
-    initialStyles?.onChange?.(currentGridStyles);
-  }, [userGridStyles]);
+  /**
+   * Grid style changes
+   */
+  const [gridStyles, setGridStyles] =
+    useState<EuiDataGridStyle>(passedGridStyles);
 
+  // Update if consumers pass new grid style configurations
+  const stablePassedGridStyles = useDeepEqual(passedGridStyles);
   useUpdateEffect(() => {
-    const { onChange, ...currentRowHeightsOptions } = rowHeightsOptions;
-    initialRowHeightsOptions?.onChange?.(currentRowHeightsOptions);
-  }, [userRowHeightsOptions]);
-
-  // Allow resetting to initial developer-specified configurations
-  const resetToInitialState = useCallback(() => {
-    setUserGridStyles({});
-    setUserRowHeightsOptions({});
-  }, []);
+    setGridStyles(stablePassedGridStyles);
+  }, [stablePassedGridStyles]);
+
+  // Update on user display selector change
+  const onUserGridStyleChange = useCallback(
+    (styles: EuiDataGridRowHeightsOptions) =>
+      setGridStyles((prevStyles) => {
+        const changedStyles = { ...prevStyles, ...styles };
+        const { onChange, ...rest } = changedStyles;
+        onChange?.(rest);
+        return changedStyles;
+      }),
+    []
+  );
 
   const densityControl = useMemo(() => {
     const show = getNestedObjectOptions(showDisplaySelector, 'allowDensity');
     return show ? (
-      <DensityControl gridStyles={gridStyles} onChange={setUserGridStyles} />
+      <DensityControl
+        gridStyles={gridStyles}
+        onChange={onUserGridStyleChange}
+      />
     ) : null;
-  }, [showDisplaySelector, gridStyles, setUserGridStyles]);
+  }, [showDisplaySelector, gridStyles, onUserGridStyleChange]);
+
+  /**
+   * Row height changes
+   */
+  const [rowHeightsOptions, setRowHeightsOptions] =
+    useState<EuiDataGridRowHeightsOptions>(passedRowHeightsOptions);
+
+  // Update if consumers pass new row height configurations
+  const stablePassedRowHeights = useDeepEqual(passedRowHeightsOptions);
+  useUpdateEffect(() => {
+    setRowHeightsOptions(stablePassedRowHeights);
+  }, [stablePassedRowHeights]);
+
+  // Update on user display selector change
+  const onUserRowHeightChange = useCallback(
+    (options: EuiDataGridRowHeightsOptions) =>
+      setRowHeightsOptions((prevOptions) => {
+        const changedOptions = { ...prevOptions, ...options };
+        const { onChange, ...rest } = changedOptions;
+        onChange?.(rest);
+        return changedOptions;
+      }),
+    []
+  );
 
   const rowHeightControl = useMemo(() => {
     const show = getNestedObjectOptions(showDisplaySelector, 'allowRowHeight');
     return show ? (
       <RowHeightControl
         rowHeightsOptions={rowHeightsOptions}
-        onChange={setUserRowHeightsOptions}
+        onChange={onUserRowHeightChange}
       />
     ) : null;
-  }, [showDisplaySelector, rowHeightsOptions, setUserRowHeightsOptions]);
+  }, [showDisplaySelector, rowHeightsOptions, onUserRowHeightChange]);
+
+  /**
+   * Reset button
+   */
+  const [showResetButton, setShowResetButton] = useState(false);
+  const initialGridStyles = useRef<EuiDataGridStyle>(passedGridStyles);
+  const initialRowHeightsOptions = useRef<EuiDataGridRowHeightsOptions>(
+    passedRowHeightsOptions
+  );
+
+  const resetToInitialState = useCallback(() => {
+    setGridStyles(initialGridStyles.current);
+    setRowHeightsOptions(initialRowHeightsOptions.current);
+  }, []);
+
+  useUpdateEffect(() => {
+    setShowResetButton(
+      rowHeightsOptions.defaultHeight !==
+        initialRowHeightsOptions.current.defaultHeight ||
+        gridStyles.fontSize !== initialGridStyles.current.fontSize ||
+        gridStyles.cellPadding !== initialGridStyles.current.cellPadding
+    );
+  }, [
+    rowHeightsOptions.defaultHeight,
+    gridStyles.fontSize,
+    gridStyles.cellPadding,
+  ]);
 
   const resetButton = useMemo(() => {
-    const show = getNestedObjectOptions(
+    const allowed = getNestedObjectOptions(
       showDisplaySelector,
       'allowResetButton'
     );
-    if (!show) return null;
-
-    const hasUserChanges =
-      Object.keys(userGridStyles).length > 0 ||
-      Object.keys(userRowHeightsOptions).length > 0;
+    if (!allowed || !showResetButton) return null;
 
-    return hasUserChanges ? (
-      <ResetButton onClick={resetToInitialState} />
-    ) : null;
-  }, [
-    showDisplaySelector,
-    resetToInitialState,
-    userGridStyles,
-    userRowHeightsOptions,
-  ]);
+    return <ResetButton onClick={resetToInitialState} />;
+  }, [showDisplaySelector, showResetButton, resetToInitialState]);
 
   const buttonLabel = useEuiI18n(
     'euiDisplaySelector.buttonText',
diff --git a/packages/eui/src/services/hooks/useDeepEqual.ts b/packages/eui/src/services/hooks/useDeepEqual.ts
@@ -6,8 +6,9 @@
  * Side Public License, v 1.
  */
 
-import { useRef } from 'react';
+import { useState } from 'react';
 import isEqual from 'lodash/isEqual';
+import { useUpdateEffect } from './useUpdateEffect';
 
 /**
  * This hook is mostly a performance concern for third-party objs/arrays that EUI
@@ -17,11 +18,13 @@ import isEqual from 'lodash/isEqual';
 export const useDeepEqual = <T = Record<string, any> | any[] | undefined>(
   object: T
 ): T => {
-  const ref = useRef(object);
+  const [memoizedObject, setMemoizedObject] = useState(object);
 
-  if (!isEqual(object, ref.current)) {
-    ref.current = object;
-  }
+  useUpdateEffect(() => {
+    if (!isEqual(object, memoizedObject)) {
+      setMemoizedObject(object);
+    }
+  }, [object]);
 
-  return ref.current;
+  return memoizedObject;
 };
