From bed0bf82d401f8bdbc2dc0dfe72076b176d09483 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 13 May 2020 10:13:36 +0200 Subject: [PATCH 1/3] Fix selector after selector refactor --- .../components/drag_and_drop_tree/drag_and_drop_tree.tsx | 7 ++++--- .../components/drag_and_drop_tree/tree_node.tsx | 5 ++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/drag_and_drop_tree.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/drag_and_drop_tree.tsx index 5fcb848f83d7c..6f147737d03d6 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/drag_and_drop_tree.tsx +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/drag_and_drop_tree.tsx @@ -64,6 +64,7 @@ export const DragAndDropTreeUI: FunctionComponent = ({ _processors.forEach((processor, idx) => { const index = flatTreeIndex++; const nodeSelector = _selector.concat(String(idx)); + const id = [treeId].concat(nodeSelector).join('.'); _items.push([ nodeSelector, = ({ index={index} level={level} processor={processor} - selector={nodeSelector} + id={id} + selector={baseSelector.concat(nodeSelector)} component={renderItem} - treeId={treeId} />, ]); @@ -88,7 +89,7 @@ export const DragAndDropTreeUI: FunctionComponent = ({ addRenderedItems(processors, [], 0); return _items; - }, [processors, renderItem, currentDragSelector]); + }, [processors, renderItem, currentDragSelector, baseSelector]); const treeId = `${readTreeId()}_PIPELINE_PROCESSORS_EDITOR`; diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/tree_node.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/tree_node.tsx index c13ff123c66da..72655e37a6052 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/tree_node.tsx +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/tree_node.tsx @@ -17,22 +17,21 @@ export interface TreeNodeComponentArgs { interface Props { component: (args: TreeNodeComponentArgs) => React.ReactNode; + id: string; selector: ProcessorSelector; processor: ProcessorInternal; - treeId: string; index: number; level: number; } export const TreeNode: FunctionComponent = ({ processor, + id, selector, index, component, level, - treeId, }) => { - const id = [treeId].concat(selector).join('.'); return ( {provided => ( From 99fa8cd0227c813ea8020a0c58e9a94c19debe39 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 13 May 2020 11:52:59 +0200 Subject: [PATCH 2/3] A major performance - Do not re-render the entire tree when editing settings --- .../components/drag_and_drop_tree/index.ts | 2 +- .../components/index.ts | 1 + .../pipeline_processors_editor/index.ts | 2 +- .../pipeline_processors_editor.container.tsx | 201 ++++++++++++++ .../pipeline_processors_editor.tsx | 253 ++++-------------- .../processors_reducer/index.ts | 2 +- .../processors_reducer/processors_reducer.ts | 4 +- 7 files changed, 267 insertions(+), 198 deletions(-) create mode 100644 x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/pipeline_processors_editor.container.tsx diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/index.ts b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/index.ts index 072923295b8aa..90ee2bbbb955f 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/index.ts +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/index.ts @@ -3,5 +3,5 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -export { DragAndDropTreeProvider } from './drag_and_drop_tree_provider'; +export { DragAndDropTreeProvider, OnDragEndArgs } from './drag_and_drop_tree_provider'; export { DragAndDropTree, RenderTreeItemFunction } from './drag_and_drop_tree'; diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/index.ts b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/index.ts index e35a262f2e79e..ba0d69d8d7f35 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/index.ts +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/index.ts @@ -12,6 +12,7 @@ export { DragAndDropTree, DragAndDropTreeProvider, RenderTreeItemFunction, + OnDragEndArgs, } from './drag_and_drop_tree'; export { PipelineProcessorEditorItem } from './pipeline_processor_editor_item'; diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/index.ts b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/index.ts index 63d4917fa3dac..c0d03994f0169 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/index.ts +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/index.ts @@ -8,4 +8,4 @@ export { PipelineProcessorsEditor, OnUpdateHandlerArg, OnUpdateHandler, -} from './pipeline_processors_editor'; +} from './pipeline_processors_editor.container'; diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/pipeline_processors_editor.container.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/pipeline_processors_editor.container.tsx new file mode 100644 index 0000000000000..04d66e473782f --- /dev/null +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/pipeline_processors_editor.container.tsx @@ -0,0 +1,201 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { + FunctionComponent, + useCallback, + useEffect, + useMemo, + useState, + Dispatch, +} from 'react'; + +import { Processor } from '../../../../common/types'; +import { OnFormUpdateArg } from '../../../shared_imports'; + +import { deserialize } from './serialize'; +import { ProcessorInternal, ProcessorSelector } from './types'; +import { useProcessorsState } from './processors_reducer'; +import { serialize, SerializeResult } from './deserialize'; +import { ProcessorRemoveModal, SettingsFormFlyout } from './components'; +import { getValue } from './utils'; + +import { PipelineProcessorsEditor as PipelineProcessorsEditorUI } from './pipeline_processors_editor'; + +export interface Props { + value: { + processors: Processor[]; + onFailure?: Processor[]; + }; + onUpdate: (arg: OnUpdateHandlerArg) => void; + isTestButtonDisabled: boolean; + onTestPipelineClick: () => void; + learnMoreAboutProcessorsUrl: string; + learnMoreAboutOnFailureProcessorsUrl: string; +} + +/** + * The settings form can be in different modes. This enables us to hold + * a reference to data dispatch to * the reducer (like the {@link ProcessorSelector} + * which will be used to update the in-memory processors data structure. + */ +type SettingsFormMode = + | { id: 'creatingTopLevelProcessor'; arg: ProcessorSelector } + | { id: 'creatingOnFailureProcessor'; arg: ProcessorSelector } + | { id: 'editingProcessor'; arg: { processor: ProcessorInternal; selector: ProcessorSelector } } + | { id: 'closed' }; + +export type SetSettingsFormMode = Dispatch; + +interface FormValidityState { + validate: OnFormUpdateArg['validate']; +} + +export interface OnUpdateHandlerArg extends FormValidityState { + getData: () => SerializeResult; +} + +export type OnUpdateHandler = (arg: OnUpdateHandlerArg) => void; + +export const PipelineProcessorsEditor: FunctionComponent = ({ + value: { processors: originalProcessors, onFailure: originalOnFailureProcessors }, + onUpdate, + isTestButtonDisabled, + learnMoreAboutOnFailureProcessorsUrl, + learnMoreAboutProcessorsUrl, + onTestPipelineClick, +}) => { + const deserializedResult = useMemo( + () => deserialize({ processors: originalProcessors, onFailure: originalOnFailureProcessors }), + [originalProcessors, originalOnFailureProcessors] + ); + + const [processorToDeleteSelector, setProcessorToDeleteSelector] = useState< + ProcessorSelector | undefined + >(); + + const [settingsFormMode, setSettingsFormMode] = useState({ id: 'closed' }); + const [processorsState, processorsDispatch] = useProcessorsState(deserializedResult); + const { processors, onFailure } = processorsState; + + const [formState, setFormState] = useState({ + validate: () => Promise.resolve(true), + }); + + const onFormUpdate = useCallback( + arg => { + setFormState({ validate: arg.validate }); + }, + [setFormState] + ); + + useEffect(() => { + onUpdate({ + validate: async () => { + const formValid = await formState.validate(); + return formValid && settingsFormMode.id === 'closed'; + }, + getData: () => serialize(processorsState), + }); + }, [processorsState, onUpdate, formState, settingsFormMode]); + + const onSubmit = useCallback( + processorTypeAndOptions => { + switch (settingsFormMode.id) { + case 'creatingTopLevelProcessor': + processorsDispatch({ + type: 'addTopLevelProcessor', + payload: { processor: processorTypeAndOptions, selector: settingsFormMode.arg }, + }); + break; + case 'creatingOnFailureProcessor': + processorsDispatch({ + type: 'addOnFailureProcessor', + payload: { + onFailureProcessor: processorTypeAndOptions, + targetSelector: settingsFormMode.arg, + }, + }); + break; + case 'editingProcessor': + processorsDispatch({ + type: 'updateProcessor', + payload: { + processor: { + ...settingsFormMode.arg.processor, + ...processorTypeAndOptions, + }, + selector: settingsFormMode.arg.selector, + }, + }); + break; + default: + } + dismissFlyout(); + }, + [processorsDispatch, settingsFormMode] + ); + + const onDragEnd = useCallback( + args => { + processorsDispatch({ + type: 'moveProcessor', + payload: args, + }); + }, + [processorsDispatch] + ); + + const dismissFlyout = () => { + setSettingsFormMode({ id: 'closed' }); + }; + + return ( + <> + + {settingsFormMode.id !== 'closed' ? ( + { + dismissFlyout(); + setFormState({ validate: () => Promise.resolve(true) }); + }} + /> + ) : ( + undefined + )} + {processorToDeleteSelector && ( + { + if (confirmed) { + processorsDispatch({ + type: 'removeProcessor', + payload: { selector: processorToDeleteSelector }, + }); + } + setProcessorToDeleteSelector(undefined); + }} + /> + )} + + ); +}; diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/pipeline_processors_editor.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/pipeline_processors_editor.tsx index 14a97ca3e86b2..d35cc059daf89 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/pipeline_processors_editor.tsx +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/pipeline_processors_editor.tsx @@ -5,193 +5,87 @@ */ import { i18n } from '@kbn/i18n'; -import React, { FunctionComponent, useState, useMemo, useEffect, useCallback } from 'react'; +import React, { FunctionComponent, useCallback, memo } from 'react'; import { EuiButtonEmpty, EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui'; -import { Processor } from '../../../../common/types'; - -import { OnFormUpdateArg } from '../../../shared_imports'; - import './pipeline_processors_editor.scss'; import { - SettingsFormFlyout, DragAndDropTree, RenderTreeItemFunction, + OnDragEndArgs, PipelineProcessorEditorItem, - ProcessorRemoveModal, ProcessorsTitleAndTestButton, OnFailureProcessorsTitle, DragAndDropTreeProvider, } from './components'; -import { deserialize } from './serialize'; -import { serialize, SerializeResult } from './deserialize'; -import { useProcessorsState } from './processors_reducer/processors_reducer'; -import { ProcessorInternal, ProcessorSelector } from './types'; -import { getValue } from './utils'; - -interface FormValidityState { - validate: OnFormUpdateArg['validate']; -} -export interface OnUpdateHandlerArg extends FormValidityState { - getData: () => SerializeResult; -} - -export type OnUpdateHandler = (arg: OnUpdateHandlerArg) => void; +import { ProcessorInternal, ProcessorSelector } from './types'; export interface Props { - value: { - processors: Processor[]; - onFailure?: Processor[]; - }; - onUpdate: (arg: OnUpdateHandlerArg) => void; + processors: ProcessorInternal[]; + onFailureProcessors: ProcessorInternal[]; + processorsDispatch: import('./processors_reducer').ProcessorsDispatch; + setSettingsFormMode: import('./pipeline_processors_editor.container').SetSettingsFormMode; + setProcessorToDeleteSelector: (processor: ProcessorSelector) => void; isTestButtonDisabled: boolean; onTestPipelineClick: () => void; learnMoreAboutProcessorsUrl: string; learnMoreAboutOnFailureProcessorsUrl: string; + onDragEnd: (args: OnDragEndArgs) => void; } const PROCESSOR_STATE_SCOPE: ProcessorSelector = ['processors']; const ON_FAILURE_STATE_SCOPE: ProcessorSelector = ['onFailure']; -/** - * The settings form can be in different modes. This enables us to hold - * a reference to data dispatch to * the reducer (like the {@link ProcessorSelector} - * which will be used to update the in-memory processors data structure. - */ -type SettingsFormMode = - | { id: 'creatingTopLevelProcessor'; arg: ProcessorSelector } - | { id: 'creatingOnFailureProcessor'; arg: ProcessorSelector } - | { id: 'editingProcessor'; arg: { processor: ProcessorInternal; selector: ProcessorSelector } } - | { id: 'closed' }; - -export const PipelineProcessorsEditor: FunctionComponent = ({ - value: { processors: originalProcessors, onFailure: originalOnFailureProcessors }, - onUpdate, - onTestPipelineClick, - learnMoreAboutProcessorsUrl, - isTestButtonDisabled, - learnMoreAboutOnFailureProcessorsUrl, -}) => { - const deserializedResult = useMemo( - () => deserialize({ processors: originalProcessors, onFailure: originalOnFailureProcessors }), - [originalProcessors, originalOnFailureProcessors] - ); - - const [processorToDeleteSelector, setProcessorToDeleteSelector] = useState< - ProcessorSelector | undefined - >(); - const [settingsFormMode, setSettingsFormMode] = useState({ id: 'closed' }); - const [processorsState, processorsDispatch] = useProcessorsState(deserializedResult); - const { processors, onFailure } = processorsState; - - const [formState, setFormState] = useState({ - validate: () => Promise.resolve(true), - }); - - const onFormUpdate = useCallback( - arg => { - setFormState({ validate: arg.validate }); - }, - [setFormState] - ); - - useEffect(() => { - onUpdate({ - validate: async () => { - const formValid = await formState.validate(); - return formValid && settingsFormMode.id === 'closed'; - }, - getData: () => serialize(processorsState), - }); - }, [processorsState, onUpdate, formState, settingsFormMode]); - - const onSubmit = useCallback( - processorTypeAndOptions => { - switch (settingsFormMode.id) { - case 'creatingTopLevelProcessor': - processorsDispatch({ - type: 'addTopLevelProcessor', - payload: { processor: processorTypeAndOptions, selector: settingsFormMode.arg }, - }); - break; - case 'creatingOnFailureProcessor': - processorsDispatch({ - type: 'addOnFailureProcessor', - payload: { - onFailureProcessor: processorTypeAndOptions, - targetSelector: settingsFormMode.arg, - }, - }); - break; - case 'editingProcessor': - processorsDispatch({ - type: 'updateProcessor', - payload: { - processor: { - ...settingsFormMode.arg.processor, - ...processorTypeAndOptions, - }, - selector: settingsFormMode.arg.selector, - }, - }); - break; - default: - } - dismissFlyout(); - }, - [processorsDispatch, settingsFormMode] - ); - - const onDragEnd = useCallback( - args => { - processorsDispatch({ - type: 'moveProcessor', - payload: args, - }); - }, - [processorsDispatch] - ); - - const dismissFlyout = () => { - setSettingsFormMode({ id: 'closed' }); - }; - - const renderTreeItem = useCallback( - ({ processor, selector }) => ( - { - switch (type) { - case 'edit': - setSettingsFormMode({ - id: 'editingProcessor', - arg: { processor, selector }, - }); - break; - case 'delete': - if (processor.onFailure?.length) { - setProcessorToDeleteSelector(selector); - } else { - processorsDispatch({ - type: 'removeProcessor', - payload: { selector }, - }); +export const PipelineProcessorsEditor: FunctionComponent = memo( + function PipelineProcessorsEditor({ + processors, + onFailureProcessors, + setSettingsFormMode, + setProcessorToDeleteSelector, + processorsDispatch, + onTestPipelineClick, + learnMoreAboutProcessorsUrl, + isTestButtonDisabled, + learnMoreAboutOnFailureProcessorsUrl, + onDragEnd, + }) { + const renderTreeItem = useCallback( + ({ processor, selector }) => { + return ( + { + switch (type) { + case 'edit': + setSettingsFormMode({ + id: 'editingProcessor', + arg: { processor, selector }, + }); + break; + case 'delete': + if (processor.onFailure?.length) { + setProcessorToDeleteSelector(selector); + } else { + processorsDispatch({ + type: 'removeProcessor', + payload: { selector }, + }); + } + break; + case 'addOnFailure': + setSettingsFormMode({ id: 'creatingOnFailureProcessor', arg: selector }); + break; } - break; - case 'addOnFailure': - setSettingsFormMode({ id: 'creatingOnFailureProcessor', arg: selector }); - break; - } - }} - processor={processor} - /> - ), - [processorsDispatch] - ); + }} + processor={processor} + /> + ); + }, + [processorsDispatch, setSettingsFormMode, setProcessorToDeleteSelector] + ); - return ( - <> + return ( @@ -263,7 +157,7 @@ export const PipelineProcessorsEditor: FunctionComponent = ({ @@ -299,35 +193,6 @@ export const PipelineProcessorsEditor: FunctionComponent = ({ - {settingsFormMode.id !== 'closed' ? ( - { - dismissFlyout(); - setFormState({ validate: () => Promise.resolve(true) }); - }} - /> - ) : ( - undefined - )} - {processorToDeleteSelector && ( - { - if (confirmed) { - processorsDispatch({ - type: 'removeProcessor', - payload: { selector: processorToDeleteSelector }, - }); - } - setProcessorToDeleteSelector(undefined); - }} - /> - )} - - ); -}; + ); + } +); diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/processors_reducer/index.ts b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/processors_reducer/index.ts index 97224f12c4855..cd9875f235949 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/processors_reducer/index.ts +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/processors_reducer/index.ts @@ -4,4 +4,4 @@ * you may not use this file except in compliance with the Elastic License. */ -export { State, reducer, useProcessorsState } from './processors_reducer'; +export { State, reducer, useProcessorsState, ProcessorsDispatch } from './processors_reducer'; diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/processors_reducer/processors_reducer.ts b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/processors_reducer/processors_reducer.ts index 130b22442b18d..41c68d488227d 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/processors_reducer/processors_reducer.ts +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/processors_reducer/processors_reducer.ts @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { Reducer, useReducer } from 'react'; +import { Reducer, useReducer, Dispatch } from 'react'; import { DeserializeResult } from '../serialize'; import { unsafeProcessorMove } from './utils'; @@ -40,6 +40,8 @@ type Action = payload: { source: ProcessorSelector; destination: ProcessorSelector }; }; +export type ProcessorsDispatch = Dispatch; + export const reducer: Reducer = (state, action) => { if (action.type === 'moveProcessor') { const { destination, source } = action.payload; From 0f477f3742b51e04f11e7a87c96890d13e3fbb2b Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 13 May 2020 11:56:21 +0200 Subject: [PATCH 3/3] Fix component smoke test --- .../__jest__/pipeline_processors_editor_helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/__jest__/pipeline_processors_editor_helpers.ts b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/__jest__/pipeline_processors_editor_helpers.ts index cb4e91bc43f04..1fb1f6175ceae 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/__jest__/pipeline_processors_editor_helpers.ts +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/__jest__/pipeline_processors_editor_helpers.ts @@ -5,7 +5,7 @@ */ import { registerTestBed } from '../../../../../../../test_utils'; -import { PipelineProcessorsEditor, Props } from '../pipeline_processors_editor'; +import { PipelineProcessorsEditor, Props } from '../pipeline_processors_editor.container'; const testBedSetup = registerTestBed(PipelineProcessorsEditor, { doMountAsync: false,