[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
elastic · Jun 28, 2024 · c026264 · c026264
1 parent 34887f2
commit c026264
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 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
@@ -31,32 +31,18 @@ describe('PreviewSection', () => {
   const component = <div>{'component'}</div>;
   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(
       <TestProvider state={context}>
-        <PreviewSection component={component} leftPosition={left} showBackButton={showBackButton} />
+        <PreviewSection component={component} leftPosition={left} />
       </TestProvider>
     );
 
     expect(getByTestId(PREVIEW_SECTION_CLOSE_BUTTON_TEST_ID)).toBeInTheDocument();
-  });
-
-  it('should render back button in header', () => {
-    const showBackButton = true;
-
-    const { getByTestId } = render(
-      <TestProvider state={context}>
-        <PreviewSection component={component} leftPosition={left} showBackButton={showBackButton} />
-      </TestProvider>
-    );
-
     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(
       <TestProvider state={context}>
-        <PreviewSection
-          component={component}
-          leftPosition={left}
-          showBackButton={showBackButton}
-          banner={banner}
-        />
+        <PreviewSection component={component} leftPosition={left} banner={banner} />
       </TestProvider>
     );
 

diff --git 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<PreviewSectionProps> = ({
   component,
-  showBackButton,
   leftPosition,
   banner,
 }: PreviewSectionProps) => {
@@ -103,7 +98,7 @@ export const PreviewSection: React.FC<PreviewSectionProps> = ({
       />
     </EuiFlexItem>
   );
-  const header = showBackButton ? (
+  const header = (
     <EuiFlexGroup justifyContent="spaceBetween" responsive={false}>
       <EuiFlexItem grow={false}>
         <EuiButtonEmpty
@@ -119,10 +114,6 @@ export const PreviewSection: React.FC<PreviewSectionProps> = ({
       </EuiFlexItem>
       {closeButton}
     </EuiFlexGroup>
-  ) : (
-    <EuiFlexGroup justifyContent="flexEnd" responsive={false}>
-      {closeButton}
-    </EuiFlexGroup>
   );
 
   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
@@ -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
@@ -70,7 +70,6 @@ export const ExpandableFlyout: React.FC<ExpandableFlyoutProps> = ({
     ? 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<ExpandableFlyoutProps> = ({
       {showPreview ? (
         <PreviewSection
           component={previewSection.component({ ...(mostRecentPreview as FlyoutPanelProps) })}
-          showBackButton={showBackButton}
           leftPosition={previewSectionLeft}
           banner={previewBanner}
         />

diff --git 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
@@ -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$<FlyoutState>(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
@@ -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
@@ -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 } }) => {