From 3604f5d21ae4ea5d72873196afe62056a2f0496c Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Thu, 7 May 2020 13:18:56 +0200 Subject: [PATCH] [Drilldowns] Preserve state when selecting different action factory (#65074) Co-authored-by: Elastic Machine --- .../action_wizard/action_wizard.tsx | 6 +- .../components/action_wizard/test_data.tsx | 2 +- ...onnected_flyout_manage_drilldowns.test.tsx | 36 ++++++ .../flyout_drilldown_wizard.tsx | 103 ++++++++++++------ .../form_drilldown_wizard.tsx | 2 +- 5 files changed, 112 insertions(+), 37 deletions(-) diff --git a/x-pack/plugins/advanced_ui_actions/public/components/action_wizard/action_wizard.tsx b/x-pack/plugins/advanced_ui_actions/public/components/action_wizard/action_wizard.tsx index 867ead688d23..4d14226777a0 100644 --- a/x-pack/plugins/advanced_ui_actions/public/components/action_wizard/action_wizard.tsx +++ b/x-pack/plugins/advanced_ui_actions/public/components/action_wizard/action_wizard.tsx @@ -32,9 +32,9 @@ export interface ActionWizardProps { /** * Action factory selected changed - * null - means user click "change" and removed action factory selection + * empty - means user click "change" and removed action factory selection */ - onActionFactoryChange: (actionFactory: ActionFactory | null) => void; + onActionFactoryChange: (actionFactory?: ActionFactory) => void; /** * current config for currently selected action factory @@ -71,7 +71,7 @@ export const ActionWizard: React.FC = ({ actionFactory={currentActionFactory} showDeselect={actionFactories.length > 1} onDeselect={() => { - onActionFactoryChange(null); + onActionFactoryChange(undefined); }} context={context} config={config} diff --git a/x-pack/plugins/advanced_ui_actions/public/components/action_wizard/test_data.tsx b/x-pack/plugins/advanced_ui_actions/public/components/action_wizard/test_data.tsx index c3e749f163c9..692e86b53f09 100644 --- a/x-pack/plugins/advanced_ui_actions/public/components/action_wizard/test_data.tsx +++ b/x-pack/plugins/advanced_ui_actions/public/components/action_wizard/test_data.tsx @@ -167,7 +167,7 @@ export function Demo({ actionFactories }: { actionFactories: Array({}); - function changeActionFactory(newActionFactory: ActionFactory | null) { + function changeActionFactory(newActionFactory?: ActionFactory) { if (!newActionFactory) { // removing action factory return setState({}); diff --git a/x-pack/plugins/drilldowns/public/components/connected_flyout_manage_drilldowns/connected_flyout_manage_drilldowns.test.tsx b/x-pack/plugins/drilldowns/public/components/connected_flyout_manage_drilldowns/connected_flyout_manage_drilldowns.test.tsx index 6749b41e81fc..52c53f32ff09 100644 --- a/x-pack/plugins/drilldowns/public/components/connected_flyout_manage_drilldowns/connected_flyout_manage_drilldowns.test.tsx +++ b/x-pack/plugins/drilldowns/public/components/connected_flyout_manage_drilldowns/connected_flyout_manage_drilldowns.test.tsx @@ -173,6 +173,42 @@ test('Create only mode', async () => { expect(await mockDynamicActionManager.state.get().events.length).toBe(1); }); +test('After switching between action factories state is restored', async () => { + const screen = render( + + ); + // wait for initial render. It is async because resolving compatible action factories is async + await wait(() => expect(screen.getAllByText(/Create/i).length).toBeGreaterThan(0)); + fireEvent.change(screen.getByLabelText(/name/i), { + target: { value: 'test' }, + }); + fireEvent.click(screen.getByText(/Go to URL/i)); + fireEvent.change(screen.getByLabelText(/url/i), { + target: { value: 'https://elastic.co' }, + }); + + // change to dashboard + fireEvent.click(screen.getByText(/change/i)); + fireEvent.click(screen.getByText(/Go to Dashboard/i)); + + // change back to url + fireEvent.click(screen.getByText(/change/i)); + fireEvent.click(screen.getByText(/Go to URL/i)); + + expect(screen.getByLabelText(/url/i)).toHaveValue('https://elastic.co'); + expect(screen.getByLabelText(/name/i)).toHaveValue('test'); + + fireEvent.click(screen.getAllByText(/Create Drilldown/i)[1]); + await wait(() => expect(notifications.toasts.addSuccess).toBeCalled()); + expect(await (mockDynamicActionManager.state.get().events[0].action.config as any).url).toBe( + 'https://elastic.co' + ); +}); + test.todo("Error when can't fetch drilldown list"); test("Error when can't save drilldown changes", async () => { diff --git a/x-pack/plugins/drilldowns/public/components/flyout_drilldown_wizard/flyout_drilldown_wizard.tsx b/x-pack/plugins/drilldowns/public/components/flyout_drilldown_wizard/flyout_drilldown_wizard.tsx index 8541aae06ff0..1f775a5ff103 100644 --- a/x-pack/plugins/drilldowns/public/components/flyout_drilldown_wizard/flyout_drilldown_wizard.tsx +++ b/x-pack/plugins/drilldowns/public/components/flyout_drilldown_wizard/flyout_drilldown_wizard.tsx @@ -41,6 +41,72 @@ export interface FlyoutDrilldownWizardProps void; + setActionConfig: (actionConfig: object) => void; + setActionFactory: (actionFactory?: ActionFactory) => void; + } +] { + const [wizardConfig, setWizardConfig] = useState( + () => + initialDrilldownWizardConfig ?? { + name: '', + } + ); + const [actionConfigCache, setActionConfigCache] = useState>( + initialDrilldownWizardConfig?.actionFactory + ? { + [initialDrilldownWizardConfig.actionFactory + .id]: initialDrilldownWizardConfig.actionConfig!, + } + : {} + ); + + return [ + wizardConfig, + { + setName: (name: string) => { + setWizardConfig({ + ...wizardConfig, + name, + }); + }, + setActionConfig: (actionConfig: object) => { + setWizardConfig({ + ...wizardConfig, + actionConfig, + }); + }, + setActionFactory: (actionFactory?: ActionFactory) => { + if (actionFactory) { + setWizardConfig({ + ...wizardConfig, + actionFactory, + actionConfig: actionConfigCache[actionFactory.id] ?? actionFactory.createConfig(), + }); + } else { + if (wizardConfig.actionFactory?.id) { + setActionConfigCache({ + ...actionConfigCache, + [wizardConfig.actionFactory.id]: wizardConfig.actionConfig!, + }); + } + + setWizardConfig({ + ...wizardConfig, + actionFactory: undefined, + actionConfig: undefined, + }); + } + }, + }, + ]; +} + export function FlyoutDrilldownWizard({ onClose, onBack, @@ -53,11 +119,8 @@ export function FlyoutDrilldownWizard) { - const [wizardConfig, setWizardConfig] = useState( - () => - initialDrilldownWizardConfig ?? { - name: '', - } + const [wizardConfig, { setActionFactory, setActionConfig, setName }] = useWizardConfigState( + initialDrilldownWizardConfig ); const isActionValid = ( @@ -95,35 +158,11 @@ export function FlyoutDrilldownWizard { - setWizardConfig({ - ...wizardConfig, - name: newName, - }); - }} + onNameChange={setName} actionConfig={wizardConfig.actionConfig} - onActionConfigChange={newActionConfig => { - setWizardConfig({ - ...wizardConfig, - actionConfig: newActionConfig, - }); - }} + onActionConfigChange={setActionConfig} currentActionFactory={wizardConfig.actionFactory} - onActionFactoryChange={actionFactory => { - if (!actionFactory) { - setWizardConfig({ - ...wizardConfig, - actionFactory: undefined, - actionConfig: undefined, - }); - } else { - setWizardConfig({ - ...wizardConfig, - actionFactory, - actionConfig: actionFactory.createConfig(), - }); - } - }} + onActionFactoryChange={setActionFactory} actionFactories={drilldownActionFactories} actionFactoryContext={actionFactoryContext!} /> diff --git a/x-pack/plugins/drilldowns/public/components/form_drilldown_wizard/form_drilldown_wizard.tsx b/x-pack/plugins/drilldowns/public/components/form_drilldown_wizard/form_drilldown_wizard.tsx index 93b3710bf6cc..3bed81a97192 100644 --- a/x-pack/plugins/drilldowns/public/components/form_drilldown_wizard/form_drilldown_wizard.tsx +++ b/x-pack/plugins/drilldowns/public/components/form_drilldown_wizard/form_drilldown_wizard.tsx @@ -19,7 +19,7 @@ export interface FormDrilldownWizardProps { onNameChange?: (name: string) => void; currentActionFactory?: ActionFactory; - onActionFactoryChange?: (actionFactory: ActionFactory | null) => void; + onActionFactoryChange?: (actionFactory?: ActionFactory) => void; actionFactoryContext: object; actionConfig?: object;