undo/redo cleanup (#1165)

* fix refactoring error which disabled annotation file clearing

* fix undo behavior on add category label

* fix various undo/redo bugs

* remove logging

* further refinement of annotation undo/redo and actions

* address PR comment

client/src/components/categorical/annoDialog.js

@@ -70,7 +70,7 @@ class AnnoDialog extends React.Component {
                   onClick={handleSecondaryButtonSubmit}
                   disabled={!text || validationError}
                   intent="none"
-                  type="submit"
+                  type="button"
                 >
                   {secondaryButtonText}
                 </Button>

client/src/components/categorical/annoDialogAddLabel.js

@@ -32,43 +32,43 @@ class Category extends React.Component {
     });
   };
 
-  handleAddNewLabelToCategory = () => {
+  handleAddNewLabelToCategory = e => {
     const { dispatch, metadataField } = this.props;
     const { newLabelText } = this.state;
 
+    this.disableAddNewLabelMode();
     dispatch({
       type: "annotation: add new label to category",
       metadataField,
       newLabelText,
       assignSelectedCells: false
     });
-    this.setState({ newLabelText: "" });
+    e.preventDefault();
   };
 
   addLabelAndAssignCells = () => {
     const { dispatch, metadataField } = this.props;
     const { newLabelText } = this.state;
 
+    this.disableAddNewLabelMode();
     dispatch({
       type: "annotation: add new label to category",
       metadataField,
       newLabelText,
       assignSelectedCells: true
     });
-
-    this.setState({ newLabelText: "" });
   };
 
   handleCreateArbitraryLabel = newLabelTextNotInOntology => {
     const { dispatch, metadataField } = this.props;
 
+    this.disableAddNewLabelMode();
     dispatch({
       type: "annotation: add new label to category",
       metadataField,
       newLabelText: newLabelTextNotInOntology,
       assignSelectedCells: false
     });
-    this.setState({ newLabelText: "" });
   };
 
   labelNameError = name => {
@@ -102,7 +102,9 @@ class Category extends React.Component {
             annotations.categoryAddingNewLabel === metadataField
           }
           inputProps={{ "data-testid": `${metadataField}:create-label-dialog` }}
-          primaryButtonProps={{ "data-testid": `${metadataField}:submit-label` }}
+          primaryButtonProps={{
+            "data-testid": `${metadataField}:submit-label`
+          }}
           title="Add new label to category"
           instruction="New, unique label name:"
           cancelTooltipContent="Close this dialog without adding a label."

client/src/components/categorical/annoDialogAddLabelFromOntology.js

@@ -32,43 +32,43 @@ class Category extends React.Component {
     });
   };
 
