Merge pull request #88 from Exabyte-io/fix/SOF-6283

fix: page crashing when some item is deleted

.eslintrc.json

@@ -27,6 +27,7 @@
         "max-len": "off",
         "no-console": "off",
         "no-unused-expressions": "warn",
+        "react/forbid-prop-types": "warn",
         "no-confusing-arrow": "off",
         "no-underscore-dangle": "off",
         "no-return-assign": "off",

src/actions/InputOutput.js

@@ -25,7 +25,7 @@ export const MATERIALS_REMOVE = "MATERIALS_REMOVE";
 
 export function removeMaterials(indices) {
     // if `indices` is undefined => removing by current index; passing empty array
-    const passedIndices = indices || [];
+    const passedIndices = indices ?? [];
     return {
         type: MATERIALS_REMOVE,
         indices: safeMakeArrayIfNot(passedIndices),

src/components/include/ModalDialog.jsx

@@ -46,6 +46,7 @@ export class ModalDialog extends React.Component {
         // animations are disabled to avoid problems with automated tests
         return (
             <Modal
+                backdrop="static"
                 id={modalId}
                 animation={false}
                 show={show}

src/components/items_list/ItemsList.jsx

@@ -32,6 +32,12 @@ class ItemsList extends React.Component {
         };
     }
 
+    componentDidUpdate(prevProps) {
+        const { materials, index } = this.props;
+        if (prevProps.materials.length > materials.length)
+            this.setState({ editedName: materials[index].name, editedIndex: index });
+    }
+
     initControlsSwitchFromKeyboard(event) {
         const { materials, index, onItemClick } = this.props;
         if (!event.shiftKey) return; // Shift key must be down
@@ -66,9 +72,37 @@ class ItemsList extends React.Component {
         this.setState({ editedName: null, editedIndex: null });
     }
 
+    /**
+     * Used when clicking remove item
+     * e.preventDefault is used to inform further
+     * elements that event is already handled and they should skip
+     * handling it, otherwise the page can crash.
+     * @param {React.MouseEvent} e - JS DOM event
+     * @param {Number} index - index of element that should be removed
+     */
+    onDeleteIconClick(e, index) {
+        const { onRemove } = this.props;
+        e.preventDefault();
+        onRemove(index);
+    }
+
+    /**
+     * this function is used for handling clicks on different elements
+     * here is used check if the event is default prevented in order to
+     * avoid propagated actions that already was handled and don't handle
+     * extra actions that can lead to page crashes
+     * @param {React.MouseEvent} e - js dom event
+     * @param {Number} index - index of element that should be removed
+     */
+    onItemListClick(e, index) {
+        const { onItemClick } = this.props;
+        if (e.defaultPrevented) return;
+        e.preventDefault();
+        onItemClick(index);
+    }
+
     renderListItem(entity, index, indexFromState) {
         const { name, isUpdated, isNonPeriodic } = entity;
-        const { onItemClick, onRemove } = this.props;
         const { editedIndex, editedName } = this.state;
         const isBeingEdited = editedIndex === index;
         const isBeingActive = index === indexFromState;
@@ -77,7 +111,7 @@ class ItemsList extends React.Component {
                 key={name + "-" + index}
                 button
                 dense
-                onClick={() => onItemClick(index)}
+                onClick={(e) => this.onItemListClick(e, index)}
                 className={setClass(
                     { active: isBeingEdited || isBeingActive },
                     { updated: isUpdated || isBeingEdited },
@@ -114,7 +148,9 @@ class ItemsList extends React.Component {
 
                 <ListItemIcon
                     className="list-item-icon icon-button-delete"
-                    onClick={() => onRemove(index)}
+                    onClick={(e) => {
+                        this.onDeleteIconClick(e, index);
+                    }}
                 >
                     <DeleteIcon />
                 </ListItemIcon>

src/utils/array.js

@@ -1,6 +1,7 @@
 import _ from "underscore";
 
 export function safeMakeArrayIfNot(x) {
+    // TODO: use code.js and Array.isArray() for this
     if (!_.isArray(x)) return [x];
     return x;
 }

tests/cucumber/features/menu/edit/reset-clone-undo-redo.feature

@@ -37,3 +37,12 @@ Feature: User can create a material, then undo the changes and/or reset to the i
       | path          | index   |
       | si-clone.json | $INT{2} |
 
+    # Delete
+    When I delete materials with index "1"
+    Then material with following data exists in state
+      | path          | index   |
+      | si-clone.json | $INT{1} |
+     And material with following data does not exist in state
+      | path    | index   |
+      | si-clone.json | $INT{2} |
+

tests/cucumber/support/step_definitions/When I delete materials with index.js

@@ -0,0 +1,8 @@
+import {materialDesignerPage} from "../widgets/material_designer_page";
+
+export default function () {
+    this.When(/^I delete materials with index "([^"]*)"$/, function (index) {
+        if (!index) throw new Error("I click delete action from the Edit menu - index is undefined");
+        materialDesignerPage.designerWidget.clickDeleteAction(index);
+    });
+};

tests/cucumber/support/widgets/material_designer_widget.js

@@ -45,6 +45,10 @@ export class MaterialDesignerWidget extends Widget {
 
     cloneCurrentMaterial() {this.headerMenu.selectMenuItemByNameAndItemNumber("Edit", 4)};
 
+    clickDeleteAction(index) {
+        this.itemsList.deleteMaterialByIndex(index);
+    }
+
     clickUndoRedoReset(index = 1) {this.headerMenu.selectMenuItemByNameAndItemNumber("Edit", index)};
 
     openSurfaceDialog() {this.headerMenu.selectMenuItemByNameAndItemNumber("Advanced", 4)}