[Grouping] improvements (elastic#27581)

* chore: remove asynchronous element selection * chore: remove duplication between duplicateElement and rawDuplicateElement * chore: renamed rawDuplicateElement to insertNodes (pure rename, not yet set based) * perf: bulk node insertion * chore: removed unnecessary element id duplications * fix: supply empty array if preexisting workbook had none
monfera · Dec 27, 2018 · 26e4b2b · 26e4b2b
1 parent 36f35f9
commit 26e4b2b
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 73 deletions.
diff --git a/x-pack/plugins/canvas/public/components/sidebar/index.js b/x-pack/plugins/canvas/public/components/sidebar/index.js
@@ -5,8 +5,10 @@
  */
 
 import { connect } from 'react-redux';
-import { duplicateElement, elementLayer } from '../../state/actions/elements';
+import { cloneSubgraphs } from '../../lib/clone_subgraphs';
+import { insertNodes, elementLayer } from '../../state/actions/elements';
 import { getSelectedPage, getSelectedElement } from '../../state/selectors/workpad';
+import { selectElement } from './../../state/actions/transient';
 
 import { Sidebar as Component } from './sidebar';
 
@@ -16,8 +18,13 @@ const mapStateToProps = state => ({
 });
 
 const mapDispatchToProps = dispatch => ({
-  duplicateElement: (pageId, selectedElement) => () =>
-    dispatch(duplicateElement(selectedElement, pageId)),
+  duplicateElement: (pageId, selectedElement) => () => {
+    // gradually unifying code with copy/paste
+    // todo: more unification w/ copy/paste; group cloning
+    const newElements = cloneSubgraphs([selectedElement]);
+    dispatch(insertNodes(newElements, pageId));
+    dispatch(selectElement(newElements[0].id));
+  },
   elementLayer: (pageId, selectedElement) => movement =>
     dispatch(
       elementLayer({

diff --git a/x-pack/plugins/canvas/public/components/workpad_page/index.js b/x-pack/plugins/canvas/public/components/workpad_page/index.js
@@ -11,12 +11,13 @@ import { notify } from '../../lib/notify';
 import { aeroelastic } from '../../lib/aeroelastic_kibana';
 import { setClipboardData, getClipboardData } from '../../lib/clipboard';
 import { cloneSubgraphs } from '../../lib/clone_subgraphs';
-import { removeElements, rawDuplicateElement } from '../../state/actions/elements';
+import { removeElements, insertNodes } from '../../state/actions/elements';
 import { getFullscreen, canUserWrite } from '../../state/selectors/app';
 import { getNodes, isWriteable } from '../../state/selectors/workpad';
 import { flatten } from '../../lib/aeroelastic/functional';
 import { withEventHandlers } from './event_handlers';
 import { WorkpadPage as Component } from './workpad_page';
+import { selectElement } from './../../state/actions/transient';
 
 const mapStateToProps = (state, ownProps) => {
   return {
@@ -27,9 +28,9 @@ const mapStateToProps = (state, ownProps) => {
 
 const mapDispatchToProps = dispatch => {
   return {
-    rawDuplicateElement: pageId => (selectedElement, root) =>
-      dispatch(rawDuplicateElement(selectedElement, pageId, root)),
+    insertNodes: pageId => selectedElements => dispatch(insertNodes(selectedElements, pageId)),
     removeElements: pageId => elementIds => dispatch(removeElements(elementIds, pageId)),
+    selectElement: selectedElement => dispatch(selectElement(selectedElement)),
   };
 };
 
@@ -80,8 +81,9 @@ export const WorkpadPage = compose(
       setUpdateCount,
       page,
       elements: pageElements,
-      rawDuplicateElement,
+      insertNodes,
       removeElements,
+      selectElement,
     }) => {
       const { shapes, selectedPrimaryShapes = [], cursor } = aeroelastic.getStore(
         page.id
@@ -150,12 +152,23 @@ export const WorkpadPage = compose(
         },
         pasteElements: () => {
           const { selectedElements, rootShapes } = JSON.parse(getClipboardData());
-          const indices = rootShapes.map(r => selectedElements.findIndex(s => s.id === r));
           const clonedElements = selectedElements && cloneSubgraphs(selectedElements);
           if (clonedElements) {
-            clonedElements.map((element, index) =>
-              rawDuplicateElement(page.id)(element, indices.indexOf(index) >= 0)
-            );
+            // first clone and persist the new node(s)
+            insertNodes(page.id)(clonedElements);
+            // then select the cloned node
+            if (rootShapes.length) {
+              if (selectedElements.length > 1) {
+                // adHocGroup branch (currently, pasting will leave only the 1st element selected, rather than forming a
+                // new adHocGroup - todo)
+                selectElement(clonedElements[0].id);
+              } else {
+                // single element or single persistentGroup branch
+                selectElement(
+                  clonedElements[selectedElements.findIndex(s => s.id === rootShapes[0])].id
+                );
+              }
+            }
           }
         },
       };

diff --git a/x-pack/plugins/canvas/public/state/actions/elements.js b/x-pack/plugins/canvas/public/state/actions/elements.js
@@ -192,51 +192,23 @@ export const fetchAllRenderables = createThunk(
   }
 );
 
-export const duplicateElement = createThunk(
-  'duplicateElement',
-  ({ dispatch, type }, element, pageId) => {
-    const newElement = { ...getDefaultElement(), ...getBareElement(element) };
-    // move the element so users can see that it was added
+export const insertNodes = createThunk('insertNodes', ({ dispatch, type }, elements, pageId) => {
+  const _insertNodes = createAction(type);
+  const newElements = elements.map(cloneDeep);
+  // move the root element so users can see that it was added
+  newElements.forEach(newElement => {
     newElement.position.top = newElement.position.top + 10;
     newElement.position.left = newElement.position.left + 10;
-    const _duplicateElement = createAction(type);
-    dispatch(_duplicateElement({ pageId, element: newElement }));
+  });
+  dispatch(_insertNodes({ pageId, elements: newElements }));
 
-    // refresh all elements if there's a filter, otherwise just render the new element
-    if (element.filter) {
-      dispatch(fetchAllRenderables());
-    } else {
-      dispatch(fetchRenderable(newElement));
-    }
-
-    // select the new element
-    dispatch(selectElement(newElement.id));
-  }
-);
-
-export const rawDuplicateElement = createThunk(
-  'rawDuplicateElement',
-  ({ dispatch, type }, element, pageId, root) => {
-    const newElement = cloneDeep(element);
-    // move the root element so users can see that it was added
-    newElement.position.top = newElement.position.top + 10;
-    newElement.position.left = newElement.position.left + 10;
-    const _rawDuplicateElement = createAction(type);
-    dispatch(_rawDuplicateElement({ pageId, element: newElement }));
-
-    // refresh all elements if there's a filter, otherwise just render the new element
-    if (element.filter) {
-      dispatch(fetchAllRenderables());
-    } else {
-      dispatch(fetchRenderable(newElement));
-    }
-
-    // select the new element
-    if (root) {
-      window.setTimeout(() => dispatch(selectElement(newElement.id)));
-    }
+  // refresh all elements just once per `insertNodes call` if there's a filter on any, otherwise just render the new element
+  if (elements.some(element => element.filter)) {
+    dispatch(fetchAllRenderables());
+  } else {
+    newElements.forEach(newElement => dispatch(fetchRenderable(newElement)));
   }
-);
+});
 
 export const removeElements = createThunk(
   'removeElements',

diff --git a/x-pack/plugins/canvas/public/state/middleware/aeroelastic.js b/x-pack/plugins/canvas/public/state/middleware/aeroelastic.js
@@ -12,8 +12,7 @@ import defaultConfiguration from '../../lib/aeroelastic/config';
 import {
   addElement,
   removeElements,
-  duplicateElement,
-  rawDuplicateElement,
+  insertNodes,
   elementLayer,
   setMultiplePositions,
   fetchAllRenderables,
@@ -332,8 +331,7 @@ export const aeroelastic = ({ dispatch, getState }) => {
 
       case removeElements.toString():
       case addElement.toString():
-      case duplicateElement.toString():
-      case rawDuplicateElement.toString():
+      case insertNodes.toString():
       case elementLayer.toString():
       case setMultiplePositions.toString():
         const page = getSelectedPage(getState());

diff --git a/x-pack/plugins/canvas/public/state/reducers/elements.js b/x-pack/plugins/canvas/public/state/reducers/elements.js
@@ -10,6 +10,7 @@ import { get } from 'lodash';
 import * as actions from '../actions/elements';
 
 const getLocation = type => (type === 'group' ? 'groups' : 'elements');
+const firstOccurrence = (element, index, array) => array.indexOf(element) === index;
 
 const getLocationFromIds = (workpadState, pageId, nodeId) => {
   const page = workpadState.pages.find(p => p.id === pageId);
@@ -122,26 +123,19 @@ export const elementsReducer = handleActions(
         trimElement(element)
       );
     },
-    [actions.duplicateElement]: (workpadState, { payload: { pageId, element } }) => {
+    [actions.insertNodes]: (workpadState, { payload: { pageId, elements } }) => {
       const pageIndex = getPageIndexById(workpadState, pageId);
       if (pageIndex < 0) {
         return workpadState;
       }
-      return push(
-        workpadState,
-        ['pages', pageIndex, getLocation(element.position.type)],
-        trimElement(element)
-      );
-    },
-    [actions.rawDuplicateElement]: (workpadState, { payload: { pageId, element } }) => {
-      const pageIndex = getPageIndexById(workpadState, pageId);
-      if (pageIndex < 0) {
-        return workpadState;
-      }
-      return push(
-        workpadState,
-        ['pages', pageIndex, getLocation(element.position.type)],
-        trimElement(element)
+      return elements.reduce(
+        (state, element) =>
+          push(
+            state,
+            ['pages', pageIndex, getLocation(element.position.type)],
+            trimElement(element)
+          ),
+        workpadState
       );
     },
     [actions.removeElements]: (workpadState, { payload: { pageId, elementIds } }) => {
@@ -151,6 +145,7 @@ export const elementsReducer = handleActions(
       }
 
       const nodeIndices = elementIds
+        .filter(firstOccurrence)
         .map(nodeId => {
           const location = getLocationFromIds(workpadState, pageId, nodeId);
           return {

diff --git a/x-pack/plugins/canvas/public/state/reducers/pages.js b/x-pack/plugins/canvas/public/state/reducers/pages.js
@@ -31,7 +31,7 @@ function clonePage(page) {
   // TODO: would be nice if we could more reliably know which parameters need to get a unique id
   // this makes a pretty big assumption about the shape of the page object
   const elements = page.elements;
-  const groups = page.groups;
+  const groups = page.groups || [];
   const nodes = elements.concat(groups);
   const newNodes = cloneSubgraphs(nodes);
   return {