Repository: ProcessMaker/modeler (js)

Issue:
Advanced Accordion always last
The advanced accordion should always be the last accordion in the inspector panel:

<img width="284" alt="Screen Shot 2019-09-30 at 11 18 43 AM" src="https://user-images.githubusercontent.com/7561061/65892326-146e5f80-e374-11e9-8cb0-b4f8bac53849.png">

In some cases, the advanced accordion does not appear last, like for the task component:
<img width="291" alt="Screen Shot 2019-09-30 at 11 19 20 AM" src="https://user-images.githubusercontent.com/7561061/65892392-37990f00-e374-11e9-8a2c-0fdab0704184.png">

This occurs because calls to `registerInspectorExtension` does not maintain the order of the inspector accordions.

Pull request:
Set advanced accordion to last item in inspector
**Processmaker video**
https://drive.google.com/open?id=19XYJ2GgGpaP51A7TpGifi27xTO6W9n69

Fixes #709 

Patch:
diff --git a/src/components/InspectorExtensionManager.js b/src/components/InspectorExtensionManager.js
new file mode 100644
index 00000000..83fb1c83
--- /dev/null
+++ b/src/components/InspectorExtensionManager.js
@@ -0,0 +1,36 @@
+export default function registerInspectorExtension(node, config) {
+  const inspectorItems = getInspectorItems(node, config);
+  addInspectorItem(inspectorItems, config);
+  inspectorItems.sort(moveAdvancedAccordionToBottom);
+}
+
+function moveAdvancedAccordionToBottom(inspectorItemA, inspectorItemB) {
+  const isAdvancedAccordion = (inspectorItem) => inspectorItem.name === 'advanced';
+  if (isAdvancedAccordion(inspectorItemA)) {
+    return 1;
+  }
+
+  if (isAdvancedAccordion(inspectorItemB)) {
+    return -1;
+  }
+
+  return 0;
+}
+
+function addInspectorItem(inspectorItems, config) {
+  const nodeIndex = inspectorItems.findIndex(item => {
+    return config.id && config.id === item.id;
+  });
+  if (nodeIndex === -1) {
+    inspectorItems.push(config);
+    return;
+  }
+  inspectorItems[nodeIndex] = config;
+}
+
+function getInspectorItems(node, config) {
+  if (typeof config.container !== 'undefined') {
+    return node.inspectorConfig[0].items;
+  }
+  return node.inspectorConfig[0].items[0].items;
+}
diff --git a/src/components/Modeler.vue b/src/components/Modeler.vue
index 3c998dd3..248b6257 100644
--- a/src/components/Modeler.vue
+++ b/src/components/Modeler.vue
@@ -127,6 +127,7 @@ import { id as associationId } from './nodes/association';
 import { id as messageFlowId } from './nodes/messageFlow';
 
 import PaperManager from './paperManager';
+import registerInspectorExtension from '@/components/InspectorExtensionManager';
 
 const version = '1.0';
 
@@ -367,47 +368,6 @@ export default {
       }
       component.mixins.push(mixin);
     },
-    /**
-     * Register an inspector component to configure extended attributes and elements
-     * for specific bpmn extensions and execution environments. If the inspector
-     * component is already registered, it is replaced by the new one.
-     */
-    registerInspectorExtension(node, config) {
-      this.addToInspector(
-        this.inspectorItemsToAddTo(node, config),
-        this.existingConfigIndex(node, config),
-        config,
-      );
-    },
-    addToInspector(inspectorItems, existingIndex, config) {
-      if (existingIndex === -1) {
-        inspectorItems.push(config);
-      } else {
-        inspectorItems[existingIndex] = config;
-      }
-    },
-    existingConfigIndex(node, config) {
-      return this.inspectorItems(node).findIndex(item => {
-        return config.id && config.id === item.id;
-      });
-    },
-    isContainer(config) {
-      return typeof config.container !== 'undefined';
-    },
-    inspectorItemsToAddTo(node, config) {
-      if (this.isContainer(config)) {
-        return this.inspectorItems(node);
-      } else {
-        // if the config we're adding is not a container, put it in the default container
-        return this.defaultContainerItems(node);
-      }
-    },
-    defaultContainerItems(node) {
-      return this.inspectorItems(node)[0].items;
-    },
-    inspectorItems(node) {
-      return node.inspectorConfig[0].items;
-    },
     /**
      * Register a BPMN Moddle extension in order to support extensions to the bpmn xml format.
      * This is used to support new attributes and elements that would be needed for specific
@@ -864,10 +824,9 @@ export default {
     });
 
     this.registerNode(Process);
-
     /* Initialize the BpmnModdle and its extensions */
     window.ProcessMaker.EventBus.$emit('modeler-init', {
-      registerInspectorExtension: this.registerInspectorExtension,
+      registerInspectorExtension,
       registerBpmnExtension: this.registerBpmnExtension,
       registerNode: this.registerNode,
     });
