From c2ededfc62eab66f214df331051cfe543c0053db Mon Sep 17 00:00:00 2001
From: Colin Megill <colinmegill@gmail.com>
Date: Thu, 21 Nov 2019 19:08:29 -0500
Subject: [PATCH] Anno editing label/category error states (#1048)

* anno error states

* label validation should not treat empty string as error

* disable Submit if no label or category entered

* edit category name error states

* flex start

* pr cleanup

* label name validation fixes

* PR comment responses
---
 .../src/components/categorical/categorical.js |  19 ++-
 client/src/components/categorical/category.js | 141 ++++++++++++++----
 client/src/components/categorical/value.js    | 134 +++++++++++++----
 3 files changed, 227 insertions(+), 67 deletions(-)

diff --git a/client/src/components/categorical/categorical.js b/client/src/components/categorical/categorical.js
index 37574027..53ff7f3b 100644
--- a/client/src/components/categorical/categorical.js
+++ b/client/src/components/categorical/categorical.js
@@ -64,9 +64,11 @@ class Categories extends React.Component {
 
   categoryNameError = name => {
     /*
-    return false if this is a LEGAL/acceptable category name,
+    return false if this is a LEGAL/acceptable category name or NULL/empty string,
     or return an error type.
     */
+    if (!name) return false;
+
     const { categoricalSelection } = this.props;
     const allCategoryNames = Object.keys(categoricalSelection);
 
@@ -83,6 +85,7 @@ class Categories extends React.Component {
 
   categoryNameErrorMessage = name => {
     const err = this.categoryNameError(name);
+    if (err === false) return null;
     if (err === "duplicate") {
       return (
         <span>
@@ -167,7 +170,6 @@ class Categories extends React.Component {
                       autoFocus
                       value={newCategoryText}
                       intent={
-                        newCategoryText &&
                         this.categoryNameError(newCategoryText)
                           ? "warning"
                           : "none"
@@ -180,11 +182,9 @@ class Categories extends React.Component {
                     <p
                       style={{
                         marginTop: 7,
-                        visibility:
-                          newCategoryText &&
-                          this.categoryNameError(newCategoryText)
-                            ? "visible"
-                            : "hidden",
+                        visibility: this.categoryNameError(newCategoryText)
+                          ? "visible"
+                          : "hidden",
                         color: Colors.ORANGE3
                       }}
                     >
@@ -227,7 +227,10 @@ class Categories extends React.Component {
                     </Tooltip>
                     <Button
                       onClick={this.handleCreateUserAnno}
-                      disabled={this.categoryNameError(newCategoryText)}
+                      disabled={
+                        !newCategoryText ||
+                        this.categoryNameError(newCategoryText)
+                      }
                       intent="primary"
                       type="submit"
                     >
diff --git a/client/src/components/categorical/category.js b/client/src/components/categorical/category.js
index a966025d..3329c7c3 100644
--- a/client/src/components/categorical/category.js
+++ b/client/src/components/categorical/category.js
@@ -35,7 +35,7 @@ class Category extends React.Component {
     this.state = {
       isChecked: true,
       isExpanded: false,
-      newCategoryText: "",
+      newCategoryText: props.metadataField,
       newLabelText: ""
     };
   }
@@ -116,9 +116,19 @@ class Category extends React.Component {
   };
 
   handleEditCategory = () => {
-    const { dispatch, metadataField } = this.props;
+    const { dispatch, metadataField, categoricalSelection } = this.props;
     const { newCategoryText } = this.state;
 
+    const allCategoryNames = _.keys(categoricalSelection);
+
+    if (
+      (allCategoryNames.indexOf(newCategoryText) > -1 &&
+        newCategoryText !== metadataField) ||
+      newCategoryText === ""
+    ) {
+      return;
+    }
+
     dispatch({
       type: "annotation: category edited",
       metadataField,
@@ -145,26 +155,27 @@ class Category extends React.Component {
 
   labelNameError = name => {
     /*
-    return false if this is a LEGAL/acceptable category name,
+    return false if this is a LEGAL/acceptable category name or NULL/empty string,
     or return an error type.
     */
-    const { metadataField, universe } = this.props;
-    const { obsByName } = universe.schema.annotations;
+    let error = false;
+    if (name) {
+      const { metadataField, universe } = this.props;
+      const { obsByName } = universe.schema.annotations;
 
-    if (obsByName[metadataField].categories.indexOf(name) !== -1) {
-      return "duplicate";
+      if (obsByName[metadataField].categories.indexOf(name) !== -1) {
+        error = "duplicate";
+      } else if (!AnnotationsHelpers.isLegalAnnotationName(name)) {
+        error = "characters";
+      }
     }
-
-    if (!AnnotationsHelpers.isLegalAnnotationName(name)) {
-      return "characters";
-    }
-
-    return false;
+    return error;
   };
 
   labelNameErrorMessage = name => {
     const { metadataField } = this.props;
     const err = this.labelNameError(name);
+    if (err === false) return null;
     if (err === "duplicate") {
       return (
         <span>
@@ -185,6 +196,83 @@ class Category extends React.Component {
     return err;
   };
 
+  categoryNameErrorMessage = () => {
+    const { newCategoryText } = this.state;
+    const err = this.editedCategoryNameError();
+    if (err === false) return null;
+
+    let markup = null;
+
+    if (err === "empty_string") {
+      markup = (
+        <span
+          style={{
+            display: "block",
+            fontStyle: "italic",
+            fontSize: 12,
+            marginTop: 5,
+            color: Colors.ORANGE3
+          }}
+        >
+          {"Category name cannot be blank"}
+        </span>
+      );
+    } else if (err === "already_exists") {
+      markup = (
+        <span
+          style={{
+            display: "block",
+            fontStyle: "italic",
+            fontSize: 12,
+            marginTop: 5,
+            color: Colors.ORANGE3
+          }}
+        >
+          {"Category name must be unique"}
+        </span>
+      );
+    } else if (err === "characters") {
+      markup = (
+        <span
+          style={{
+            display: "block",
+            fontStyle: "italic",
+            fontSize: 12,
+            marginTop: 5,
+            color: Colors.ORANGE3
+          }}
+        >
+          {"Only alphanumeric and underscore allowed"}
+        </span>
+      );
+    }
+
+    return markup;
+  };
+
+  editedCategoryNameError = () => {
+    const { metadataField, categoricalSelection } = this.props;
+    const { newCategoryText } = this.state;
+    const allCategoryNames = _.keys(categoricalSelection);
+
+    const isEmptyString = newCategoryText === "";
+    const categoryNameAlreadyExists =
+      allCategoryNames.indexOf(newCategoryText) > -1;
+    const sameName = newCategoryText === metadataField;
+
+    let error = false;
+
+    if (isEmptyString) {
+      error = "empty_string";
+    } else if (categoryNameAlreadyExists && !sameName) {
+      error = "already_exists";
+    } else if (!AnnotationsHelpers.isLegalAnnotationName(newCategoryText)) {
+      error = "characters";
+    }
+
+    return error;
+  };
+
   toggleAll() {
     const { dispatch, metadataField } = this.props;
     dispatch({
@@ -251,6 +339,7 @@ class Category extends React.Component {
       ...cat.categoryValueIndices
     ]);
     const optTuplesAsKey = _.map(optTuples, t => t[0]).join(""); // animation
+    const allCategoryNames = _.keys(categoricalSelection);
 
     return (
       <div
@@ -271,7 +360,7 @@ class Category extends React.Component {
             style={{
               display: "flex",
               justifyContent: "flex-start",
-              alignItems: "baseline"
+              alignItems: "flex-start"
             }}
           >
             <label className="bp3-control bp3-checkbox">
@@ -332,7 +421,7 @@ class Category extends React.Component {
                     rightElement={
                       <Button
                         minimal
-                        disabled={newCategoryText.length === 0}
+                        disabled={this.editedCategoryNameError()}
                         style={{ position: "relative", top: -1 }}
                         type="button"
                         icon="small-tick"
@@ -342,6 +431,7 @@ class Category extends React.Component {
                       />
                     }
                   />
+                  {this.categoryNameErrorMessage()}
                 </form>
               ) : (
                 metadataField
@@ -385,10 +475,7 @@ class Category extends React.Component {
                           autoFocus
                           value={newLabelText}
                           intent={
-                            // universe.schema.annotations.obsByName[
-                            //   metadataField
-                            // ].categories.indexOf(newLabelText) !== -1
-                            newLabelText && this.labelNameError(newLabelText)
+                            this.labelNameError(newLabelText)
                               ? "warning"
                               : "none"
                           }
@@ -400,13 +487,9 @@ class Category extends React.Component {
                         <p
                           style={{
                             marginTop: 7,
-                            visibility:
-                              // universe.schema.annotations.obsByName[
-                              //   metadataField
-                              // ].categories.indexOf(newLabelText) !== -1
-                              newLabelText && this.labelNameError(newLabelText)
-                                ? "visible"
-                                : "hidden",
+                            visibility: this.labelNameError(newLabelText)
+                              ? "visible"
+                              : "hidden",
                             color: Colors.ORANGE3
                           }}
                         >
@@ -423,11 +506,7 @@ class Category extends React.Component {
                         </Tooltip>
                         <Button
                           disabled={
-                            newLabelText.length === 0 ||
-                            // universe.schema.annotations.obsByName[
-                            //   metadataField
-                            // ].categories.indexOf(newLabelText) !== -1
-                            this.labelNameError(newLabelText)
+                            !newLabelText || this.labelNameError(newLabelText)
                           }
                           onClick={this.handleAddNewLabelToCategory}
                           intent="primary"
diff --git a/client/src/components/categorical/value.js b/client/src/components/categorical/value.js
index 090550df..16a74480 100644
--- a/client/src/components/categorical/value.js
+++ b/client/src/components/categorical/value.js
@@ -31,19 +31,26 @@ class CategoryValue extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
-      editedLabelText: ""
+      editedLabelText: String(
+        props.categoricalSelection[props.metadataField].categoryValues[
+          props.categoryIndex
+        ]
+      ).valueOf()
     };
   }
 
-  handleDeleteValue = () => {
-    const {
-      dispatch,
-      metadataField,
-      categoryIndex,
-      categoricalSelection
-    } = this.props;
+  getLabel = () => {
+    const { metadataField, categoryIndex, categoricalSelection } = this.props;
     const category = categoricalSelection[metadataField];
     const label = category.categoryValues[categoryIndex];
+
+    return label;
+  };
+
+  handleDeleteValue = () => {
+    const { dispatch, metadataField } = this.props;
+    const label = this.getLabel();
+
     dispatch({
       type: "annotation: delete label",
       metadataField,
@@ -52,14 +59,8 @@ class CategoryValue extends React.Component {
   };
 
   handleAddCurrentSelectionToThisLabel = () => {
-    const {
-      dispatch,
-      metadataField,
-      categoryIndex,
-      categoricalSelection
-    } = this.props;
-    const category = categoricalSelection[metadataField];
-    const label = category.categoryValues[categoryIndex];
+    const { dispatch, metadataField, categoryIndex } = this.props;
+    const label = this.getLabel();
     dispatch({
       type: "annotation: label current cell selection",
       metadataField,
@@ -69,15 +70,9 @@ class CategoryValue extends React.Component {
   };
 
   handleEditValue = () => {
-    const {
-      dispatch,
-      metadataField,
-      categoryIndex,
-      categoricalSelection
-    } = this.props;
+    const { dispatch, metadataField, categoryIndex } = this.props;
     const { editedLabelText } = this.state;
-    const category = categoricalSelection[metadataField];
-    const label = category.categoryValues[categoryIndex];
+    const label = this.getLabel();
     dispatch({
       type: "annotation: label edited",
       editedLabel: editedLabelText,
@@ -85,7 +80,80 @@ class CategoryValue extends React.Component {
       categoryIndex,
       label
     });
-    this.setState({ editedLabelText: "" });
+  };
+
+  valueNameErrorMessage = () => {
+    const { editedLabelText } = this.state;
+    const err = this.valueNameError();
+    if (!err) return null;
+
+    let markup = null;
+
+    if (err === "empty_string") {
+      markup = (
+        <span
+          style={{
+            fontStyle: "italic",
+            fontSize: 12,
+            marginTop: 5,
+            color: Colors.ORANGE3
+          }}
+        >
+          {"Label cannot be blank"}
+        </span>
+      );
+    } else if (err === "duplicate") {
+      markup = (
+        <span
+          style={{
+            fontStyle: "italic",
+            fontSize: 12,
+            marginTop: 5,
+            color: Colors.ORANGE3
+          }}
+        >
+          {"Label must be unique"}
+        </span>
+      );
+    } else if (err === "characters") {
+      markup = (
+        <span
+          style={{
+            fontStyle: "italic",
+            fontSize: 12,
+            marginTop: 5,
+            color: Colors.ORANGE3
+          }}
+        >
+          {"Only alphanumeric and underscore allowed"}
+        </span>
+      );
+    }
+    return markup;
+  };
+
+  valueNameError = () => {
+    const { editedLabelText } = this.state;
+    const { categoricalSelection, metadataField, categoryIndex } = this.props;
+
+    let err = null;
+
+    const category = categoricalSelection[metadataField];
+    const displayString = String(
+      category.categoryValues[categoryIndex]
+    ).valueOf();
+
+    if (editedLabelText === "") {
+      err = "empty_string";
+    } else if (
+      category.categoryValues.indexOf(editedLabelText) > -1 &&
+      editedLabelText !== displayString
+    ) {
+      err = "duplicate";
+    } else if (!AnnotationsHelpers.isLegalAnnotationName(editedLabelText)) {
+      err = "characters";
+    }
+    return err;
   };
 
   activateEditLabelMode = () => {
@@ -115,7 +183,7 @@ class CategoryValue extends React.Component {
     });
   };
 
-  shouldComponentUpdate = nextProps => {
+  shouldComponentUpdate = (nextProps, nextState) => {
     /*
     Checks to see if at least one of the following changed:
     * world state
@@ -125,7 +193,7 @@ class CategoryValue extends React.Component {
 
     If and only if true, update the component
     */
-    const { props } = this;
+    const { props, state } = this;
     const { metadataField, categoryIndex, categoricalSelection } = props;
     const { categoricalSelection: newCategoricalSelection } = nextProps;
 
@@ -141,13 +209,15 @@ class CategoryValue extends React.Component {
     const colorAccessorChange = props.colorAccessor !== nextProps.colorAccessor;
     const annotationsChange = props.annotations !== nextProps.annotations;
     const crossfilterChange = props.crossfilter !== nextProps.crossfilter;
+    const editingLabel = state.editedLabelText !== nextState.editedLabelText;
 
     return (
       valueSelectionChange ||
       worldChange ||
       colorAccessorChange ||
       annotationsChange ||
-      crossfilterChange
+      crossfilterChange ||
+      editingLabel
     );
   };
 
@@ -222,6 +292,8 @@ class CategoryValue extends React.Component {
       flippedProps
     } = this.props;
 
+    const { editedLabelText } = this.state;
+
     if (!categoricalSelection) return null;
 
     const category = categoricalSelection[metadataField];
@@ -347,6 +419,9 @@ class CategoryValue extends React.Component {
               <form
                 onSubmit={e => {
                   e.preventDefault();
+                  if (this.valueNameError()) {
+                    return;
+                  }
                   this.handleEditValue();
                 }}
               >
@@ -357,6 +432,7 @@ class CategoryValue extends React.Component {
                   }}
                   small
                   autoFocus
+                  intent={this.valueNameError() ? "warning" : "none"}
                   onChange={e => {
                     this.setState({ editedLabelText: e.target.value });
                   }}
@@ -365,6 +441,7 @@ class CategoryValue extends React.Component {
                     <Button
                       minimal
                       style={{ position: "relative", top: -1 }}
+                      disabled={this.valueNameError()}
                       type="button"
                       icon="small-tick"
                       data-testclass="submitEdit"
@@ -373,6 +450,7 @@ class CategoryValue extends React.Component {
                     />
                   }
                 />
+                {this.valueNameErrorMessage()}
               </form>
             ) : null}
             {/*