-  handleAddNewLabelToCategory = () => {
+  handleAddNewLabelToCategory = e => {
     const { dispatch, metadataField } = this.props;
     const { newLabelText } = this.state;
 
+    this.disableAddNewLabelFromOntologyMode();
     dispatch({
       type: "annotation: add new label to category",
       metadataField,
       newLabelText,
       assignSelectedCells: false
     });
-    this.setState({ newLabelText: "" });
+    e.preventDefault();
   };
 
   addLabelAndAssignCells = () => {
     const { dispatch, metadataField } = this.props;
     const { newLabelText } = this.state;
 
+    this.disableAddNewLabelFromOntologyMode();
     dispatch({
       type: "annotation: add new label to category",
       metadataField,
       newLabelText,
       assignSelectedCells: true
     });
-
-    this.setState({ newLabelText: "" });
   };
 
   handleCreateArbitraryLabel = newLabelTextNotInOntology => {
     const { dispatch, metadataField } = this.props;
 
+    this.disableAddNewLabelFromOntologyMode();
     dispatch({
       type: "annotation: add new label to category",
       metadataField,
       newLabelText: newLabelTextNotInOntology,
       assignSelectedCells: false
     });
-    this.setState({ newLabelText: "" });
   };
 
   labelNameError = name => {

client/src/components/categorical/annoDialogEditCategoryName.js

@@ -30,13 +30,14 @@ class AnnoDialogEditCategoryName extends React.Component {
   };
 
   disableEditCategoryMode = () => {
-    const { dispatch } = this.props;
+    const { dispatch, metadataField } = this.props;
     dispatch({
       type: "annotation: disable category edit mode"
     });
+    this.setState({ newCategoryText: metadataField });
   };
 
-  handleEditCategory = () => {
+  handleEditCategory = e => {
     const { dispatch, metadataField, categoricalSelection } = this.props;
     const { newCategoryText } = this.state;
 
@@ -50,12 +51,14 @@ class AnnoDialogEditCategoryName extends React.Component {
       return;
     }
 
+    this.disableEditCategoryMode();
     dispatch({
       type: "annotation: category edited",
       metadataField,
       newCategoryText,
       data: newCategoryText
     });
+    e.preventDefault();
   };
 
   categoryNameErrorMessage = () => {
@@ -121,8 +124,12 @@ class AnnoDialogEditCategoryName extends React.Component {
             annotations.isEditingCategoryName &&
             annotations.categoryBeingEdited === metadataField
           }
-          inputProps={{ "data-testid": `${metadataField}:edit-category-name-dialog` }}
+          inputProps={{
-          primaryButtonProps={{ "data-testid": `${metadataField}:submit-category-edit` }}
+            "data-testid": `${metadataField}:edit-category-name-dialog`
+          }}
+          primaryButtonProps={{
+            "data-testid": `${metadataField}:submit-category-edit`
+          }}
           title="Edit category name"
           instruction="New, unique category name:"
           cancelTooltipContent="Close this dialog without editing this category."
@@ -134,7 +141,9 @@ class AnnoDialogEditCategoryName extends React.Component {
           handleCancel={this.disableEditCategoryMode}
           annoInput={
             <AnnoInputs
-              inputProps={{ "data-testid": `${metadataField}:edit-category-name-text`}}
+              inputProps={{
+                "data-testid": `${metadataField}:edit-category-name-text`
+              }}
               useSuggest={false}
               text={newCategoryText}
               handleTextChange={this.handleCategoryEditTextChange}

client/src/components/categorical/categorical.js

@@ -24,7 +24,7 @@ class Categories extends React.Component {
     };
   }
 
-  handleCreateUserAnno = () => {
+  handleCreateUserAnno = e => {
     const { dispatch } = this.props;
     const { newCategoryText, categoryToDuplicate } = this.state;
     dispatch({
@@ -37,6 +37,7 @@ class Categories extends React.Component {
       categoryToDuplicate: null,
       newCategoryText: ""
     });
+    e.preventDefault();
   };
 
   handleEnableAnnoMode = () => {

client/src/components/categorical/value.js

@@ -83,10 +83,11 @@ class CategoryValue extends React.Component {
     });
   };
 
-  handleEditValue = () => {
+  handleEditValue = e => {
     const { dispatch, metadataField, categoryIndex } = this.props;
     const { editedLabelText } = this.state;
     const label = this.getLabel();
+    this.cancelEditMode();
     dispatch({
       type: "annotation: label edited",
       editedLabel: editedLabelText,
@@ -94,12 +95,13 @@ class CategoryValue extends React.Component {
       categoryIndex,
       label
     });
+    e.preventDefault();
   };
 
   handleCreateArbitraryLabel = editedLabelTextNotInOntology => {
     const { dispatch, metadataField, categoryIndex } = this.props;
     const label = this.getLabel();
-
+    this.cancelEditMode();
     dispatch({
       type: "annotation: label edited",
       metadataField,
@@ -150,7 +152,7 @@ class CategoryValue extends React.Component {
     });
   };
 
-  cancelEdit = () => {
+  cancelEditMode = () => {
     const { dispatch, metadataField, categoryIndex } = this.props;
     dispatch({
       type: "annotation: cancel edit label mode",
@@ -433,8 +435,12 @@ class CategoryValue extends React.Component {
               <div>
                 <AnnoDialog
                   isActive={editModeActive}
-                  inputProps={{ "data-testid": `${metadataField}:edit-label-name-dialog` }}
+                  inputProps={{
-                  primaryButtonProps={{ "data-testid": `${metadataField}:${displayString}:submit-label-edit` }}
+                    "data-testid": `${metadataField}:edit-label-name-dialog`
+                  }}
+                  primaryButtonProps={{
+                    "data-testid": `${metadataField}:${displayString}:submit-label-edit`
+                  }}
                   title="Edit label"
                   instruction={`New label text must be unique within category ${metadataField}:`}
                   cancelTooltipContent="Close this dialog without editing label text."
@@ -444,12 +450,14 @@ class CategoryValue extends React.Component {
                   validationError={this.labelNameError(editedLabelText)}
                   errorMessage={this.labelNameErrorMessage(editedLabelText)}
                   handleSubmit={this.handleEditValue}
-                  handleCancel={this.cancelEdit}
+                  handleCancel={this.cancelEditMode}
                   annoInput={
                     <AnnoInputs
                       useSuggest={ontologyEnabled}
                       text={editedLabelText}
-                      inputProps={{ "data-testid": `${metadataField}:${displayString}:edit-label-name` }}
+                      inputProps={{
+                        "data-testid": `${metadataField}:${displayString}:edit-label-name`
+                      }}
                       handleCreateArbitraryLabel={
                         this.handleCreateArbitraryLabel
                       }

client/src/reducers/annotations.js

@@ -80,15 +80,6 @@ const Annotations = (
         isAddingNewLabel: false,
         categoryAddingNewLabel: null
       };
-    /* this handles both cases, since at this point it's just a string */
-    case "annotation: add new label to category":
-      return {
-        ...state,
-        isAddingNewLabel: false,
-        categoryAddingNewLabel: null,
-        isAddingNewLabelFromOntology: false,
-        categoryAddingNewLabelFromOntology: null
-      };
     case "annotation: activate category edit mode":
       return {
         ...state,
@@ -101,12 +92,6 @@ const Annotations = (
         isEditingCategoryName: false,
         categoryBeingEdited: null
       };
-    case "annotation: category edited":
-      return {
-        ...state,
-        isEditingCategoryName: false,
-        categoryBeingEdited: null
-      };
 
     /* LABEL */
     case "annotation: activate edit label mode":
@@ -124,12 +109,6 @@ const Annotations = (
         isEditingLabelName: false,
         labelEditable: { category: null, label: null }
       };
-    case "annotation: label edited":
-      return {
-        ...state,
-        isEditingLabelName: false,
-        labelEditable: { category: null, label: null }
-      };
     default:
       return state;
   }

client/src/reducers/undoable.js

@@ -85,7 +85,6 @@ const Undoable = (reducer, undoableKeys, options = {}) => {
     );
     const newPast = [...past];
     const newState = newPast.pop();
-    const newStateFilterState = newState[filterStateKey];
     const newFuture = push(future, currentUndoableState);
     const nextState = {
       ...currentState,
@@ -94,7 +93,6 @@ const Undoable = (reducer, undoableKeys, options = {}) => {
       [futureKey]: newFuture,
       [pendingKey]: null
     };
-    nextState[filterStateKey] = newStateFilterState;
     return nextState;
   }
 
@@ -139,12 +137,13 @@ const Undoable = (reducer, undoableKeys, options = {}) => {
   */
   function skip(currentState, action, filterState) {
     const past = currentState[pastKey] || [];
+    const future = currentState[futureKey] || [];
     const pending = currentState[pendingKey];
     const res = reducer(currentState, action);
     return {
       ...res,
       [pastKey]: past,
-      [futureKey]: [],
+      [futureKey]: future,
       [filterStateKey]: filterState,
       [pendingKey]: pending
     };
@@ -218,6 +217,7 @@ const Undoable = (reducer, undoableKeys, options = {}) => {
     },
     action
   ) => {
+    if (debug > 1) console.log("---- ACTION", action.type);
     const aType = action.type;
     switch (aType) {
       case "@@undoable/undo": {

client/src/reducers/undoableConfig.js

@@ -35,18 +35,30 @@ const skipOnActions = new Set([
   "get single gene expression for coloring error",
 
   "category value mouse hover start",
-  "category value mouse hover end"
+  "category value mouse hover end",
+
+  /* autosave annotations */
+  "writable obs annotations - save complete",
+  "writable obs annotations - save started",
+  "writable obs annotations - save error",
+
+  /* annotation component action */
+  "annotation: activate add new label mode",
+  "annotation: activate add new ontology label mode",
+  "annotation: disable add new ontology label mode",
+  "annotation: disable add new label mode",
+  "annotation: activate category edit mode",
+  "annotation: disable category edit mode",
+  "annotation: activate edit label mode",
+  "annotation: cancel edit label mode",
+  "set annotations collection name"
 ]);
 
 /*
 identical, repeated occurances of these action types will be debounced.
 Entire action must be identical (all keys).
 */
-const debounceOnActions = new Set([
+const debounceOnActions = new Set([]);
-  "color by categorical metadata",
-  "color by continuous metadata",
-  "color by expression"
-]);
 
 /*
 history will be cleared when these actions occur
@@ -91,7 +103,8 @@ const saveOnActions = new Set([
   "annotation: delete category",
   "annotation: label edited",
   "annotation: label current cell selection",
-  "annotation: delete label"
+  "annotation: delete label",
+  "annotation: category edited"
 ]);
 
 /**
@@ -172,8 +185,6 @@ const actionFilter = debug => (state, action, prevFilterState) => {
   }
   if (
     debounceOnActions.has(actionType) &&
-    prevFilterState !== undefined &&
-    prevFilterState.prevAction !== undefined &&
     shallowObjectEq(action, prevFilterState.prevAction)
   ) {
     return { [actionKey]: "skip", [stateKey]: filterState };
@@ -229,7 +240,12 @@ function shallowArrayEq(arrA, arrB) {
 }
 
 /* configuration for the undoable meta reducer */
-const debug = false; // set truish for undoble debugging
+/*
+debug: set to any falsish value to disable logging of helpful debugging information.
+Set to true or 1 for base logging, high number for more verbosity (currently only 1/true
+or 2).
+*/
+const debug = false;
 const undoableConfig = {
   debug,
   historyLimit: 50, // maximum history size

server/common/rest.py

@@ -64,7 +64,7 @@ def annotations_put_fbs_helper(data_adaptor, annotations, fbs):
     new_label_df = decode_matrix_fbs(fbs)
     if not new_label_df.empty:
         data_adaptor.check_new_labels(new_label_df)
-        annotations.write_labels(new_label_df, data_adaptor)
+    annotations.write_labels(new_label_df, data_adaptor)
 
 
 def annotations_obs_put(request, data_adaptor, annotations):