Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution][Details Flyout] Fix multi-preview url sync #186130

Merged
merged 1 commit into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -84,7 +80,6 @@ interface PreviewSectionProps {
*/
export const PreviewSection: React.FC<PreviewSectionProps> = ({
component,
showBackButton,
leftPosition,
banner,
}: PreviewSectionProps) => {
Expand All @@ -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
Expand All @@ -119,10 +114,6 @@ export const PreviewSection: React.FC<PreviewSectionProps> = ({
</EuiFlexItem>
{closeButton}
</EuiFlexGroup>
) : (
<EuiFlexGroup justifyContent="flexEnd" responsive={false}>
{closeButton}
</EuiFlexGroup>
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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'
Expand Down Expand Up @@ -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]);

Expand Down
2 changes: 0 additions & 2 deletions packages/kbn-expandable-flyout/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -129,7 +128,6 @@ export const ExpandableFlyout: React.FC<ExpandableFlyoutProps> = ({
{showPreview ? (
<PreviewSection
component={previewSection.component({ ...(mostRecentPreview as FlyoutPanelProps) })}
showBackButton={showBackButton}
leftPosition={previewSectionLeft}
banner={previewBanner}
/>
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-expandable-flyout/src/provider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('UrlSynchronizer', () => {
expect(mockSet).toHaveBeenCalledWith('urlKey', {
left: undefined,
right: undefined,
preview: undefined,
preview: [undefined],
});
});

Expand Down
6 changes: 3 additions & 3 deletions packages/kbn-expandable-flyout/src/provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions packages/kbn-expandable-flyout/src/reducer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`, () => {
Expand All @@ -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', () => {
Expand All @@ -662,7 +662,7 @@ describe('reducer', () => {
preview: [previewPanel1],
},
},
needsSync: true,
needsSync: false,
});
});

Expand All @@ -687,7 +687,7 @@ describe('reducer', () => {
preview: [previewPanel1],
},
},
needsSync: true,
needsSync: false,
});
});
});
Expand Down
10 changes: 8 additions & 2 deletions packages/kbn-expandable-flyout/src/reducer.ts
christineweng marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { createReducer } from '@reduxjs/toolkit';
import deepEqual from 'react-fast-compare';
import {
openPanelsAction,
openLeftPanelAction,
Expand Down Expand Up @@ -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;
}
Expand All @@ -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 } }) => {
Expand Down