From c026264640b8211004bf80f9221ad5eceeb091d4 Mon Sep 17 00:00:00 2001 From: christineweng <18648970+christineweng@users.noreply.github.com> Date: Fri, 28 Jun 2024 13:14:34 -0500 Subject: [PATCH] [Security Solution][Details Flyout] Fix multi-preview url sync (#186130) ## Summary This PR fixed a preview rendering bug and made some improvements to the preview navigation: - If a preview is already opened, opening another preview should work properly. `urlChangedAction` is now dispatching the last item in `preview` array. - Added checks to not append a preview if it was last added - When state is stored in url (everywhere except rule preview), `Back` action in preview utilizes the browser go back functionality. This allows full synchronization between redux and url state. - Keep the latest preview in url to avoid url length explosion Note: in order to make the interaction smooth, the `Back` button is now always present. When there is one preview open, clicking `Go back` will close the preview. **How to test** - Enable feature flag `entityAlertPreviewEnabled` - Generate some alerts, go to Alerts Page - Expand detail on an alert, expand details, entities, clicking the host and user names https://github.com/elastic/kibana/assets/18648970/4552c3d5-541b-4551-8188-fafaf6235c2c --- .../src/components/preview_section.test.tsx | 25 +++---------------- .../src/components/preview_section.tsx | 11 +------- .../src/hooks/use_expandable_flyout_api.ts | 13 +++++++--- packages/kbn-expandable-flyout/src/index.tsx | 2 -- .../src/provider.test.tsx | 2 +- .../kbn-expandable-flyout/src/provider.tsx | 6 ++--- .../kbn-expandable-flyout/src/reducer.test.ts | 8 +++--- packages/kbn-expandable-flyout/src/reducer.ts | 10 ++++++-- 8 files changed, 29 insertions(+), 48 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 index 223d73fa5cf43..27cff985534e8 100644 --- a/packages/kbn-expandable-flyout/src/components/preview_section.test.tsx +++ b/packages/kbn-expandable-flyout/src/components/preview_section.test.tsx @@ -31,32 +31,18 @@ describe('PreviewSection', () => { const component =
{'component'}
; const left = 500; - it('should render close button in header', () => { - const showBackButton = false; - + it('should render back button and close button in header', () => { const { getByTestId } = render( - + ); expect(getByTestId(PREVIEW_SECTION_CLOSE_BUTTON_TEST_ID)).toBeInTheDocument(); - }); - - it('should render back button in header', () => { - const showBackButton = true; - - const { getByTestId } = render( - - - - ); - expect(getByTestId(PREVIEW_SECTION_BACK_BUTTON_TEST_ID)).toBeInTheDocument(); }); it('should render banner', () => { - const showBackButton = false; const title = 'test'; const banner: PreviewBanner = { title, @@ -66,12 +52,7 @@ describe('PreviewSection', () => { const { getByTestId, getByText } = render( - + ); diff --git a/packages/kbn-expandable-flyout/src/components/preview_section.tsx b/packages/kbn-expandable-flyout/src/components/preview_section.tsx index 7a49e2ca463ce..bc0c1c2e9d93a 100644 --- a/packages/kbn-expandable-flyout/src/components/preview_section.tsx +++ b/packages/kbn-expandable-flyout/src/components/preview_section.tsx @@ -68,10 +68,6 @@ interface PreviewSectionProps { * Left position used when rendering the panel */ leftPosition: number; - /** - * Display the back button in the header - */ - showBackButton: boolean; /** * Preview banner shown at the top of preview panel */ @@ -84,7 +80,6 @@ interface PreviewSectionProps { */ export const PreviewSection: React.FC = ({ component, - showBackButton, leftPosition, banner, }: PreviewSectionProps) => { @@ -103,7 +98,7 @@ export const PreviewSection: React.FC = ({ /> ); - const header = showBackButton ? ( + const header = ( = ({ {closeButton} - ) : ( - - {closeButton} - ); return ( diff --git a/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_api.ts b/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_api.ts index dcfcf429e1086..5c7b716b003b5 100644 --- a/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_api.ts +++ b/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_api.ts @@ -7,6 +7,7 @@ */ import { useCallback, useMemo } from 'react'; +import { useHistory } from 'react-router-dom'; import { REDUX_ID_FOR_MEMORY_STORAGE } from '../constants'; import { useExpandableFlyoutContext } from '../context'; import { @@ -30,6 +31,7 @@ export type { ExpandableFlyoutApi }; */ export const useExpandableFlyoutApi = () => { const dispatch = useDispatch(); + const history = useHistory(); const { urlKey } = useExpandableFlyoutContext(); // if no urlKey is provided, we are in memory storage mode and use the reserved word 'memory' @@ -75,10 +77,13 @@ export const useExpandableFlyoutApi = () => { [dispatch, id] ); - const previousPreviewPanel = useCallback( - () => dispatch(previousPreviewPanelAction({ id })), - [dispatch, id] - ); + const previousPreviewPanel = useCallback(() => { + dispatch(previousPreviewPanelAction({ id })); + + if (id !== REDUX_ID_FOR_MEMORY_STORAGE) { + history.goBack(); + } + }, [dispatch, id, history]); const closePanels = useCallback(() => dispatch(closePanelsAction({ id })), [dispatch, id]); diff --git a/packages/kbn-expandable-flyout/src/index.tsx b/packages/kbn-expandable-flyout/src/index.tsx index 7bcfa050fbfa5..ba11b597e0b06 100644 --- a/packages/kbn-expandable-flyout/src/index.tsx +++ b/packages/kbn-expandable-flyout/src/index.tsx @@ -70,7 +70,6 @@ export const ExpandableFlyout: React.FC = ({ ? mostRecentPreview?.params?.banner : undefined; - const showBackButton = !!preview && preview.length > 1; const previewSection = useMemo( () => registeredPanels.find((panel) => panel.key === mostRecentPreview?.id), [mostRecentPreview, registeredPanels] @@ -129,7 +128,6 @@ export const ExpandableFlyout: React.FC = ({ {showPreview ? ( diff --git a/packages/kbn-expandable-flyout/src/provider.test.tsx b/packages/kbn-expandable-flyout/src/provider.test.tsx index c6246eff9fa32..f47b538ed851e 100644 --- a/packages/kbn-expandable-flyout/src/provider.test.tsx +++ b/packages/kbn-expandable-flyout/src/provider.test.tsx @@ -67,7 +67,7 @@ describe('UrlSynchronizer', () => { expect(mockSet).toHaveBeenCalledWith('urlKey', { left: undefined, right: undefined, - preview: undefined, + preview: [undefined], }); }); diff --git a/packages/kbn-expandable-flyout/src/provider.tsx b/packages/kbn-expandable-flyout/src/provider.tsx index d88df39ab61eb..7ad9ddd084963 100644 --- a/packages/kbn-expandable-flyout/src/provider.tsx +++ b/packages/kbn-expandable-flyout/src/provider.tsx @@ -49,14 +49,14 @@ export const UrlSynchronizer = () => { dispatch( urlChangedAction({ ...currentValue, - preview: currentValue?.preview?.[0], + preview: currentValue?.preview?.at(-1), id: urlKey, }) ); } const subscription = urlStorage.change$(urlKey).subscribe((value) => { - dispatch(urlChangedAction({ ...value, preview: value?.preview?.[0], id: urlKey })); + dispatch(urlChangedAction({ ...value, preview: value?.preview?.at(-1), id: urlKey })); }); return () => subscription.unsubscribe(); @@ -68,7 +68,7 @@ export const UrlSynchronizer = () => { } const { left, right, preview } = panels; - urlStorage.set(urlKey, { left, right, preview }); + urlStorage.set(urlKey, { left, right, preview: [preview?.at(-1)] }); }, [needsSync, panels, urlKey, urlStorage]); return null; diff --git a/packages/kbn-expandable-flyout/src/reducer.test.ts b/packages/kbn-expandable-flyout/src/reducer.test.ts index df03d9277f946..7f0a5c289e295 100644 --- a/packages/kbn-expandable-flyout/src/reducer.test.ts +++ b/packages/kbn-expandable-flyout/src/reducer.test.ts @@ -622,7 +622,7 @@ describe('reducer', () => { const action = previousPreviewPanelAction({ id: id1 }); const newState: State = reducer(state, action); - expect(newState).toEqual({ ...initialState, needsSync: true }); + expect(newState).toEqual({ ...initialState, needsSync: false }); }); it(`should return unmodified state when previous preview panel when no preview panel exist`, () => { @@ -638,7 +638,7 @@ describe('reducer', () => { const action = previousPreviewPanelAction({ id: id1 }); const newState: State = reducer(state, action); - expect(newState).toEqual({ ...state, needsSync: true }); + expect(newState).toEqual({ ...state, needsSync: false }); }); it('should remove only last preview panel', () => { @@ -662,7 +662,7 @@ describe('reducer', () => { preview: [previewPanel1], }, }, - needsSync: true, + needsSync: false, }); }); @@ -687,7 +687,7 @@ describe('reducer', () => { preview: [previewPanel1], }, }, - needsSync: true, + needsSync: false, }); }); }); diff --git a/packages/kbn-expandable-flyout/src/reducer.ts b/packages/kbn-expandable-flyout/src/reducer.ts index 617ad5e3a7c95..32465f554ab7f 100644 --- a/packages/kbn-expandable-flyout/src/reducer.ts +++ b/packages/kbn-expandable-flyout/src/reducer.ts @@ -7,6 +7,7 @@ */ import { createReducer } from '@reduxjs/toolkit'; +import deepEqual from 'react-fast-compare'; import { openPanelsAction, openLeftPanelAction, @@ -69,7 +70,11 @@ export const reducer = createReducer(initialState, (builder) => { builder.addCase(openPreviewPanelAction, (state, { payload: { preview, id } }) => { if (id in state.byId) { if (state.byId[id].preview) { - state.byId[id].preview?.push(preview); + const previewIdenticalToLastOne = deepEqual(preview, state.byId[id].preview?.at(-1)); + // Only append preview when it does not match the last item in state.byId[id].preview + if (!previewIdenticalToLastOne) { + state.byId[id].preview?.push(preview); + } } else { state.byId[id].preview = preview ? [preview] : undefined; } @@ -89,7 +94,8 @@ export const reducer = createReducer(initialState, (builder) => { state.byId[id].preview?.pop(); } - state.needsSync = true; + // if state is stored in url, click go back in preview should utilize browser history + state.needsSync = false; }); builder.addCase(closePanelsAction, (state, { payload: { id } }) => {