Skip to content

Commit

Permalink
[8.x] [kbn-expandable-flyout] - refactor push/overlay to use redux in…
Browse files Browse the repository at this point in the history 
…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)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Philippe
Oberti","email":"philippe.oberti@elastic.co"},"sourceCommit":{"committedDate":"2024-09-17T20:15:40Z","message":"[kbn-expandable-flyout]
- refactor push/overlay to use redux instead of hooks
(#192745)","sha":"31c9e75f56fefb18d6ad1e8e53e75c2117c6c343","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Threat
Hunting:Investigations","v8.16.0"],"title":"[kbn-expandable-flyout] -
refactor push/overlay to use redux instead of
hooks","number":192745,"url":"#192745
- refactor push/overlay to use redux instead of hooks
(#192745)","sha":"31c9e75f56fefb18d6ad1e8e53e75c2117c6c343"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"#192745
- refactor push/overlay to use redux instead of hooks
(#192745)","sha":"31c9e75f56fefb18d6ad1e8e53e75c2117c6c343"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Philippe Oberti <philippe.oberti@elastic.co>
  • Loading branch information
@kibanamachine @PhilippeOberti
kibanamachine and PhilippeOberti committed Sep 17, 2024
1 parent 664d067 commit 60f5269
Show file tree
Hide file tree
Showing 20 changed files with 490 additions and 189 deletions.
3 changes: 3 additions & 0 deletions packages/kbn-expandable-flyout/src/components/preview_section.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ describe('PreviewSection', () => {
},
},
},
ui: {
pushVsOverlay: 'overlay',
},
};

const component = <div>{'component'}</div>;
Expand Down
134 changes: 107 additions & 27 deletions packages/kbn-expandable-flyout/src/components/settings_menu.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down
56 changes: 36 additions & 20 deletions packages/kbn-expandable-flyout/src/components/settings_menu.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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',
Expand Down Expand Up @@ -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 = [
Expand All @@ -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"
Expand Down Expand Up @@ -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>
Expand Down
3 changes: 3 additions & 0 deletions packages/kbn-expandable-flyout/src/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
50 changes: 0 additions & 50 deletions packages/kbn-expandable-flyout/src/hooks/use_flyout_type.test.ts
View file

This file was deleted.

Loading

0 comments on commit 60f5269

Please sign in to comment.