Skip to content

Commit

Permalink
[Security Solution][Flyout] Don't share state using react context (el…
Browse files Browse the repository at this point in the history
…astic#174666)

## Summary

This is a first PR in series of improving the performance / dx of how we
are doing state management in the flyout component.

We used to share both state and api for the flyout using context api;
now the url state / callbacks are just grouped under a single hook, and
for the in-memory we are doing isolated redux store.

Further PRs will involve splitting the api hook into separate portions
for the state and callbacks (so that we dont re-render the components
that just need the callback access when state changes), and some typing
/ code cleanups.
  • Loading branch information
lgestc authored and CoenWarmer committed Feb 15, 2024
1 parent f45e2e1 commit f4ddc9c
Show file tree
Hide file tree
Showing 40 changed files with 610 additions and 645 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-expandable-flyout/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ export { useExpandableFlyoutContext, type ExpandableFlyoutContext } from './src/
export { ExpandableFlyoutProvider } from './src/provider';

export type { ExpandableFlyoutProps } from './src';
export type { FlyoutPanelProps, PanelPath } from './src/types';
export type { FlyoutPanelProps, PanelPath, ExpandableFlyoutApi } from './src/types';

export { EXPANDABLE_FLYOUT_URL_KEY } from './src/constants';
53 changes: 17 additions & 36 deletions packages/kbn-expandable-flyout/src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import { createAction } from '@reduxjs/toolkit';
import { FlyoutPanelProps } from './types';

