[8.x] [kbn-expandable-flyout] - refactor push/overlay to use redux in…

…stead of hooks (#192745) (#193225) # Backport This will backport the following commits from `main` to `8.x`: - [[kbn-expandable-flyout] - refactor push/overlay to use redux instead of hooks (#192745)](#192745)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Philippe Oberti <[email protected]>
elastic · Sep 17, 2024 · 60f5269 · 60f5269
1 parent 664d067
commit 60f5269
Show file tree

Hide file tree

Showing 20 changed files with 490 additions and 189 deletions.
diff --git a/packages/kbn-expandable-flyout/src/components/preview_section.test.tsx b/packages/kbn-expandable-flyout/src/components/preview_section.test.tsx
@@ -33,6 +33,9 @@ describe('PreviewSection', () => {
         },
       },
     },
+    ui: {
+      pushVsOverlay: 'overlay',
+    },
   };
 
   const component = <div>{'component'}</div>;

diff --git a/packages/kbn-expandable-flyout/src/components/settings_menu.test.tsx b/packages/kbn-expandable-flyout/src/components/settings_menu.test.tsx
@@ -11,26 +11,39 @@ import React from 'react';
 import { render } from '@testing-library/react';
 
 import { SettingsMenu } from './settings_menu';
-import { EuiFlyoutProps } from '@elastic/eui';
 import {
   SETTINGS_MENU_BUTTON_TEST_ID,
+  SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_OVERLAY_TEST_ID,
   SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_PUSH_TEST_ID,
   SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_TEST_ID,
   SETTINGS_MENU_FLYOUT_TYPE_INFORMATION_ICON_TEST_ID,
   SETTINGS_MENU_FLYOUT_TYPE_TITLE_TEST_ID,
 } from './test_ids';
+import { TestProvider } from '../test/provider';
+import { localStorageMock } from '../../__mocks__';
+import { EXPANDABLE_FLYOUT_LOCAL_STORAGE, PUSH_VS_OVERLAY_LOCAL_STORAGE } from '../constants';
+import { initialPanelsState } from '../store/state';
 
 describe('SettingsMenu', () => {
+  beforeEach(() => {
+    Object.defineProperty(window, 'localStorage', {
+      value: localStorageMock(),
+    });
+  });
+
   it('should render the flyout type button group', () => {
-    const flyoutTypeProps = {
-      type: 'overlay' as EuiFlyoutProps['type'],
-      onChange: jest.fn(),
-      disabled: false,
-      tooltip: '',
+    const flyoutCustomProps = {
+      hideSettings: false,
+      pushVsOverlay: {
+        disabled: false,
+        tooltip: '',
+      },
     };
 
     const { getByTestId, queryByTestId } = render(
-      <SettingsMenu flyoutTypeProps={flyoutTypeProps} />
+      <TestProvider>
+        <SettingsMenu flyoutCustomProps={flyoutCustomProps} />
+      </TestProvider>
     );
 
     getByTestId(SETTINGS_MENU_BUTTON_TEST_ID).click();
@@ -42,48 +55,115 @@ describe('SettingsMenu', () => {
     expect(getByTestId(SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_TEST_ID)).toBeInTheDocument();
   });
 
+  it('should have the type selected if option is enabled', () => {
+    const state = {
+      panels: initialPanelsState,
+      ui: {
+        pushVsOverlay: 'push' as const,
+      },
+    };
+    const flyoutCustomProps = {
+      hideSettings: false,
+      pushVsOverlay: {
+        disabled: false,
+        tooltip: '',
+      },
+    };
+
+    const { getByTestId } = render(
+      <TestProvider state={state}>
+        <SettingsMenu flyoutCustomProps={flyoutCustomProps} />
+      </TestProvider>
+    );
+
+    getByTestId(SETTINGS_MENU_BUTTON_TEST_ID).click();
+
+    expect(getByTestId(SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_PUSH_TEST_ID)).toHaveClass(
+      'euiButtonGroupButton-isSelected'
+    );
+    expect(getByTestId(SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_OVERLAY_TEST_ID)).not.toHaveClass(
+      'euiButtonGroupButton-isSelected'
+    );
+  });
+
   it('should select correct the flyout type', () => {
-    const onChange = jest.fn();
-    const flyoutTypeProps = {
-      type: 'overlay' as EuiFlyoutProps['type'],
-      onChange,
-      disabled: false,
-      tooltip: '',
+    const flyoutCustomProps = {
+      hideSettings: false,
+      pushVsOverlay: {
+        disabled: false,
+        tooltip: '',
+      },
     };
 
-    const { getByTestId } = render(<SettingsMenu flyoutTypeProps={flyoutTypeProps} />);
+    const { getByTestId } = render(
+      <TestProvider>
+        <SettingsMenu flyoutCustomProps={flyoutCustomProps} />
+      </TestProvider>
+    );
+
+    expect(localStorage.getItem(EXPANDABLE_FLYOUT_LOCAL_STORAGE)).toEqual(null);
 
     getByTestId(SETTINGS_MENU_BUTTON_TEST_ID).click();
     getByTestId(SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_PUSH_TEST_ID).click();
 
-    expect(onChange).toHaveBeenCalledWith('push');
+    expect(localStorage.getItem(EXPANDABLE_FLYOUT_LOCAL_STORAGE)).toEqual(
+      JSON.stringify({ [PUSH_VS_OVERLAY_LOCAL_STORAGE]: 'push' })
+    );
+
+    getByTestId(SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_OVERLAY_TEST_ID).click();
+
+    expect(localStorage.getItem(EXPANDABLE_FLYOUT_LOCAL_STORAGE)).toEqual(
+      JSON.stringify({ [PUSH_VS_OVERLAY_LOCAL_STORAGE]: 'overlay' })
+    );
   });
 
   it('should render the the flyout type button group disabled', () => {
-    const flyoutTypeProps = {
-      type: 'overlay' as EuiFlyoutProps['type'],
-      onChange: jest.fn(),
-      disabled: true,
-      tooltip: 'This option is disabled',
+    const flyoutCustomProps = {
+      hideSettings: false,
+      pushVsOverlay: {
+        disabled: true,
+        tooltip: 'This option is disabled',
+      },
     };
 
-    const { getByTestId } = render(<SettingsMenu flyoutTypeProps={flyoutTypeProps} />);
+    const { getByTestId } = render(
+      <TestProvider>
+        <SettingsMenu flyoutCustomProps={flyoutCustomProps} />
+      </TestProvider>
+    );
+
+    expect(localStorage.getItem(EXPANDABLE_FLYOUT_LOCAL_STORAGE)).toEqual(null);
 
     getByTestId(SETTINGS_MENU_BUTTON_TEST_ID).click();
     expect(getByTestId(SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_TEST_ID)).toHaveAttribute('disabled');
+
     expect(getByTestId(SETTINGS_MENU_FLYOUT_TYPE_INFORMATION_ICON_TEST_ID)).toBeInTheDocument();
+
+    expect(getByTestId(SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_OVERLAY_TEST_ID)).toHaveClass(
+      'euiButtonGroupButton-isSelected'
+    );
+    expect(getByTestId(SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_PUSH_TEST_ID)).not.toHaveClass(
+      'euiButtonGroupButton-isSelected'
+    );
+
+    getByTestId(SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_PUSH_TEST_ID).click();
+
+    expect(localStorage.getItem(EXPANDABLE_FLYOUT_LOCAL_STORAGE)).toEqual(null);
   });
 
   it('should not render the information icon if the tooltip is empty', () => {
-    const flyoutTypeProps = {
-      type: 'overlay' as EuiFlyoutProps['type'],
-      onChange: jest.fn(),
-      disabled: true,
-      tooltip: '',
+    const flyoutCustomProps = {
+      hideSettings: false,
+      pushVsOverlay: {
+        disabled: true,
+        tooltip: '',
+      },
     };
 
     const { getByTestId, queryByTestId } = render(
-      <SettingsMenu flyoutTypeProps={flyoutTypeProps} />
+      <TestProvider>
+        <SettingsMenu flyoutCustomProps={flyoutCustomProps} />
+      </TestProvider>
     );
 
     getByTestId(SETTINGS_MENU_BUTTON_TEST_ID).click();

diff --git a/packages/kbn-expandable-flyout/src/components/settings_menu.tsx b/packages/kbn-expandable-flyout/src/components/settings_menu.tsx
@@ -22,6 +22,7 @@ import {
 import { css } from '@emotion/css';
 import React, { memo, useCallback, useState } from 'react';
 import { i18n } from '@kbn/i18n';
+import { changePushVsOverlayAction } from '../store/actions';
 import {
   SETTINGS_MENU_BUTTON_TEST_ID,
   SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_OVERLAY_TEST_ID,
@@ -30,6 +31,7 @@ import {
   SETTINGS_MENU_FLYOUT_TYPE_INFORMATION_ICON_TEST_ID,
   SETTINGS_MENU_FLYOUT_TYPE_TITLE_TEST_ID,
 } from './test_ids';
+import { selectPushVsOverlay, useDispatch, useSelector } from '../store/redux';
 
 const SETTINGS_MENU_ICON_BUTTON = i18n.translate('expandableFlyout.settingsMenu.popoverButton', {
   defaultMessage: 'Open flyout settings menu',
@@ -59,48 +61,62 @@ const FLYOUT_TYPE_PUSH_TOOLTIP = i18n.translate('expandableFlyout.settingsMenu.p
   defaultMessage: 'Displays the flyout next to the page',
 });
 
-interface SettingsMenuProps {
+export interface FlyoutCustomProps {
   /**
-   * Current flyout type
+   * Hide the gear icon and settings menu if true
    */
-  flyoutTypeProps: {
-    /**
-     * 'push' or 'overlay'
-     */
-    type: EuiFlyoutProps['type'];
-    /**
-     * Callback to change the flyout type
-     */
-    onChange: (type: EuiFlyoutProps['type']) => void;
+  hideSettings?: boolean;
+  /**
+   * Control if the option to render in overlay or push mode is enabled or not
+   */
+  pushVsOverlay?: {
     /**
-     * Disables the button group for flyout where the option shouldn't be available
+     * Disables the option
      */
     disabled: boolean;
     /**
-     * Allows to show a tooltip to explain why the option is disabled
+     * Tooltip to display
      */
     tooltip: string;
   };
 }
 
+export interface SettingsMenuProps {
+  /**
+   * Custom props to populate the content of the settings meny
+   */
+  flyoutCustomProps?: FlyoutCustomProps;
+}
+
 /**
  * Renders a menu to allow the user to customize the flyout.
  * Current customization are:
  * - Flyout type: overlay or push
  */
 export const SettingsMenu: React.FC<SettingsMenuProps> = memo(
-  ({ flyoutTypeProps }: SettingsMenuProps) => {
+  ({ flyoutCustomProps }: SettingsMenuProps) => {
+    const dispatch = useDispatch();
+
+    // for flyout where the push vs overlay option is disable in the UI we fall back to overlay mode
+    const type = useSelector(selectPushVsOverlay);
+    const flyoutType = flyoutCustomProps?.pushVsOverlay?.disabled ? 'overlay' : type;
+
     const [isPopoverOpen, setPopover] = useState(false);
     const togglePopover = () => {
       setPopover(!isPopoverOpen);
     };
 
     const pushVsOverlayOnChange = useCallback(
       (id: string) => {
-        flyoutTypeProps.onChange(id as EuiFlyoutProps['type']);
+        dispatch(
+          changePushVsOverlayAction({
+            type: id as EuiFlyoutProps['type'] as 'overlay' | 'push',
+            savedToLocalStorage: !flyoutCustomProps?.pushVsOverlay?.disabled,
+          })
+        );
         setPopover(false);
       },
-      [flyoutTypeProps]
+      [dispatch, flyoutCustomProps?.pushVsOverlay?.disabled]
     );
 
     const panels = [
@@ -112,8 +128,8 @@ export const SettingsMenu: React.FC<SettingsMenuProps> = memo(
             <EuiTitle size="xxs" data-test-subj={SETTINGS_MENU_FLYOUT_TYPE_TITLE_TEST_ID}>
               <h3>
                 {FLYOUT_TYPE_TITLE}{' '}
-                {flyoutTypeProps.tooltip && (
-                  <EuiToolTip position="top" content={flyoutTypeProps.tooltip}>
+                {flyoutCustomProps?.pushVsOverlay?.tooltip && (
+                  <EuiToolTip position="top" content={flyoutCustomProps?.pushVsOverlay?.tooltip}>
                     <EuiIcon
                       data-test-subj={SETTINGS_MENU_FLYOUT_TYPE_INFORMATION_ICON_TEST_ID}
                       type="iInCircle"
@@ -142,9 +158,9 @@ export const SettingsMenu: React.FC<SettingsMenuProps> = memo(
                   toolTipContent: FLYOUT_TYPE_PUSH_TOOLTIP,
                 },
               ]}
-              idSelected={flyoutTypeProps.type as string}
+              idSelected={flyoutType}
               onChange={pushVsOverlayOnChange}
-              isDisabled={flyoutTypeProps.disabled}
+              isDisabled={flyoutCustomProps?.pushVsOverlay?.disabled}
               data-test-subj={SETTINGS_MENU_FLYOUT_TYPE_BUTTON_GROUP_TEST_ID}
             />
           </EuiPanel>

diff --git a/packages/kbn-expandable-flyout/src/constants.ts b/packages/kbn-expandable-flyout/src/constants.ts
@@ -11,3 +11,6 @@
  * This is a reserved word that we use as an id when no urlKey is provided and we are in memory storage mode
  */
 export const REDUX_ID_FOR_MEMORY_STORAGE = 'memory';
+
+export const EXPANDABLE_FLYOUT_LOCAL_STORAGE = 'expandableFlyout.ui';
+export const PUSH_VS_OVERLAY_LOCAL_STORAGE = 'pushVsOverlay';
diff --git a/packages/kbn-expandable-flyout/src/hooks/use_flyout_type.test.ts b/packages/kbn-expandable-flyout/src/hooks/use_flyout_type.test.ts
-Original file line number
+Diff line change
@@ Expand Up / @@ -33,6 +33,9 @@ describe('PreviewSection', () => { @@
             },
           },
         },
+        ui: {
+          pushVsOverlay: 'overlay',
+        },
       };
       const component = <div>{'component'}</div>;
@@ Expand Down @@