From 31c9e75f56fefb18d6ad1e8e53e75c2117c6c343 Mon Sep 17 00:00:00 2001 From: Philippe Oberti Date: Tue, 17 Sep 2024 22:15:40 +0200 Subject: [PATCH] [kbn-expandable-flyout] - refactor push/overlay to use redux instead of hooks (#192745) --- .../src/components/preview_section.test.tsx | 3 + .../src/components/settings_menu.test.tsx | 134 ++++++++++++++---- .../src/components/settings_menu.tsx | 56 +++++--- .../kbn-expandable-flyout/src/constants.ts | 3 + .../src/hooks/use_flyout_type.test.ts | 50 ------- .../src/hooks/use_flyout_type.ts | 60 -------- .../use_initialize_from_local_storage.test.ts | 73 ++++++++++ .../use_initialize_from_local_storage.ts | 32 +++++ .../src/index.stories.tsx | 21 +++ .../kbn-expandable-flyout/src/index.test.tsx | 18 +++ packages/kbn-expandable-flyout/src/index.tsx | 38 ++--- .../src/provider.test.tsx | 9 ++ .../src/store/actions.ts | 14 ++ .../src/store/middlewares.test.ts | 59 ++++++++ .../src/store/middlewares.ts | 33 +++++ .../src/store/reducers.test.ts | 37 ++++- .../src/store/reducers.ts | 9 +- .../kbn-expandable-flyout/src/store/redux.ts | 8 +- .../kbn-expandable-flyout/src/store/state.ts | 16 +++ .../src/test/provider.tsx | 6 +- 20 files changed, 490 insertions(+), 189 deletions(-) delete mode 100644 packages/kbn-expandable-flyout/src/hooks/use_flyout_type.test.ts delete mode 100644 packages/kbn-expandable-flyout/src/hooks/use_flyout_type.ts create mode 100644 packages/kbn-expandable-flyout/src/hooks/use_initialize_from_local_storage.test.ts create mode 100644 packages/kbn-expandable-flyout/src/hooks/use_initialize_from_local_storage.ts create mode 100644 packages/kbn-expandable-flyout/src/store/middlewares.test.ts create mode 100644 packages/kbn-expandable-flyout/src/store/middlewares.ts diff --git a/packages/kbn-expandable-flyout/src/components/preview_section.test.tsx b/packages/kbn-expandable-flyout/src/components/preview_section.test.tsx index ba2b8987cc0a8..9916f2e784dfa 100644 --- 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 =
{'component'}
; diff --git a/packages/kbn-expandable-flyout/src/components/settings_menu.test.tsx b/packages/kbn-expandable-flyout/src/components/settings_menu.test.tsx index bfcea23d52649..9ef33a649671d 100644 --- 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( - + + + ); 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( + + + + ); + + 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(); + const { getByTestId } = render( + + + + ); + + 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(); + const { getByTestId } = render( + + + + ); + + 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( - + + + ); 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 index 8e4b4bc997d9f..7229921bfdd39 100644 --- 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,37 +61,46 @@ 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 = 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); @@ -97,10 +108,15 @@ export const SettingsMenu: React.FC = memo( 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 = memo(

{FLYOUT_TYPE_TITLE}{' '} - {flyoutTypeProps.tooltip && ( - + {flyoutCustomProps?.pushVsOverlay?.tooltip && ( + = 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} /> diff --git a/packages/kbn-expandable-flyout/src/constants.ts b/packages/kbn-expandable-flyout/src/constants.ts index 267cdf1f831aa..7ec81a9de4b67 100644 --- 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 deleted file mode 100644 index 5ec65b15077de..0000000000000 --- a/packages/kbn-expandable-flyout/src/hooks/use_flyout_type.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -import { renderHook } from '@testing-library/react-hooks'; -import { useExpandableFlyoutContext } from '../context'; -import { useFlyoutType } from './use_flyout_type'; -import { localStorageMock } from '../../__mocks__'; - -jest.mock('../context'); - -describe('useFlyoutType', () => { - beforeEach(() => { - Object.defineProperty(window, 'localStorage', { - value: localStorageMock(), - }); - }); - - it('should return the value in localStorage if set', () => { - (useExpandableFlyoutContext as jest.Mock).mockReturnValue({ urlKey: 'flyout' }); - - localStorage.setItem('expandableFlyout.pushVsOverlayMode.flyout', 'push'); - - const hookResult = renderHook(() => useFlyoutType()); - - expect(hookResult.result.current.flyoutType).toEqual('push'); - }); - - it('should return overlay if nothing is set in localStorage', () => { - (useExpandableFlyoutContext as jest.Mock).mockReturnValue({ urlKey: 'flyout' }); - - const hookResult = renderHook(() => useFlyoutType()); - - expect(hookResult.result.current.flyoutType).toEqual('overlay'); - }); - - it('should set value in localStorage', () => { - (useExpandableFlyoutContext as jest.Mock).mockReturnValue({ urlKey: 'flyout' }); - - const hookResult = renderHook(() => useFlyoutType()); - - hookResult.result.current.flyoutTypeChange('push'); - expect(localStorage.getItem('expandableFlyout.pushVsOverlayMode.flyout')).toEqual('push'); - }); -}); diff --git a/packages/kbn-expandable-flyout/src/hooks/use_flyout_type.ts b/packages/kbn-expandable-flyout/src/hooks/use_flyout_type.ts deleted file mode 100644 index e3c824f4132e0..0000000000000 --- a/packages/kbn-expandable-flyout/src/hooks/use_flyout_type.ts +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -import { useCallback, useMemo, useState } from 'react'; -import { EuiFlyoutProps } from '@elastic/eui'; -import { useExpandableFlyoutContext } from '../context'; - -const expandableFlyoutLocalStorageKey = 'expandableFlyout.'; -const pushVsOverlayModeLocalStorageKey = 'pushVsOverlayMode.'; - -export interface UseFlyoutTypeResult { - /** - * The current flyout type - */ - flyoutType: EuiFlyoutProps['type']; - /** - * Callback to change the flyout type - */ - flyoutTypeChange: (flyoutType: EuiFlyoutProps['type']) => void; -} - -/** - * Hook to store and retrieve the flyout type (push vs overlay) from local storage. - * The key is generated based on the current URL key. - */ -export const useFlyoutType = (): UseFlyoutTypeResult => { - const { urlKey } = useExpandableFlyoutContext(); - const pushVsOverlayLocalStorageKey = useMemo( - () => `${expandableFlyoutLocalStorageKey}${pushVsOverlayModeLocalStorageKey}${urlKey}`, - [urlKey] - ); - - const initialFlyoutType: EuiFlyoutProps['type'] = - (localStorage.getItem(pushVsOverlayLocalStorageKey) as EuiFlyoutProps['type']) || 'overlay'; - - const [flyoutType, setFlyoutType] = useState(initialFlyoutType); - - const flyoutTypeChange = useCallback( - (type: EuiFlyoutProps['type']) => { - // we only save to localStorage the value for flyouts that have a urlKey. - // The state of the memory flyout is not persisted. - if (urlKey && type) { - localStorage.setItem(pushVsOverlayLocalStorageKey, type); - } - setFlyoutType(type); - }, - [pushVsOverlayLocalStorageKey, setFlyoutType, urlKey] - ); - - return { - flyoutType, - flyoutTypeChange, - }; -}; diff --git a/packages/kbn-expandable-flyout/src/hooks/use_initialize_from_local_storage.test.ts b/packages/kbn-expandable-flyout/src/hooks/use_initialize_from_local_storage.test.ts new file mode 100644 index 0000000000000..70cc4f31f2636 --- /dev/null +++ b/packages/kbn-expandable-flyout/src/hooks/use_initialize_from_local_storage.test.ts @@ -0,0 +1,73 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { renderHook } from '@testing-library/react-hooks'; +import { useInitializeFromLocalStorage } from './use_initialize_from_local_storage'; +import { localStorageMock } from '../../__mocks__'; +import { EXPANDABLE_FLYOUT_LOCAL_STORAGE, PUSH_VS_OVERLAY_LOCAL_STORAGE } from '../constants'; +import { useDispatch } from '../store/redux'; +import { changePushVsOverlayAction } from '../store/actions'; + +jest.mock('../store/redux'); + +describe('useInitializeFromLocalStorage', () => { + beforeEach(() => { + Object.defineProperty(window, 'localStorage', { + value: localStorageMock(), + }); + }); + + // if this test fails, it's very likely because the data format of the values saved in local storage + // has changed and we might need to run a migration + it('should retrieve push/overlay value from local storage', () => { + const mockUseDispatch = jest.fn(); + (useDispatch as jest.Mock).mockImplementation(() => mockUseDispatch); + + localStorage.setItem( + EXPANDABLE_FLYOUT_LOCAL_STORAGE, + JSON.stringify({ + [PUSH_VS_OVERLAY_LOCAL_STORAGE]: 'push', + }) + ); + + renderHook(() => useInitializeFromLocalStorage()); + + expect(mockUseDispatch).toHaveBeenCalledWith( + changePushVsOverlayAction({ + type: 'push', + savedToLocalStorage: false, + }) + ); + }); + + it('should not dispatch action if expandable flyout key is not present in local storage', () => { + const mockUseDispatch = jest.fn(); + (useDispatch as jest.Mock).mockImplementation(() => mockUseDispatch); + + localStorage.setItem( + EXPANDABLE_FLYOUT_LOCAL_STORAGE, + JSON.stringify({ + wrong_key: 'push', + }) + ); + + renderHook(() => useInitializeFromLocalStorage()); + + expect(mockUseDispatch).not.toHaveBeenCalled(); + }); + + it('should not dispatch action if expandable flyout key is present in local storage but not push/overlay', () => { + const mockUseDispatch = jest.fn(); + (useDispatch as jest.Mock).mockImplementation(() => mockUseDispatch); + + renderHook(() => useInitializeFromLocalStorage()); + + expect(mockUseDispatch).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/kbn-expandable-flyout/src/hooks/use_initialize_from_local_storage.ts b/packages/kbn-expandable-flyout/src/hooks/use_initialize_from_local_storage.ts new file mode 100644 index 0000000000000..7af92a726a394 --- /dev/null +++ b/packages/kbn-expandable-flyout/src/hooks/use_initialize_from_local_storage.ts @@ -0,0 +1,32 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { EXPANDABLE_FLYOUT_LOCAL_STORAGE, PUSH_VS_OVERLAY_LOCAL_STORAGE } from '../constants'; +import { useDispatch } from '../store/redux'; +import { changePushVsOverlayAction } from '../store/actions'; + +/** + * Hook to initialize the push vs overlay redux state from local storage + */ +export const useInitializeFromLocalStorage = () => { + const dispatch = useDispatch(); + + const expandableFlyout = localStorage.getItem(EXPANDABLE_FLYOUT_LOCAL_STORAGE); + if (!expandableFlyout) return; + + const pushVsOverlay = JSON.parse(expandableFlyout)[PUSH_VS_OVERLAY_LOCAL_STORAGE]; + if (pushVsOverlay) { + dispatch( + changePushVsOverlayAction({ + type: pushVsOverlay as 'push' | 'overlay', + savedToLocalStorage: false, + }) + ); + } +}; diff --git a/packages/kbn-expandable-flyout/src/index.stories.tsx b/packages/kbn-expandable-flyout/src/index.stories.tsx index a7b1e95e43805..6e6e7207d8f15 100644 --- a/packages/kbn-expandable-flyout/src/index.stories.tsx +++ b/packages/kbn-expandable-flyout/src/index.stories.tsx @@ -114,6 +114,9 @@ export const Right: Story = () => { }, }, }, + ui: { + pushVsOverlay: 'overlay', + }, }; return ( @@ -141,6 +144,9 @@ export const Left: Story = () => { }, }, }, + ui: { + pushVsOverlay: 'overlay', + }, }; return ( @@ -172,6 +178,9 @@ export const Preview: Story = () => { }, }, }, + ui: { + pushVsOverlay: 'overlay', + }, }; return ( @@ -206,6 +215,9 @@ export const MultiplePreviews: Story = () => { }, }, }, + ui: { + pushVsOverlay: 'overlay', + }, }; return ( @@ -231,6 +243,9 @@ export const CollapsedPushVsOverlay: Story = () => { }, }, }, + ui: { + pushVsOverlay: 'push', + }, }; return ( @@ -255,6 +270,9 @@ export const ExpandedPushVsOverlay: Story = () => { }, }, }, + ui: { + pushVsOverlay: 'push', + }, }; return ( @@ -279,6 +297,9 @@ export const DisableTypeSelection: Story = () => { }, }, }, + ui: { + pushVsOverlay: 'overlay', + }, }; return ( diff --git a/packages/kbn-expandable-flyout/src/index.test.tsx b/packages/kbn-expandable-flyout/src/index.test.tsx index 14146e2da4541..f465a16501761 100644 --- a/packages/kbn-expandable-flyout/src/index.test.tsx +++ b/packages/kbn-expandable-flyout/src/index.test.tsx @@ -36,6 +36,9 @@ describe('ExpandableFlyout', () => { panels: { byId: {}, }, + ui: { + pushVsOverlay: 'overlay', + }, }; const result = render( @@ -60,6 +63,9 @@ describe('ExpandableFlyout', () => { }, }, }, + ui: { + pushVsOverlay: 'overlay' as const, + }, }; const { getByTestId } = render( @@ -84,6 +90,9 @@ describe('ExpandableFlyout', () => { }, }, }, + ui: { + pushVsOverlay: 'overlay' as const, + }, }; const { getByTestId } = render( @@ -110,6 +119,9 @@ describe('ExpandableFlyout', () => { }, }, }, + ui: { + pushVsOverlay: 'overlay' as const, + }, }; const { getByTestId } = render( @@ -134,6 +146,9 @@ describe('ExpandableFlyout', () => { }, }, }, + ui: { + pushVsOverlay: 'overlay' as const, + }, }; const { queryByTestId } = render( @@ -159,6 +174,9 @@ describe('ExpandableFlyout', () => { }, }, }, + ui: { + pushVsOverlay: 'overlay' as const, + }, }; const { getByTestId } = render( diff --git a/packages/kbn-expandable-flyout/src/index.tsx b/packages/kbn-expandable-flyout/src/index.tsx index 32af7128be9c1..4904661b2da88 100644 --- a/packages/kbn-expandable-flyout/src/index.tsx +++ b/packages/kbn-expandable-flyout/src/index.tsx @@ -11,8 +11,8 @@ import React, { useMemo } from 'react'; import type { Interpolation, Theme } from '@emotion/react'; import { EuiFlyoutProps } from '@elastic/eui'; import { EuiFlexGroup, EuiFlyout } from '@elastic/eui'; -import { useFlyoutType } from './hooks/use_flyout_type'; -import { SettingsMenu } from './components/settings_menu'; +import { useInitializeFromLocalStorage } from './hooks/use_initialize_from_local_storage'; +import { FlyoutCustomProps, SettingsMenu } from './components/settings_menu'; import { useSectionSizes } from './hooks/use_sections_sizes'; import { useWindowSize } from './hooks/use_window_size'; import { useExpandableFlyoutState } from './hooks/use_expandable_flyout_state'; @@ -22,6 +22,7 @@ import { RightSection } from './components/right_section'; import type { FlyoutPanelProps, Panel } from './types'; import { LeftSection } from './components/left_section'; import { isPreviewBanner } from './components/preview_section'; +import { selectPushVsOverlay, useSelector } from './store/redux'; const flyoutInnerStyles = { height: '100%' }; @@ -41,19 +42,7 @@ export interface ExpandableFlyoutProps extends Omit { /** * Set of properties that drive a settings menu */ - flyoutCustomProps?: { - /** - * Hide the gear icon and settings menu if true - */ - hideSettings?: boolean; - /** - * Control if the option to render in overlay or push mode is enabled or not - */ - pushVsOverlay?: { - disabled: boolean; - tooltip: string; - }; - }; + flyoutCustomProps?: FlyoutCustomProps; } /** @@ -70,7 +59,13 @@ export const ExpandableFlyout: React.FC = ({ ...flyoutProps }) => { const windowWidth = useWindowSize(); - const { flyoutType, flyoutTypeChange } = useFlyoutType(); + + useInitializeFromLocalStorage(); + + // 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 { left, right, preview } = useExpandableFlyoutState(); const { closeFlyout } = useExpandableFlyoutApi(); @@ -156,16 +151,7 @@ export const ExpandableFlyout: React.FC = ({ /> ) : null} - {!flyoutCustomProps?.hideSettings && ( - - )} + {!flyoutCustomProps?.hideSettings && } ); }; diff --git a/packages/kbn-expandable-flyout/src/provider.test.tsx b/packages/kbn-expandable-flyout/src/provider.test.tsx index 5aa386090aa30..bdd4183c53276 100644 --- a/packages/kbn-expandable-flyout/src/provider.test.tsx +++ b/packages/kbn-expandable-flyout/src/provider.test.tsx @@ -38,6 +38,9 @@ describe('UrlSynchronizer', () => { }, needsSync: true, }, + ui: { + pushVsOverlay: 'overlay', + }, }; render( @@ -61,6 +64,9 @@ describe('UrlSynchronizer', () => { byId: {}, needsSync: true, }, + ui: { + pushVsOverlay: 'overlay', + }, }; render( @@ -95,6 +101,9 @@ describe('UrlSynchronizer', () => { }, needsSync: true, }, + ui: { + pushVsOverlay: 'overlay', + }, }; render( diff --git a/packages/kbn-expandable-flyout/src/store/actions.ts b/packages/kbn-expandable-flyout/src/store/actions.ts index 237a3d0226b05..2886118369b0e 100644 --- a/packages/kbn-expandable-flyout/src/store/actions.ts +++ b/packages/kbn-expandable-flyout/src/store/actions.ts @@ -21,6 +21,8 @@ export enum ActionType { previousPreviewPanel = 'previous_preview_panel', closeFlyout = 'close_flyout', urlChanged = 'urlChanged', + + changePushVsOverlay = 'change_push_overlay', } export const openPanelsAction = createAction<{ @@ -120,3 +122,15 @@ export const urlChangedAction = createAction<{ */ id: string; }>(ActionType.urlChanged); + +export const changePushVsOverlayAction = createAction<{ + /** + * Type of flyout to render, value and only be 'push' or 'overlay' + */ + type: 'push' | 'overlay'; + /** + * Used in the redux middleware to decide if the value needs to be saved to local storage. + * This is used to avoid saving the value to local storage when the value is changed by code instead of by a user action. + */ + savedToLocalStorage: boolean; +}>(ActionType.changePushVsOverlay); diff --git a/packages/kbn-expandable-flyout/src/store/middlewares.test.ts b/packages/kbn-expandable-flyout/src/store/middlewares.test.ts new file mode 100644 index 0000000000000..ccbb5d5443db7 --- /dev/null +++ b/packages/kbn-expandable-flyout/src/store/middlewares.test.ts @@ -0,0 +1,59 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { localStorageMock } from '../../__mocks__'; +import { EXPANDABLE_FLYOUT_LOCAL_STORAGE, PUSH_VS_OVERLAY_LOCAL_STORAGE } from '../constants'; +import { savePushVsOverlayToLocalStorageMiddleware } from './middlewares'; +import { createAction, type MiddlewareAPI } from '@reduxjs/toolkit'; +import { changePushVsOverlayAction } from './actions'; + +const noTypeAction = createAction<{ + type: 'no_type'; +}>('no_type_action'); +const randomAction = createAction<{ + type: 'random_type'; +}>('random_action'); + +describe('pushVsOverlayMiddleware', () => { + beforeEach(() => { + Object.defineProperty(window, 'localStorage', { + value: localStorageMock(), + }); + }); + + it('should ignore action without type', () => { + savePushVsOverlayToLocalStorageMiddleware({} as MiddlewareAPI)(jest.fn)(noTypeAction); + + expect(localStorage.getItem(EXPANDABLE_FLYOUT_LOCAL_STORAGE)).toEqual(null); + }); + + it('should ignore action of types other than changePushVsOverlayAction', () => { + savePushVsOverlayToLocalStorageMiddleware({} as MiddlewareAPI)(jest.fn)(randomAction); + + expect(localStorage.getItem(EXPANDABLE_FLYOUT_LOCAL_STORAGE)).toEqual(null); + }); + + it('should save value to local storage if action is of type changePushVsOverlayAction', () => { + savePushVsOverlayToLocalStorageMiddleware({} as MiddlewareAPI)(jest.fn)( + changePushVsOverlayAction({ type: 'push', savedToLocalStorage: true }) + ); + + expect(localStorage.getItem(EXPANDABLE_FLYOUT_LOCAL_STORAGE)).toEqual( + JSON.stringify({ [PUSH_VS_OVERLAY_LOCAL_STORAGE]: 'push' }) + ); + }); + + it('should not save value to local storage if savedToLocalStorage is false', () => { + savePushVsOverlayToLocalStorageMiddleware({} as MiddlewareAPI)(jest.fn)( + changePushVsOverlayAction({ type: 'push', savedToLocalStorage: false }) + ); + + expect(localStorage.getItem(EXPANDABLE_FLYOUT_LOCAL_STORAGE)).toEqual(null); + }); +}); diff --git a/packages/kbn-expandable-flyout/src/store/middlewares.ts b/packages/kbn-expandable-flyout/src/store/middlewares.ts new file mode 100644 index 0000000000000..c9e04ea2846d7 --- /dev/null +++ b/packages/kbn-expandable-flyout/src/store/middlewares.ts @@ -0,0 +1,33 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import type { Action, Dispatch, MiddlewareAPI } from '@reduxjs/toolkit'; +import { changePushVsOverlayAction } from './actions'; +import { EXPANDABLE_FLYOUT_LOCAL_STORAGE, PUSH_VS_OVERLAY_LOCAL_STORAGE } from '../constants'; + +/** + * Middleware to save the push vs overlay state to local storage + */ +export const savePushVsOverlayToLocalStorageMiddleware = + (store: MiddlewareAPI) => (next: Dispatch) => (action: Action) => { + if (!action.type) { + return next(action); + } + + if (changePushVsOverlayAction.match(action) && action.payload.savedToLocalStorage) { + localStorage.setItem( + EXPANDABLE_FLYOUT_LOCAL_STORAGE, + JSON.stringify({ + [PUSH_VS_OVERLAY_LOCAL_STORAGE]: action.payload.type, + }) + ); + } + + return next(action); + }; diff --git a/packages/kbn-expandable-flyout/src/store/reducers.test.ts b/packages/kbn-expandable-flyout/src/store/reducers.test.ts index aafd72196d0f5..78caea13bd2d3 100644 --- a/packages/kbn-expandable-flyout/src/store/reducers.test.ts +++ b/packages/kbn-expandable-flyout/src/store/reducers.test.ts @@ -8,9 +8,10 @@ */ import { FlyoutPanelProps } from '../types'; -import { panelsReducer } from './reducers'; -import { initialPanelsState, PanelsState } from './state'; +import { panelsReducer, uiReducer } from './reducers'; +import { initialPanelsState, PanelsState, initialUiState, UiState } from './state'; import { + changePushVsOverlayAction, closeLeftPanelAction, closePanelsAction, closePreviewPanelAction, @@ -781,3 +782,35 @@ describe('panelsReducer', () => { }); }); }); + +describe('uiReducer', () => { + describe('should handle changePushVsOverlayAction action', () => { + it('should set value if id does not exist', () => { + const state: UiState = initialUiState; + const action = changePushVsOverlayAction({ + type: 'push', + savedToLocalStorage: false, + }); + const newState: UiState = uiReducer(state, action); + + expect(newState).toEqual({ + pushVsOverlay: 'push', + }); + }); + + it('should override value if id already exists', () => { + const state: UiState = { + pushVsOverlay: 'push', + }; + const action = changePushVsOverlayAction({ + type: 'overlay', + savedToLocalStorage: false, + }); + const newState: UiState = uiReducer(state, action); + + expect(newState).toEqual({ + pushVsOverlay: 'overlay', + }); + }); + }); +}); diff --git a/packages/kbn-expandable-flyout/src/store/reducers.ts b/packages/kbn-expandable-flyout/src/store/reducers.ts index 8971fd55f7571..54918f5c6d7bb 100644 --- a/packages/kbn-expandable-flyout/src/store/reducers.ts +++ b/packages/kbn-expandable-flyout/src/store/reducers.ts @@ -20,8 +20,9 @@ import { previousPreviewPanelAction, openPreviewPanelAction, urlChangedAction, + changePushVsOverlayAction, } from './actions'; -import { initialPanelsState } from './state'; +import { initialPanelsState, initialUiState } from './state'; export const panelsReducer = createReducer(initialPanelsState, (builder) => { builder.addCase(openPanelsAction, (state, { payload: { preview, left, right, id } }) => { @@ -149,3 +150,9 @@ export const panelsReducer = createReducer(initialPanelsState, (builder) => { state.needsSync = false; }); }); + +export const uiReducer = createReducer(initialUiState, (builder) => { + builder.addCase(changePushVsOverlayAction, (state, { payload: { type } }) => { + state.pushVsOverlay = type; + }); +}); diff --git a/packages/kbn-expandable-flyout/src/store/redux.ts b/packages/kbn-expandable-flyout/src/store/redux.ts index 0e81ba74de2de..9951334a247f3 100644 --- a/packages/kbn-expandable-flyout/src/store/redux.ts +++ b/packages/kbn-expandable-flyout/src/store/redux.ts @@ -11,14 +11,17 @@ import { createContext } from 'react'; import { createDispatchHook, createSelectorHook, ReactReduxContextValue } from 'react-redux'; import { configureStore } from '@reduxjs/toolkit'; import { createSelector } from 'reselect'; -import { panelsReducer } from './reducers'; +import { panelsReducer, uiReducer } from './reducers'; import { initialState, State } from './state'; +import { savePushVsOverlayToLocalStorageMiddleware } from './middlewares'; export const store = configureStore({ reducer: { panels: panelsReducer, + ui: uiReducer, }, devTools: process.env.NODE_ENV !== 'production', + middleware: [savePushVsOverlayToLocalStorageMiddleware], }); export const Context = createContext>({ @@ -35,3 +38,6 @@ const panelsSelector = createSelector(stateSelector, (state) => state.panels); export const selectPanelsById = (id: string) => createSelector(panelsSelector, (state) => state.byId[id] || {}); export const selectNeedsSync = () => createSelector(panelsSelector, (state) => state.needsSync); + +const uiSelector = createSelector(stateSelector, (state) => state.ui); +export const selectPushVsOverlay = createSelector(uiSelector, (state) => state.pushVsOverlay); diff --git a/packages/kbn-expandable-flyout/src/store/state.ts b/packages/kbn-expandable-flyout/src/store/state.ts index 12f1b0135460b..a794d0db34d28 100644 --- a/packages/kbn-expandable-flyout/src/store/state.ts +++ b/packages/kbn-expandable-flyout/src/store/state.ts @@ -44,13 +44,29 @@ export const initialPanelsState: PanelsState = { needsSync: false, }; +export interface UiState { + /** + * Push vs overlay information + */ + pushVsOverlay: 'push' | 'overlay'; +} + +export const initialUiState: UiState = { + pushVsOverlay: 'overlay', +}; + export interface State { /** * All panels related information */ panels: PanelsState; + /** + * All ui related information + */ + ui: UiState; } export const initialState: State = { panels: initialPanelsState, + ui: initialUiState, }; diff --git a/packages/kbn-expandable-flyout/src/test/provider.tsx b/packages/kbn-expandable-flyout/src/test/provider.tsx index b6914099e2e42..0dc2656e15c7e 100644 --- a/packages/kbn-expandable-flyout/src/test/provider.tsx +++ b/packages/kbn-expandable-flyout/src/test/provider.tsx @@ -11,8 +11,9 @@ import { Provider as ReduxProvider } from 'react-redux'; import { configureStore } from '@reduxjs/toolkit'; import React, { FC, PropsWithChildren } from 'react'; import { I18nProvider } from '@kbn/i18n-react'; +import { savePushVsOverlayToLocalStorageMiddleware } from '../store/middlewares'; import { ExpandableFlyoutContextProvider } from '../context'; -import { panelsReducer } from '../store/reducers'; +import { panelsReducer, uiReducer } from '../store/reducers'; import { Context } from '../store/redux'; import { initialState, State } from '../store/state'; @@ -29,10 +30,11 @@ export const TestProvider: FC> = ({ const store = configureStore({ reducer: { panels: panelsReducer, + ui: uiReducer, }, devTools: false, preloadedState: state, - enhancers: [], + middleware: [savePushVsOverlayToLocalStorageMiddleware], }); return (