diff --git a/client/src/components/categorical/categorical.js b/client/src/components/categorical/categorical.js index 53ff7f3b..f2dc5b37 100644 --- a/client/src/components/categorical/categorical.js +++ b/client/src/components/categorical/categorical.js @@ -67,42 +67,42 @@ class Categories extends React.Component { return false if this is a LEGAL/acceptable category name or NULL/empty string, or return an error type. */ - if (!name) return false; + + /* allow empty string */ + if (name === "") return false; const { categoricalSelection } = this.props; const allCategoryNames = Object.keys(categoricalSelection); + /* check category name syntax */ + const error = AnnotationsHelpers.annotationNameIsErroneous(name); + if (error) { + return error; + } + + /* disallow duplicates */ if (allCategoryNames.indexOf(name) !== -1) { return "duplicate"; } - if (!AnnotationsHelpers.isLegalAnnotationName(name)) { - return "characters"; - } - + /* otherwise, no error */ return false; }; categoryNameErrorMessage = name => { const err = this.categoryNameError(name); if (err === false) return null; - if (err === "duplicate") { - return ( - - {name} already exists - - no duplicates allowed - - ); - } - if (err === "characters") { - return ( - - {name} contains illegal - characters. Hint: use alpha-numeric and underscore - - ); - } - return err; + + const errorMessageMap = { + /* map error code to human readable error message */ + "empty-string": "Blank names not allowed", + duplicate: "Name must be unique", + "trim-spaces": "Leading and trailing spaces not allowed", + "illegal-characters": "Only alphanumeric, underscore and period allowed", + "multi-space-run": "Multiple consecutive spaces not allowed" + }; + const errorMessage = errorMessageMap[err] ?? "error"; + return {errorMessage}; }; render() { diff --git a/client/src/components/categorical/category.js b/client/src/components/categorical/category.js index 3329c7c3..e8ecec45 100644 --- a/client/src/components/categorical/category.js +++ b/client/src/components/categorical/category.js @@ -158,25 +158,30 @@ class Category extends React.Component { return false if this is a LEGAL/acceptable category name or NULL/empty string, or return an error type. */ - let error = false; - if (name) { - const { metadataField, universe } = this.props; - const { obsByName } = universe.schema.annotations; - if (obsByName[metadataField].categories.indexOf(name) !== -1) { - error = "duplicate"; - } else if (!AnnotationsHelpers.isLegalAnnotationName(name)) { - error = "characters"; - } - } - return error; + /* allow empty string */ + if (name === "") return false; + + /* check for label syntax errors */ + const error = AnnotationsHelpers.annotationNameIsErroneous(name); + if (error) return error; + + /* disallow duplicates */ + const { metadataField, universe } = this.props; + const { obsByName } = universe.schema.annotations; + if (obsByName[metadataField].categories.indexOf(name) !== -1) + return "duplicate"; + + /* otherwise, no error */ + return false; }; labelNameErrorMessage = name => { const { metadataField } = this.props; const err = this.labelNameError(name); - if (err === false) return null; + if (err === "duplicate") { + /* duplicate error is special cased because it has special formatting */ return ( {name} already exists @@ -185,15 +190,23 @@ class Category extends React.Component { ); } - if (err === "characters") { - return ( - - {name} contains illegal - characters. Hint: use alpha-numeric and underscore - - ); + + if (err) { + /* all other errors - map code to human error message */ + const errorMessageMap = { + "empty-string": "Blank names not allowed", + duplicate: "Name must be unique", + "trim-spaces": "Leading and trailing spaces not allowed", + "illegal-characters": + "Only alphanumeric, underscore and period allowed", + "multi-space-run": "Multiple consecutive spaces not allowed" + }; + const errorMessage = errorMessageMap[err] ?? "error"; + return {errorMessage}; } - return err; + + /* no error, no message generated */ + return null; }; categoryNameErrorMessage = () => { @@ -201,76 +214,51 @@ class Category extends React.Component { const err = this.editedCategoryNameError(); if (err === false) return null; - let markup = null; - - if (err === "empty_string") { - markup = ( - - {"Category name cannot be blank"} - - ); - } else if (err === "already_exists") { - markup = ( - - {"Category name must be unique"} - - ); - } else if (err === "characters") { - markup = ( - - {"Only alphanumeric and underscore allowed"} - - ); - } - - return markup; + const errorMessageMap = { + /* map error code to human readable error message */ + "empty-string": "Blank names not allowed", + duplicate: "Category name must be unique", + "trim-spaces": "Leading and trailing spaces not allowed", + "illegal-characters": "Only alphanumeric, underscore and period allowed", + "multi-space-run": "Multiple consecutive spaces not allowed" + }; + const errorMessage = errorMessageMap[err] ?? "error"; + return ( + + {errorMessage} + + ); }; editedCategoryNameError = () => { const { metadataField, categoricalSelection } = this.props; const { newCategoryText } = this.state; - const allCategoryNames = _.keys(categoricalSelection); - const isEmptyString = newCategoryText === ""; + /* check for syntax errors in category name */ + const error = AnnotationsHelpers.annotationNameIsErroneous(newCategoryText); + if (error) { + return error; + } + + /* check for duplicative categories */ + const allCategoryNames = _.keys(categoricalSelection); 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"; + if (categoryNameAlreadyExists && !sameName) { + return "duplicate"; } - return error; + /* otherwise, no error */ + return false; }; toggleAll() { diff --git a/client/src/components/categorical/value.js b/client/src/components/categorical/value.js index b1e5f002..6a1ef38f 100644 --- a/client/src/components/categorical/value.js +++ b/client/src/components/categorical/value.js @@ -10,7 +10,8 @@ import { Popover, Position, PopoverInteractionKind, - Tooltip + Tooltip, + Colors } from "@blueprintjs/core"; import Occupancy from "./occupancy"; import * as globals from "../../globals"; @@ -85,75 +86,58 @@ class CategoryValue extends React.Component { valueNameErrorMessage = () => { const { editedLabelText } = this.state; const err = this.valueNameError(); - if (!err) return null; + if (err === false) return null; - let markup = null; - - if (err === "empty_string") { - markup = ( - - {"Label cannot be blank"} - - ); - } else if (err === "duplicate") { - markup = ( - - {"Label must be unique"} - - ); - } else if (err === "characters") { - markup = ( - - {"Only alphanumeric and underscore allowed"} - - ); - } - return markup; + const errorMessageMap = { + /* map error code to human readable error message */ + "empty-string": "Blank names not allowed", + duplicate: "Label must be unique", + "trim-spaces": "Leading and trailing spaces not allowed", + "illegal-characters": "Only alphanumeric, underscore and period allowed", + "multi-space-run": "Multiple consecutive spaces not allowed" + }; + const errorMessage = errorMessageMap[err] ?? "error"; + return ( + + {errorMessage} + + ); }; valueNameError = () => { const { editedLabelText } = this.state; const { categoricalSelection, metadataField, categoryIndex } = this.props; - let err = null; + /* + check label syntax + */ + const err = AnnotationsHelpers.annotationNameIsErroneous(editedLabelText); + if (err) return err; + /* + disallow duplicates + */ const category = categoricalSelection[metadataField]; const displayString = String( category.categoryValues[categoryIndex] ).valueOf(); - - if (editedLabelText === "") { - err = "empty_string"; - } else if ( + if ( category.categoryValues.indexOf(editedLabelText) > -1 && editedLabelText !== displayString - ) { - err = "duplicate"; - } else if (!AnnotationsHelpers.isLegalAnnotationName(editedLabelText)) { - err = "characters"; - } - return err; + ) + return "duplicate"; + + /* + otherwise, all good! + */ + return false; }; activateEditLabelMode = () => { @@ -221,6 +205,21 @@ class CategoryValue extends React.Component { ); }; + componentDidUpdate(prevProps) { + const { categoricalSelection, metadataField, categoryIndex } = this.props; + if ( + prevProps.categoricalSelection !== categoricalSelection || + prevProps.metadataField !== metadataField || + prevProps.categoryIndex !== categoryIndex + ) { + this.setState({ + editedLabelText: String( + categoricalSelection[metadataField].categoryValues[categoryIndex] + ).valueOf() + }); + } + } + toggleOn = () => { const { dispatch, metadataField, categoryIndex } = this.props; dispatch({ @@ -546,6 +545,7 @@ class CategoryValue extends React.Component { data-testclass="handleEditValue" data-testid={`handleEditValue-${metadataField}`} onClick={this.activateEditLabelMode} + disabled={annotations.isEditingLabelName} /> ) : null} {displayString !== globals.unassignedCategoryLabel ? ( diff --git a/client/src/util/stateManager/annotationsHelpers.js b/client/src/util/stateManager/annotationsHelpers.js index 4fc0954e..e55e2d3a 100644 --- a/client/src/util/stateManager/annotationsHelpers.js +++ b/client/src/util/stateManager/annotationsHelpers.js @@ -183,7 +183,35 @@ export function createWritableAnnotationDimensions(world, crossfilter) { return crossfilter; } -const legalNames = /^\w+$/; -export function isLegalAnnotationName(name) { - return legalNames.test(name); +const legalCharacters = /^(\w|[ .])+$/; +export function annotationNameIsErroneous(name) { + /* + Validate the name - return: + * false - a valid name + * string - a named error, indicating why it was invalid. + + Tests: + 0. must be string, non-null + 1. no leading or trailing spaces + 2. only accept alpha, numeric, underscore, period and space + 3. no runs of multiple spaces + */ + + if (name === "") { + return "empty-string"; + } + if (name[0] === " " || name[name.length - 1] === " ") { + return "trim-spaces"; + } + if (!legalCharacters.test(name)) { + return "illegal-characters"; + } + for (let i = 1, l = name.length; i < l; i += 1) { + if (name[i] === " " && name[i - 1] === " ") { + return "multi-space-run"; + } + } + + /* all is well! Indicte not erroneous with a false */ + return false; }