fix: Rename/delete tags throughout assets (#764)

Renames or deletes tags from asset/project files when tags are renamed/deleted in tag input component Solves [AB#17862]
microsoft · Apr 19, 2019 · 54ecbe1 · 54ecbe1
2 parents 1e0a460 + 4d99c1c
commit 54ecbe1
Show file tree

Hide file tree

Showing 15 changed files with 572 additions and 79 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/src/common/localization/en-us.ts b/src/common/localization/en-us.ts
@@ -256,6 +256,15 @@ export const english: IAppStrings = {
                 apply: "Apply Tag with Hot Key",
                 lock: "Lock Tag with Hot Key",
             },
+            rename: {
+                title: "Rename Tag",
+                confirmation: "Are you sure you want to rename this tag? It will be renamed throughout all assets",
+            },
+            delete: {
+                title: "Delete Tag",
+                confirmation: "Are you sure you want to delete this tag? It will be deleted throughout all assets \
+                and any regions where this is the only tag will also be deleted",
+            },
         },
         canvas: {
             removeAllRegions: {
@@ -267,7 +276,8 @@ export const english: IAppStrings = {
             enforceTaggedRegions: {
                 title: "Invalid region(s) detected",
                 // tslint:disable-next-line:max-line-length
-                description: "1 or more regions have not been tagged.  Ensure all regions are tagged before continuing to next asset.",
+                description: "1 or more regions have not been tagged.  Ensure all regions are tagged before \
+                continuing to next asset.",
             },
         },
     },

diff --git a/src/common/localization/es-cl.ts b/src/common/localization/es-cl.ts
@@ -258,6 +258,16 @@ export const spanish: IAppStrings = {
                 apply: "Aplicar etiqueta con tecla de acceso rápido",
                 lock: "Bloquear etiqueta con tecla de acceso rápido",
             },
+            rename: {
+                title: "Cambiar el nombre de la etiqueta",
+                confirmation: "¿Está seguro que quiere cambiar el nombre de esta etiqueta? \
+                Será cambiada en todos los activos",
+            },
+            delete: {
+                title: "Delete Tag",
+                confirmation: "¿Está seguro que quiere borrar esta etiqueta? Será borrada en todos \
+                los activos y en las regiones donde esta etiqueta sea la única, la region también será borrada",
+            },
         },
         canvas: {
             removeAllRegions: {

diff --git a/src/common/mockFactory.ts b/src/common/mockFactory.ts
@@ -810,14 +810,16 @@ export default class MockFactory {
      */
     public static projectActions(): IProjectActions {
         return {
-            loadProject: jest.fn((project: IProject) => Promise.resolve()),
-            saveProject: jest.fn((project: IProject) => Promise.resolve()),
-            deleteProject: jest.fn((project: IProject) => Promise.resolve()),
+            loadProject: jest.fn(() => Promise.resolve()),
+            saveProject: jest.fn(() => Promise.resolve()),
+            deleteProject: jest.fn(() => Promise.resolve()),
             closeProject: jest.fn(() => Promise.resolve()),
-            loadAssets: jest.fn((project: IProject) => Promise.resolve()),
-            exportProject: jest.fn((project: IProject) => Promise.resolve()),
-            loadAssetMetadata: jest.fn((project: IProject, asset: IAsset) => Promise.resolve()),
-            saveAssetMetadata: jest.fn((project: IProject, assetMetadata: IAssetMetadata) => Promise.resolve()),
+            loadAssets: jest.fn(() => Promise.resolve()),
+            exportProject: jest.fn(() => Promise.resolve()),
+            loadAssetMetadata: jest.fn(() => Promise.resolve()),
+            saveAssetMetadata: jest.fn(() => Promise.resolve()),
+            updateProjectTag: jest.fn(() => Promise.resolve()),
+            deleteProjectTag: jest.fn(() => Promise.resolve()),
         };
     }
 

diff --git a/src/common/strings.ts b/src/common/strings.ts
@@ -255,6 +255,14 @@ export interface IAppStrings {
                 apply: string;
                 lock: string;
             },
+            rename: {
+                title: string;
+                confirmation: string;
+            },
+            delete: {
+                title: string;
+                confirmation: string;
+            },
         }
         canvas: {
             removeAllRegions: {

diff --git a/src/react/components/common/tagInput/tagInput.tsx b/src/react/components/common/tagInput/tagInput.tsx
@@ -1,4 +1,4 @@
-import React, { KeyboardEvent } from "react";
+import React, { KeyboardEvent, RefObject } from "react";
 import ReactDOM from "react-dom";
 import Align from "rc-align";
 import { randomIntInRange } from "../../../../common/utils";
@@ -31,9 +31,9 @@ export interface ITagInputProps {
     /** Function to call on clicking individual tag while holding CTRL key */
     onCtrlTagClick?: (tag: ITag) => void;
     /** Function to call when tag is renamed */
-    onTagRenamed?: (oldTag: string, newTag: string) => void;
+    onTagRenamed?: (tagName: string, newTagName: string) => void;
     /** Function to call when tag is deleted */
-    onTagDeleted?: (tag: ITag) => void;
+    onTagDeleted?: (tagName: string) => void;
     /** Always show tag input box */
     showTagInputBox?: boolean;
     /** Always show tag search box */
@@ -72,7 +72,7 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
         portalElement: defaultDOMNode(),
     };
 
-    private tagItemRefs: { [id: string]: TagInputItem } = {};
+    private tagItemRefs: Map<string, RefObject<TagInputItem>> = new Map<string, RefObject<TagInputItem>>();
     private portalDiv = document.createElement("div");
 
     public render() {
@@ -109,7 +109,7 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
                     }
                     {this.getColorPickerPortal()}
                     <div className="tag-input-items">
-                        {this.getTagItems()}
+                        {this.renderTagItems()}
                     </div>
                     {
                         this.state.addTags &&
@@ -154,11 +154,13 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
         }
     }
 
-    private getTagNode = (tag: ITag) => {
+    private getTagNode = (tag: ITag): Element => {
         if (!tag) {
             return defaultDOMNode();
         }
-        return ReactDOM.findDOMNode(this.tagItemRefs[tag.name]) as Element;
+
+        const itemRef = this.tagItemRefs.get(tag.name);
+        return (itemRef ? ReactDOM.findDOMNode(itemRef.current) : defaultDOMNode()) as Element;
     }
 
     private onEditTag = (tag: ITag) => {
@@ -219,20 +221,25 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
         }, () => this.props.onChange(tags));
     }
 
-    private updateTag = (oldTag: ITag, newTag: ITag) => {
-        if (oldTag === newTag) {
+    private updateTag = (tag: ITag, newTag: ITag) => {
+        if (tag === newTag) {
             return;
         }
         if (!newTag.name.length) {
             toast.warn(strings.tags.warnings.emptyName);
             return;
         }
-        if (newTag.name !== oldTag.name && this.state.tags.some((t) => t.name === newTag.name)) {
+        const nameChange = tag.name !== newTag.name;
+        if (nameChange && this.state.tags.some((t) => t.name === newTag.name)) {
             toast.warn(strings.tags.warnings.existingName);
             return;
         }
+        if (nameChange && this.props.onTagRenamed) {
+            this.props.onTagRenamed(tag.name, newTag.name);
+            return;
+        }
         const tags = this.state.tags.map((t) => {
-            return (t.name === oldTag.name) ? newTag : t;
+            return (t.name === tag.name) ? newTag : t;
         });
         this.setState({
             tags,
@@ -293,12 +300,15 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
         return this.state.editingTagNode || document;
     }
 
-    private getTagItems = () => {
-        let props = this.getTagItemProps();
+    private renderTagItems = () => {
+        let props = this.createTagItemProps();
         const query = this.state.searchQuery;
+        this.tagItemRefs.clear();
+
         if (query.length) {
             props = props.filter((prop) => prop.tag.name.toLowerCase().includes(query.toLowerCase()));
         }
+
         return props.map((prop) =>
             <TagInputItem
                 key={prop.tag.name}
@@ -308,16 +318,16 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
     }
 
     private setTagItemRef = (item, tag) => {
-        if (item) {
-            this.tagItemRefs[tag.name] = item;
-        }
+        this.tagItemRefs.set(tag.name, item);
+        return item;
     }
 
-    private getTagItemProps = (): ITagInputItemProps[] => {
+    private createTagItemProps = (): ITagInputItemProps[] => {
         const tags = this.state.tags;
         const selectedRegionTagSet = this.getSelectedRegionTagSet();
-        return tags.map((tag) => {
-            const item: ITagInputItemProps = {
+
+        return tags.map((tag) => (
+            {
                 tag,
                 index: tags.findIndex((t) => t.name === tag.name),
                 isLocked: this.props.lockedTags && this.props.lockedTags.findIndex((t) => t === tag.name) > -1,
@@ -326,9 +336,8 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
                 appliedToSelectedRegions: selectedRegionTagSet.has(tag.name),
                 onClick: this.handleClick,
                 onChange: this.updateTag,
-            };
-            return item;
-        });
+            } as ITagInputItemProps
+        ));
     }
 
     private getSelectedRegionTagSet = (): Set<string> => {
@@ -346,6 +355,7 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
     private onAltClick = (tag: ITag, clickedColor: boolean) => {
         const { editingTag } = this.state;
         const newEditingTag = editingTag && editingTag.name === tag.name ? null : tag;
+
         this.setState({
             editingTag: newEditingTag,
             editingTagNode: this.getTagNode(newEditingTag),
@@ -355,16 +365,16 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
     }
 
     private handleClick = (tag: ITag, props: ITagClickProps) => {
+        // Lock tags
         if (props.ctrlKey && this.props.onCtrlTagClick) {
             this.props.onCtrlTagClick(tag);
             this.setState({ clickedColor: props.clickedColor });
-        } else if (props.altKey) {
+        } else if (props.altKey) { // Edit tag
             this.onAltClick(tag, props.clickedColor);
-        } else {
+        } else { // Select tag
             const { editingTag, selectedTag } = this.state;
             const inEditMode = editingTag && tag.name === editingTag.name;
             const alreadySelected = selectedTag && selectedTag.name === tag.name;
-
             const newEditingTag = inEditMode ? null : editingTag;
 
             this.setState({
@@ -389,12 +399,19 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
         if (!tag) {
             return;
         }
+        if (this.props.onTagDeleted) {
+            this.props.onTagDeleted(tag.name);
+            return;
+        }
+
         const index = this.state.tags.indexOf(tag);
         const tags = this.state.tags.filter((t) => t.name !== tag.name);
+
         this.setState({
             tags,
             selectedTag: this.getNewSelectedTag(tags, index),
         }, () => this.props.onChange(tags));
+
         if (this.props.lockedTags.find((l) => l === tag.name)) {
             this.props.onLockedTagsChange(
                 this.props.lockedTags.filter((lockedTag) => lockedTag !== tag.name),

diff --git a/src/react/components/pages/editorPage/canvas.test.tsx b/src/react/components/pages/editorPage/canvas.test.tsx
@@ -179,7 +179,7 @@ describe("Editor Canvas", () => {
         });
         const canvas = wrapper.instance() as Canvas;
         expect(wrapper.state().currentAsset).toEqual(assetMetadata);
-        expect(() => canvas.updateCanvasToolsRegions()).not.toThrowError();
+        expect(() => canvas.updateCanvasToolsRegionTags()).not.toThrowError();
     });
 
     it("canvas content source is updated when asset is deactivated", () => {