Merge pull request #797 from plotly/adjust-panels-sort

sortMenu adjustments

package.json

@@ -1,7 +1,7 @@
 {
   "name": "react-chart-editor",
   "description": "plotly.js chart editor react component UI",
-  "version": "0.33.3",
+  "version": "0.33.4",
   "author": "Plotly, Inc.",
   "bugs": {
     "url": "https://github.com/plotly/react-chart-editor/issues"
@@ -125,4 +125,4 @@
     "watch": "babel src --watch --out-dir lib --source-maps | node-sass -w src/styles/main.scss lib/react-chart-editor.css",
     "watch-test": "jest --watch"
   }
-}
+}

src/components/PanelMenuWrapper.js

@@ -51,13 +51,9 @@ class PanelsWithSidebar extends Component {
     const sections = [];
     const groupLookup = {};
     let groupIndex;
-    const panels = React.Children.toArray(children);
+    const childrenArray = sortMenu(React.Children.toArray(children), menuPanelOrder);
 
-    if (menuPanelOrder) {
-      sortMenu(panels, menuPanelOrder);
-    }
-
-    panels.forEach(child => {
+    childrenArray.forEach(child => {
       if (!child) {
         return;
       }

src/lib/__tests__/sortMenu-test.js

@@ -1,15 +1,19 @@
 import sortMenu from '../sortMenu';
 
 describe('sortMenu', () => {
-  it('modifies original array to follow the group, then name order provided', () => {
+  it('returns a new array of sorted children', () => {
     const initialArray = [
       {props: {group: 'DEV', name: 'Inspector'}},
       {props: {group: 'DEV', name: 'JSON'}},
     ];
     const orderProp = [{group: 'DEV', name: 'JSON'}, {group: 'DEV', name: 'Inspector'}];
+    const newArray = sortMenu(initialArray, orderProp);
 
-    sortMenu(initialArray, orderProp);
     expect(initialArray).toEqual([
+      {props: {group: 'DEV', name: 'Inspector'}},
+      {props: {group: 'DEV', name: 'JSON'}},
+    ]);
+    expect(newArray).toEqual([
       {props: {group: 'DEV', name: 'JSON'}},
       {props: {group: 'DEV', name: 'Inspector'}},
     ]);
@@ -32,9 +36,9 @@ describe('sortMenu', () => {
       {group: 'Style', name: 'Color Bars'},
       {group: 'Style', name: 'Annotation'},
     ];
+    const newArray = sortMenu(initialArray, orderProp);
 
-    sortMenu(initialArray, orderProp);
-    expect(initialArray).toEqual([
+    expect(newArray).toEqual([
       {props: {group: 'DEV', name: 'JSON'}},
       {props: {group: 'DEV', name: 'Inspector'}},
       {props: {group: 'Structure', name: 'Subplots'}},
@@ -59,9 +63,9 @@ describe('sortMenu', () => {
       {group: 'Style', name: 'Color Bars'},
       {group: 'Style', name: 'Annotation'},
     ];
+    const newArray = sortMenu(initialArray, orderProp);
 
-    sortMenu(initialArray, orderProp);
-    expect(initialArray).toEqual([
+    expect(newArray).toEqual([
       {props: {group: 'Structure', name: 'Subplots'}},
       {props: {group: 'Structure', name: 'Create'}},
       {props: {group: 'Style', name: 'Color Bars'}},
@@ -79,9 +83,9 @@ describe('sortMenu', () => {
       {props: {group: 'Structure', name: 'Create'}},
     ];
     const orderProp = [{group: 'Style', name: 'Traces'}];
+    const newArray = sortMenu(initialArray, orderProp);
 
-    sortMenu(initialArray, orderProp);
-    expect(initialArray).toEqual([
+    expect(newArray).toEqual([
       {props: {group: 'Style', name: 'Traces'}},
       {props: {group: 'Style', name: 'Axes'}},
       {props: {group: 'Style', name: 'General'}},
@@ -96,16 +100,15 @@ describe('sortMenu', () => {
       {props: {group: 'Style', name: 'Color Bars'}},
       {props: {group: 'Style', name: 'Annotation'}},
     ];
-
     const orderProp = [
       {group: 'Non Existent', name: 'Subplots'},
       {group: 'Structure', name: 'Create'},
       {group: 'Style', name: 'Color Bars'},
       {group: 'Style', name: 'Annotation'},
     ];
+    const newArray = sortMenu(initialArray, orderProp);
 
-    sortMenu(initialArray, orderProp);
-    expect(initialArray).toEqual([
+    expect(newArray).toEqual([
       {props: {group: 'Structure', name: 'Create'}},
       {props: {group: 'Structure', name: 'Subplots'}},
       {props: {group: 'Style', name: 'Color Bars'}},
@@ -120,15 +123,14 @@ describe('sortMenu', () => {
       {props: {group: 'Style', name: 'Color Bars'}},
       {props: {group: 'Style', name: 'Annotation'}},
     ];
-
     const orderProp = [
       {group: 'Structure', name: 'Non Existent'},
       {group: 'Style', name: 'Color Bars'},
       {group: 'Style', name: 'Annotation'},
     ];
+    const newArray = sortMenu(initialArray, orderProp);
 
-    sortMenu(initialArray, orderProp);
-    expect(initialArray).toEqual([
+    expect(newArray).toEqual([
       {props: {group: 'Style', name: 'Color Bars'}},
       {props: {group: 'Style', name: 'Annotation'}},
       {props: {group: 'Structure', name: 'Create'}},
@@ -143,19 +145,43 @@ describe('sortMenu', () => {
       {props: {group: 'Style', name: 'Color Bars'}},
       {props: {group: 'Style', name: 'Annotation'}},
     ];
-
     const orderProp = [
       {group: 'Structure', name: 'Annotation'},
       {group: 'Style', name: 'Color Bars'},
       {group: 'Style', name: 'Annotation'},
     ];
+    const newArray = sortMenu(initialArray, orderProp);
 
-    sortMenu(initialArray, orderProp);
-    expect(initialArray).toEqual([
+    expect(newArray).toEqual([
       {props: {group: 'Style', name: 'Color Bars'}},
       {props: {group: 'Style', name: 'Annotation'}},
       {props: {group: 'Structure', name: 'Create'}},
       {props: {group: 'Structure', name: 'Subplots'}},
     ]);
   });
+
+  it('does not sort children with no group or name props', () => {
+    const initialArray = [
+      {props: {type: 'Logo'}},
+      {props: {group: 'A', name: 'A'}},
+      {props: {group: 'Structure', name: 'Subplots'}},
+      {props: {group: 'Structure', name: 'Create'}},
+      {props: {type: 'ButtonA'}},
+      {props: {type: 'ButtonB'}},
+    ];
+    const orderProp = [
+      {group: 'Structure', name: 'Create'},
+      {group: 'Structure', name: 'Subplots'},
+    ];
+    const newArray = sortMenu(initialArray, orderProp);
+
+    expect(newArray).toEqual([
+      {props: {type: 'Logo'}},
+      {props: {group: 'Structure', name: 'Create'}},
+      {props: {group: 'Structure', name: 'Subplots'}},
+      {props: {group: 'A', name: 'A'}},
+      {props: {type: 'ButtonA'}},
+      {props: {type: 'ButtonB'}},
+    ]);
+  });
 });

src/lib/sortMenu.js

@@ -8,10 +8,39 @@ function sortAlphabetically(a, b) {
   return sortByGroup || sortByName;
 }
 
-export default function sortMenu(panels, order) {
-  // validates order, if a desired panel matches no panel in the panels array,
-  // it is excluded from ordering considerations
+export default function sortMenu(children, order) {
+  // Break out early if no order is provided
+  if (!order) {
+    return children;
+  }
 
+  // PART 1: only sorting panels (i.e. child with a group and name prop)
+  // and not other elements (like Buttons, or Logo)
+  let panelsStartIndex = null;
+  let panelsEndIndex = null;
+  for (let i = 0; i < children.length; i++) {
+    if (children[i].props.group && children[i].props.name && !panelsStartIndex) {
+      panelsStartIndex = i;
+      break;
+    }
+  }
+  for (let i = panelsStartIndex; i < children.length; i++) {
+    if (!children[i].props.group && !children[i].props.name && !panelsEndIndex) {
+      panelsEndIndex = i - 1;
+      break;
+    } else if (i === children.length - 1) {
+      panelsEndIndex = i;
+    }
+  }
+
+  const prePanelsChildren = panelsStartIndex === 0 ? [] : children.slice(0, panelsStartIndex);
+  const panels =
+    panelsStartIndex !== panelsEndIndex ? children.slice(panelsStartIndex, panelsEndIndex + 1) : [];
+  const postPanelsChildren =
+    panelsEndIndex === children.length ? [] : children.slice(panelsEndIndex + 1);
+
+  // PART 2: validate order prop, if a desired panel specified in order, matches no actual panel rendered
+  // in the panels array, it is excluded from ordering considerations
   // eslint-disable-next-line
   order = order.filter(desiredPanel =>
     panels.some(
@@ -22,8 +51,8 @@ export default function sortMenu(panels, order) {
   );
 
   const desiredGroupOrder = order.map(panel => panel.group).filter(getUniqueValues);
-  const desiredNameOrder = order.map(panel => panel.name).filter(getUniqueValues);
 
+  // PART 3: Sort panels
   panels.sort((a, b) => {
     const panelAHasGroupCustomOrder = desiredGroupOrder.includes(a.props.group);
     const panelBHasGroupCustomOrder = desiredGroupOrder.includes(b.props.group);
@@ -57,6 +86,16 @@ export default function sortMenu(panels, order) {
       }
 
       if (indexOfGroupA === indexOfGroupB) {
+        // Since Subpanels names can be reused in different groups
+        // we compute desired order here to get the desired index right.
+        // We are filtering on unique values, just in case, even if we don't
+        // have to because within a given group we'd assume that there will be
+        // no 2 subpanels named the same.
+        const desiredNameOrder = order
+          .filter(panel => panel.group === a.props.group)
+          .map(panel => panel.name)
+          .filter(getUniqueValues);
+
         const panelAHasNameCustomOrder = desiredNameOrder.includes(a.props.name);
         const panelBHasNameCustomOrder = desiredNameOrder.includes(b.props.name);
 
@@ -79,4 +118,7 @@ export default function sortMenu(panels, order) {
     }
     return 0;
   });
+
+  // PART 4: Return all children
+  return prePanelsChildren.concat(panels).concat(postPanelsChildren);
 }