diff --git a/src/components/inspectors/InspectorPanel.vue b/src/components/inspectors/InspectorPanel.vue
index 87dbdda2..99d39086 100644
--- a/src/components/inspectors/InspectorPanel.vue
+++ b/src/components/inspectors/InspectorPanel.vue
@@ -198,17 +198,17 @@ export default {
 .inspector-container {
   text-align: left;
   user-select: none;
-}
 
-.form-accordtion-container {
-  padding: 0;
-}
+  .form-group {
+    padding: 0 0.5rem;
 
-.form-group {
-  padding: 0 0.5rem;
-}
+    > label {
+      font-size: 0.8125rem;
+    }
+  }
 
-.inspector-font-size {
-  font-size: 0.875rem;
+  .inspector-font-size {
+    font-size: 0.875rem;
+  }
 }
 </style>


Test patch:
diff --git a/tests/unit/inspectorExtensionManager.spec.js b/tests/unit/inspectorExtensionManager.spec.js
new file mode 100644
index 00000000..cb8cde96
--- /dev/null
+++ b/tests/unit/inspectorExtensionManager.spec.js
@@ -0,0 +1,69 @@
+import registerInspectorExtension from '@/components/InspectorExtensionManager';
+
+describe('Inspector Extension Manager', function() {
+  let node;
+  let config;
+
+  beforeEach(() => {
+    node = {
+      inspectorConfig: [{
+        items: [
+          {
+            id: 1,
+          },
+          {
+            id: 2,
+          },
+        ],
+      }],
+    };
+    config = {
+      id: 5,
+      container: true,
+      name: 'advanced',
+    };
+  });
+
+  it('should always sort the advanced accordion to the end when the config is a container', function() {
+    registerInspectorExtension(node, config);
+
+    const [lastItem] = node.inspectorConfig[0].items.slice(-1);
+    expect(lastItem).toBe(config);
+  });
+
+  it('should always sort the advanced accordion to the end when the config is not a container', function() {
+    delete config.container;
+    const items = node.inspectorConfig[0].items;
+    node.inspectorConfig[0].items = [{ items }];
+
+    registerInspectorExtension(node, config);
+
+    const [lastItem] = node.inspectorConfig[0].items[0].items.slice(-1);
+    expect(lastItem).toBe(config);
+  });
+
+  it('should update items that have the same id', function() {
+    config.id = 1;
+
+    registerInspectorExtension(node, config);
+
+    const items = node.inspectorConfig[0].items;
+    const [lastItem] = node.inspectorConfig[0].items.slice(-1);
+    expect(items.length).toBe(2);
+    expect(lastItem).toBe(config);
+  });
+
+  it('should only move advanced types to the end', function() {
+    const secondConfig = {
+      id: 6,
+      name: 'Not advanced',
+      container: true,
+    };
+
+    registerInspectorExtension(node, config);
+    registerInspectorExtension(node, secondConfig);
+
+    const [lastItem] = node.inspectorConfig[0].items.slice(-1);
+    expect(lastItem).toBe(config);
+  });
+});