From 7bc58bba2b294a68143fc495702fffff48e618c1 Mon Sep 17 00:00:00 2001 From: Bruce Martin Date: Mon, 24 Feb 2020 18:51:26 -0700 Subject: [PATCH] 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 --- .../src/components/categorical/annoDialog.js | 2 +- .../categorical/annoDialogAddLabel.js | 14 ++++---- .../annoDialogAddLabelFromOntology.js | 10 +++--- .../categorical/annoDialogEditCategoryName.js | 19 +++++++--- .../src/components/categorical/categorical.js | 3 +- client/src/components/categorical/value.js | 22 ++++++++---- client/src/reducers/annotations.js | 21 ----------- client/src/reducers/undoable.js | 6 ++-- client/src/reducers/undoableConfig.js | 36 +++++++++++++------ server/common/rest.py | 2 +- 10 files changed, 75 insertions(+), 60 deletions(-) diff --git a/client/src/components/categorical/annoDialog.js b/client/src/components/categorical/annoDialog.js index d51d3f76..202ff251 100644 --- a/client/src/components/categorical/annoDialog.js +++ b/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} diff --git a/client/src/components/categorical/annoDialogAddLabel.js b/client/src/components/categorical/annoDialogAddLabel.js index cdccb31d..16787680 100644 --- a/client/src/components/categorical/annoDialogAddLabel.js +++ b/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." diff --git a/client/src/components/categorical/annoDialogAddLabelFromOntology.js b/client/src/components/categorical/annoDialogAddLabelFromOntology.js index 1e4e359f..ac723ddc 100644 --- a/client/src/components/categorical/annoDialogAddLabelFromOntology.js +++ b/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 => { diff --git a/client/src/components/categorical/annoDialogEditCategoryName.js b/client/src/components/categorical/annoDialogEditCategoryName.js index b3e94302..f0f06ad8 100644 --- a/client/src/components/categorical/annoDialogEditCategoryName.js +++ b/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` }} - primaryButtonProps={{ "data-testid": `${metadataField}:submit-category-edit` }} + inputProps={{ + "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={ { + 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 = () => { diff --git a/client/src/components/categorical/value.js b/client/src/components/categorical/value.js index 153a5d45..538bd9c4 100644 --- a/client/src/components/categorical/value.js +++ b/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 {
{ ); 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": { diff --git a/client/src/reducers/undoableConfig.js b/client/src/reducers/undoableConfig.js index ce048fc4..9e9b7c88 100644 --- a/client/src/reducers/undoableConfig.js +++ b/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([ - "color by categorical metadata", - "color by continuous metadata", - "color by expression" -]); +const debounceOnActions = new Set([]); /* 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 diff --git a/server/common/rest.py b/server/common/rest.py index 6b192c5e..c00a63c4 100644 --- a/server/common/rest.py +++ b/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):