export enum ActionType {
Expand All @@ -20,39 +21,19 @@ export enum ActionType {
closeFlyout = 'close_flyout',
}

export type Action =
| {
type: ActionType.openFlyout;
payload: {
right?: FlyoutPanelProps;
left?: FlyoutPanelProps;
preview?: FlyoutPanelProps;
};
}
| {
type: ActionType.openRightPanel;
payload: FlyoutPanelProps;
}
| {
type: ActionType.openLeftPanel;
payload: FlyoutPanelProps;
}
| {
type: ActionType.openPreviewPanel;
payload: FlyoutPanelProps;
}
| {
type: ActionType.closeRightPanel;
}
| {
type: ActionType.closeLeftPanel;
}
| {
type: ActionType.closePreviewPanel;
}
| {
type: ActionType.previousPreviewPanel;
}
| {
type: ActionType.closeFlyout;
};
export const openPanelsAction = createAction<{
right?: FlyoutPanelProps;
left?: FlyoutPanelProps;
preview?: FlyoutPanelProps;
}>(ActionType.openFlyout);

export const openRightPanelAction = createAction<FlyoutPanelProps>(ActionType.openRightPanel);
export const openLeftPanelAction = createAction<FlyoutPanelProps>(ActionType.openLeftPanel);
export const openPreviewPanelAction = createAction<FlyoutPanelProps>(ActionType.openPreviewPanel);

export const closePanelsAction = createAction(ActionType.closeFlyout);
export const closeRightPanelAction = createAction(ActionType.closeRightPanel);
export const closeLeftPanelAction = createAction(ActionType.closeLeftPanel);
export const closePreviewPanelAction = createAction(ActionType.closePreviewPanel);

export const previousPreviewPanelAction = createAction(ActionType.previousPreviewPanel);
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import {
PREVIEW_SECTION_CLOSE_BUTTON_TEST_ID,
PREVIEW_SECTION_TEST_ID,
} from './test_ids';
import { ExpandableFlyoutContext, ExpandableFlyoutContextValue } from '../context';
import { ExpandableFlyoutContextValue } from '../context';
import { TestProvider } from '../test/provider';

describe('PreviewSection', () => {
const context = {
Expand All @@ -36,9 +37,9 @@ describe('PreviewSection', () => {
const showBackButton = false;

const { getByTestId } = render(
<ExpandableFlyoutContext.Provider value={context}>
<TestProvider state={context.panels}>
<PreviewSection component={component} leftPosition={left} showBackButton={showBackButton} />
</ExpandableFlyoutContext.Provider>
</TestProvider>
);

expect(getByTestId(PREVIEW_SECTION_CLOSE_BUTTON_TEST_ID)).toBeInTheDocument();
Expand All @@ -48,9 +49,9 @@ describe('PreviewSection', () => {
const showBackButton = true;

const { getByTestId } = render(
<ExpandableFlyoutContext.Provider value={context}>
<TestProvider state={context.panels}>
<PreviewSection component={component} leftPosition={left} showBackButton={showBackButton} />
</ExpandableFlyoutContext.Provider>
</TestProvider>
);

expect(getByTestId(PREVIEW_SECTION_BACK_BUTTON_TEST_ID)).toBeInTheDocument();
Expand All @@ -66,14 +67,14 @@ describe('PreviewSection', () => {
};

const { getByTestId, getByText } = render(
<ExpandableFlyoutContext.Provider value={context}>
<TestProvider state={context.panels}>
<PreviewSection
component={component}
leftPosition={left}
showBackButton={showBackButton}
banner={banner}
/>
</ExpandableFlyoutContext.Provider>
</TestProvider>
);

expect(getByTestId(`${PREVIEW_SECTION_TEST_ID}BannerPanel`)).toHaveClass(
Expand Down
15 changes: 11 additions & 4 deletions packages/kbn-expandable-flyout/src/context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@
*/

import { createContext, useContext } from 'react';
import { type ExpandableFlyoutContextValue } from './types';
import { useFlyoutMemoryState } from './context/memory_state_provider';
import { useFlyoutUrlState } from './context/url_state_provider';
import { type ExpandableFlyoutApi } from './types';

export type { ExpandableFlyoutContextValue };
export type { ExpandableFlyoutApi as ExpandableFlyoutContextValue };

type ExpandableFlyoutContextValue = 'memory' | 'url';

export const ExpandableFlyoutContext = createContext<ExpandableFlyoutContextValue | undefined>(
undefined
Expand All @@ -18,7 +22,7 @@ export const ExpandableFlyoutContext = createContext<ExpandableFlyoutContextValu
/**
* Retrieve context's properties
*/
export const useExpandableFlyoutContext = (): ExpandableFlyoutContextValue => {
export const useExpandableFlyoutContext = (): ExpandableFlyoutApi => {
const contextValue = useContext(ExpandableFlyoutContext);

if (!contextValue) {
Expand All @@ -27,5 +31,8 @@ export const useExpandableFlyoutContext = (): ExpandableFlyoutContextValue => {
);
}

return contextValue;
const memoryState = useFlyoutMemoryState();
const urlState = useFlyoutUrlState();

return contextValue === 'memory' ? memoryState : urlState;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,48 @@
* Side Public License, v 1.
*/

import React, { FC, PropsWithChildren, useCallback, useMemo, useReducer } from 'react';
import { ActionType } from '../actions';
import React, { createContext, FC, useCallback, useMemo } from 'react';
import {
createDispatchHook,
createSelectorHook,
Provider as ReduxProvider,
ReactReduxContextValue,
} from 'react-redux';
import { configureStore } from '@reduxjs/toolkit';

import { reducer } from '../reducer';
import type { ExpandableFlyoutContextValue, FlyoutPanelProps } from '../types';
import { initialState } from '../reducer';
import { ExpandableFlyoutContext } from '../context';
import { initialState, State } from '../state';
import type { ExpandableFlyoutApi, FlyoutPanelProps } from '../types';
import {
closeLeftPanelAction,
closePanelsAction,
closePreviewPanelAction,
closeRightPanelAction,
openLeftPanelAction,
openPanelsAction,
openPreviewPanelAction,
openRightPanelAction,
previousPreviewPanelAction,
} from '../actions';

/**
* In-memory state provider for the expandable flyout, for cases when we don't want changes to be persisted
* in the url.
*/
export const MemoryStateProvider: FC<PropsWithChildren<{}>> = ({ children }) => {
const [state, dispatch] = useReducer(reducer, initialState);
export const store = configureStore({
reducer,
devTools: process.env.NODE_ENV !== 'production',
preloadedState: {},
enhancers: [],
});

export const Context = createContext<ReactReduxContextValue<State>>({
store,
storeState: initialState,
});

const useDispatch = createDispatchHook(Context);
const useSelector = createSelectorHook(Context);

export const useFlyoutMemoryState = (): ExpandableFlyoutApi => {
const state = useSelector((s) => s);
const dispatch = useDispatch();

const openPanels = useCallback(
({
Expand All @@ -29,41 +58,41 @@ export const MemoryStateProvider: FC<PropsWithChildren<{}>> = ({ children }) =>
right?: FlyoutPanelProps;
left?: FlyoutPanelProps;
preview?: FlyoutPanelProps;
}) => dispatch({ type: ActionType.openFlyout, payload: { left, right, preview } }),
}) => dispatch(openPanelsAction({ right, left, preview })),
[dispatch]
);

const openRightPanel = useCallback(
(panel: FlyoutPanelProps) => dispatch({ type: ActionType.openRightPanel, payload: panel }),
[]
(panel: FlyoutPanelProps) => dispatch(openRightPanelAction(panel)),
[dispatch]
);

const openLeftPanel = useCallback(
(panel: FlyoutPanelProps) => dispatch({ type: ActionType.openLeftPanel, payload: panel }),
[]
(panel: FlyoutPanelProps) => dispatch(openLeftPanelAction(panel)),
[dispatch]
);

const openPreviewPanel = useCallback(
(panel: FlyoutPanelProps) => dispatch({ type: ActionType.openPreviewPanel, payload: panel }),
[]
(panel: FlyoutPanelProps) => dispatch(openPreviewPanelAction(panel)),
[dispatch]
);

const closeRightPanel = useCallback(() => dispatch({ type: ActionType.closeRightPanel }), []);
const closeRightPanel = useCallback(() => dispatch(closeRightPanelAction()), [dispatch]);

const closeLeftPanel = useCallback(() => dispatch({ type: ActionType.closeLeftPanel }), []);
const closeLeftPanel = useCallback(() => dispatch(closeLeftPanelAction()), [dispatch]);

const closePreviewPanel = useCallback(() => dispatch({ type: ActionType.closePreviewPanel }), []);
const closePreviewPanel = useCallback(() => dispatch(closePreviewPanelAction()), [dispatch]);

const previousPreviewPanel = useCallback(
() => dispatch({ type: ActionType.previousPreviewPanel }),
[]
() => dispatch(previousPreviewPanelAction()),
[dispatch]
);

const closePanels = useCallback(() => {
dispatch({ type: ActionType.closeFlyout });
}, []);
dispatch(closePanelsAction());
}, [dispatch]);

const contextValue: ExpandableFlyoutContextValue = useMemo(
const api: ExpandableFlyoutApi = useMemo(
() => ({
panels: state,
openFlyout: openPanels,
Expand All @@ -90,9 +119,17 @@ export const MemoryStateProvider: FC<PropsWithChildren<{}>> = ({ children }) =>
]
);

return api;
};

/**
* In-memory state provider for the expandable flyout, for cases when we don't want changes to be persisted
* in the url.
*/
export const MemoryStateProvider: FC = ({ children }) => {
return (
<ExpandableFlyoutContext.Provider value={contextValue}>
<ReduxProvider context={Context} store={store}>
{children}
</ExpandableFlyoutContext.Provider>
</ReduxProvider>
);
};
20 changes: 6 additions & 14 deletions packages/kbn-expandable-flyout/src/context/url_state_provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@
* Side Public License, v 1.
*/

import React, { FC, PropsWithChildren, useCallback, useMemo } from 'react';
import { FlyoutPanelProps, ExpandableFlyoutContextValue } from '../types';
import { useCallback, useMemo } from 'react';
import { FlyoutPanelProps, ExpandableFlyoutApi } from '../types';
import { useRightPanel } from '../hooks/use_right_panel';
import { useLeftPanel } from '../hooks/use_left_panel';
import { usePreviewPanel } from '../hooks/use_preview_panel';
import { ExpandableFlyoutContext } from '../context';
import { State } from '../reducer';
import { State } from '../state';

/**
* Private component that manages flyout state with url query params
*/
export const UrlStateProvider: FC<PropsWithChildren<{}>> = ({ children }) => {
export const useFlyoutUrlState = (): ExpandableFlyoutApi => {
const { setRightPanelState, rightPanelState } = useRightPanel();
const { setLeftPanelState, leftPanelState } = useLeftPanel();
const { previewState, setPreviewState } = usePreviewPanel();
Expand Down Expand Up @@ -82,7 +78,7 @@ export const UrlStateProvider: FC<PropsWithChildren<{}>> = ({ children }) => {
setPreviewState([]);
}, [setRightPanelState, setLeftPanelState, setPreviewState]);

const contextValue: ExpandableFlyoutContextValue = useMemo(
const contextValue: ExpandableFlyoutApi = useMemo(
() => ({
panels,
openFlyout: openPanels,
Expand All @@ -109,9 +105,5 @@ export const UrlStateProvider: FC<PropsWithChildren<{}>> = ({ children }) => {
]
);

return (
<ExpandableFlyoutContext.Provider value={contextValue}>
{children}
</ExpandableFlyoutContext.Provider>
);
return contextValue;
};
Loading

0 comments on commit f4ddc9c

Please sign in to comment.