mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 17:28:26 -04:00
[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
4552c3d5
-541b-4551-8188-fafaf6235c2c
This commit is contained in:
parent
34887f2d8c
commit
c026264640
8 changed files with 29 additions and 48 deletions
|
@ -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>
|
||||
);
|
||||
|
||||
|
|
|
@ -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 (
|
||||
|
|
|
@ -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]);
|
||||
|
||||
|
|
|
@ -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}
|
||||
/>
|
||||
|
|
|
@ -67,7 +67,7 @@ describe('UrlSynchronizer', () => {
|
|||
expect(mockSet).toHaveBeenCalledWith('urlKey', {
|
||||
left: undefined,
|
||||
right: undefined,
|
||||
preview: undefined,
|
||||
preview: [undefined],
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
@ -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;
|
||||
|
|
|
@ -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,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -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 } }) => {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue