Fixed Symbols with text components (#5983)

* Add setName to Component

* Clear properly symbol refs when removed from the non symbol parent

* Improve isInstanceOf

* Fix symbols for text component updates

* Prevent component selection switch during text editing and drag #5517

* Add the possibility to add blocks via `Components.addType`

* Up TS

src/commands/view/SelectComponent.ts

@@ -292,23 +292,30 @@ export default {
     if (em.get('_cmpDrag')) return em.set('_cmpDrag');
 
     const el = ev.target as HTMLElement;
-    let model = getComponentModel(el);
+    let cmp = getComponentModel(el);
 
-    if (!model) {
+    if (!cmp) {
       let parentEl = el.parentNode;
 
-      while (!model && parentEl && !isDoc(parentEl)) {
-        model = getComponentModel(parentEl);
+      while (!cmp && parentEl && !isDoc(parentEl)) {
+        cmp = getComponentModel(parentEl);
         parentEl = parentEl.parentNode;
       }
     }
 
-    if (model) {
-      // Avoid selection of inner text components during editing
-      if (em.isEditing() && !model.get('textable') && model.isChildOf('text')) {
+    if (cmp) {
+      if (
+        em.isEditing() &&
+        // Avoid selection of inner text components during editing
+        ((!cmp.get('textable') && cmp.isChildOf('text')) ||
+          // Prevents selecting another component if the pointer was pressed and
+          // dragged outside of the editing component
+          em.getEditing() !== cmp)
+      ) {
         return;
       }
-      this.select(model, ev);
+
+      this.select(cmp, ev);
     }
   },
 

src/dom_components/index.ts

@@ -53,7 +53,7 @@
  *
  * @module Components
  */
-import { debounce, isArray, isEmpty, isFunction, isString, isSymbol, result } from 'underscore';
+import { debounce, isArray, isBoolean, isEmpty, isFunction, isString, isSymbol, result } from 'underscore';
 import { ItemManagerModule } from '../abstract/Module';
 import { AddOptions, ObjectAny } from '../common';
 import EditorModel from '../editor/model/Editor';
@@ -113,6 +113,7 @@ import {
 } from './model/SymbolUtils';
 import { ComponentsEvents, SymbolInfo } from './types';
 import Symbols from './model/Symbols';
+import { BlockProperties } from '../block_manager/model/Block';
 
 export type ComponentEvent =
   | 'component:create'
@@ -146,6 +147,7 @@ export interface AddComponentTypeOptions {
   isComponent?: (el: HTMLElement) => boolean | ComponentDefinitionDefined | undefined;
   model?: Partial<ComponentModelDefinition> & ThisType<ComponentModelDefinition & Component>;
   view?: Partial<ComponentViewDefinition> & ThisType<ComponentViewDefinition & ComponentView>;
+  block?: boolean | Partial<BlockProperties>;
   extend?: string;
   extendView?: string;
   extendFn?: string[];
@@ -517,12 +519,12 @@ export default class ComponentManager extends ItemManagerModule<DomComponentsCon
    */
   addType(type: string, methods: AddComponentTypeOptions) {
     const { em } = this;
-    const { model = {}, view = {}, isComponent, extend, extendView, extendFn = [], extendFnView = [] } = methods;
+    const { model = {}, view = {}, isComponent, extend, extendView, extendFn = [], extendFnView = [], block } = methods;
     const compType = this.getType(type);
     const extendType = this.getType(extend!);
     const extendViewType = this.getType(extendView!);
     const typeToExtend = extendType ? extendType : compType ? compType : this.getType('default');
-    const modelToExt = typeToExtend.model;
+    const modelToExt = typeToExtend.model as typeof Component;
     const viewToExt = extendViewType ? extendViewType.view : typeToExtend.view;
 
     // Function for extending source object methods
@@ -543,12 +545,16 @@ export default class ComponentManager extends ItemManagerModule<DomComponentsCon
     if (typeof model === 'object') {
       const modelDefaults = { defaults: model.defaults };
       delete model.defaults;
+      const typeExtends = new Set(modelToExt.typeExtends);
+      typeExtends.add(modelToExt.getDefaults().type);
+
       methods.model = modelToExt.extend(
         {
           ...model,
           ...getExtendedObj(extendFn, model, modelToExt),
         },
         {
+          typeExtends,
           isComponent: compType && !extendType && !isComponent ? modelToExt.isComponent : isComponent || (() => 0),
         }
       );
@@ -577,6 +583,16 @@ export default class ComponentManager extends ItemManagerModule<DomComponentsCon
       this.componentTypes.unshift(methods as any);
     }
 
+    if (block) {
+      const defBlockProps: BlockProperties = {
+        id: type,
+        label: type,
+        content: { type },
+      };
+      const blockProps: BlockProperties = block === true ? defBlockProps : { ...defBlockProps, ...block };
+      em.Blocks.add(blockProps.id || type, blockProps);
+    }
+
     const event = `component:type:${compType ? 'update' : 'add'}`;
     em?.trigger(event, compType || methods);
 

src/dom_components/model/Component.ts

@@ -331,7 +331,7 @@ export default class Component extends StyleableModel<ComponentProperties> {
 
   __postRemove() {
     const { em } = this;
-    const um = em?.get('UndoManager');
+    const um = em?.UndoManager;
     if (um) {
       um.remove(this.components());
       um.remove(this.getSelectors());
@@ -1347,6 +1347,14 @@ export default class Component extends StyleableModel<ComponentProperties> {
     );
   }
 
+  /**
+   * Update component name.
+   * @param {String} name New name.
+   */
+  setName(name?: string, opts: SetOptions = {}) {
+    this.set('custom-name', name, opts);
+  }
+
   /**
    * Get the icon string
    * @return {String}
@@ -1763,7 +1771,10 @@ export default class Component extends StyleableModel<ComponentProperties> {
 
     if (!cmp) return false;
 
-    return this instanceof cmp;
+    // A tiny hack to make isInstanceOf work properly where there a multiple inheritance
+    const { typeExtends } = this.constructor as typeof Component;
+
+    return this instanceof cmp || typeExtends.has(type);
   }
 
   /**
@@ -1853,6 +1864,8 @@ export default class Component extends StyleableModel<ComponentProperties> {
     selector && selector.set({ name: id, label: id });
   }
 
+  static typeExtends = new Set<string>();
+
   static getDefaults() {
     return result(this.prototype, 'defaults');
   }

src/dom_components/model/Components.ts

@@ -16,7 +16,7 @@ import {
 import ComponentText from './ComponentText';
 import ComponentWrapper from './ComponentWrapper';
 import { ComponentsEvents } from '../types';
-import { isSymbolInstance, isSymbolRoot } from './SymbolUtils';
+import { isSymbolInstance, isSymbolRoot, updateSymbolComps } from './SymbolUtils';
 
 export const getComponentIds = (cmp?: Component | Component[] | Components, res: string[] = []) => {
   if (!cmp) return [];
@@ -205,7 +205,10 @@ Component> {
       }
 
       const inner = removed.components();
-      inner.forEach(it => this.removeChildren(it, coll, opts));
+      inner.forEach(it => {
+        updateSymbolComps(it, it, inner, { ...opts, skipRefsUp: true });
+        this.removeChildren(it, coll, opts);
+      });
     }
 
     // Remove stuff registered in DomComponents.handleChanges

src/dom_components/model/SymbolUtils.ts

@@ -181,17 +181,28 @@ export const updateSymbolComps = (symbol: Component, m: Component, c: Components
 
   // Reset
   if (!o) {
+    const coll = m as unknown as Components;
     const toUp = getSymbolsToUpdate(symbol, {
       ...toUpOpts,
       changed: 'components:reset',
     });
-    // @ts-ignore
-    const cmps = m.models as Component[];
+    const cmps = coll.models;
+    const newSymbols = new Set<Component>();
     logSymbol(symbol, 'reset', toUp, { components: cmps });
-    toUp.forEach(symb => {
-      const newMods = cmps.map(mod => mod.clone({ symbol: true }));
-      // @ts-ignore
-      symb.components().reset(newMods, { fromInstance: symbol, ...c });
+
+    toUp.forEach(rel => {
+      const relCmps = rel.components();
+      const toReset = cmps.map((cmp, i) => {
+        // This particular case here is to handle reset from `resetFromString`
+        // where we can receive an array of regulat components or already
+        // existing symbols (updated already before reset)
+        if (!isSymbol(cmp) || newSymbols.has(cmp)) {
+          newSymbols.add(cmp);
+          return cmp.clone({ symbol: true });
+        }
+        return relCmps.at(i);
+      });
+      relCmps.reset(toReset, { fromInstance: symbol, ...c } as any);
     });
     // Add
   } else if (o.add) {
@@ -236,7 +247,7 @@ export const updateSymbolComps = (symbol: Component, m: Component, c: Components
       );
 
     // Propagate remove only if the component is an inner symbol
-    if (!isSymbolRoot(m)) {
+    if (!isSymbolRoot(m) && !o.skipRefsUp) {
       const changed = 'components:remove';
       const { index } = o;
       const parent = m.parent();

src/navigator/view/ItemView.ts

@@ -230,22 +230,18 @@ export default class ItemView extends View {
    */
   handleEditEnd(ev?: KeyboardEvent) {
     ev?.stopPropagation();
-    const { em, $el, clsNoEdit, clsEdit } = this;
+    const { em, $el, clsNoEdit, clsEdit, model } = this;
     const inputEl = this.getInputName();
     const name = inputEl.textContent!;
     inputEl.scrollLeft = 0;
     inputEl[inputProp] = 'false';
-    this.setName(name, { component: this.model, propName: 'custom-name' });
+    model.setName(name);
     em.setEditing(false);
     $el.find(`.${this.inputNameCls}`).addClass(clsNoEdit).removeClass(clsEdit);
     // Ensure to always update the layer name #4544
     this.updateName();
   }
 
-  setName(name: string, { propName }: { propName: string; component?: Component }) {
-    this.model.set(propName, name);
-  }
-
   /**
    * Get the input containing the name of the component
    * @return {HTMLElement}

test/specs/dom_components/model/Symbols.ts

@@ -413,6 +413,91 @@ describe('Symbols', () => {
     });
   });
 
+  test('Removing a component containing an instance, will remove the reference in the main', () => {
+    const container = wrapper.append('<custom-el></custom-el>')[0];
+    const comp = container.append(simpleComp)[0];
+    const symbol = createSymbol(comp);
+
+    const commonInfo = {
+      isSymbol: true,
+      main: symbol,
+      instances: [comp],
+    };
+
+    expect(getSymbolInfo(symbol)).toEqual({
+      ...commonInfo,
+      isMain: true,
+      isInstance: false,
+      relatives: [comp],
+    });
+    expect(comp.parent()).toEqual(container);
+
+    container.remove();
+
+    expect(getSymbolInfo(symbol)).toEqual({
+      ...commonInfo,
+      isMain: true,
+      isInstance: false,
+      relatives: [],
+      instances: [],
+    });
+
+    // the main doesn't lose its children
+    expect(symbol.getInnerHTML()).toBe('Component');
+  });
+
+  test('Symbols are working properly when using resetFromString (text component)', () => {
+    const comp = wrapper.append('<div data-a="b">Component <b id="bld">bold</b></div>')[0];
+    const innerNode = comp.components().at(0);
+    const innerCmp = comp.components().at(1);
+    expect(innerNode.toHTML()).toBe('Component ');
+    expect(innerCmp.getInnerHTML()).toBe('bold');
+
+    const symbol = createSymbol(comp);
+    const comp2 = createSymbol(comp);
+    const symbolInner = symbol.components().at(1);
+    const innerCmp2 = comp2.components().at(1);
+
+    expect(getSymbolInfo(innerCmp)).toEqual({
+      isSymbol: true,
+      main: symbolInner,
+      instances: [innerCmp, innerCmp2],
+      isMain: false,
+      isInstance: true,
+      relatives: [symbolInner, innerCmp2],
+    });
+
+    comp.components().resetFromString('Component2 <b id="bld">bold2</b>');
+    expect(comp.components().at(1)).toBe(innerCmp);
+    expect(comp2.components().at(1)).toBe(innerCmp2);
+    expect(innerCmp.getInnerHTML()).toBe('bold2');
+
+    expect(getSymbolInfo(innerCmp)).toEqual({
+      isSymbol: true,
+      main: symbolInner,
+      instances: [innerCmp, innerCmp2],
+      isMain: false,
+      isInstance: true,
+      relatives: [symbolInner, innerCmp2],
+    });
+
+    expect(getSymbolInfo(innerCmp2)).toEqual({
+      isSymbol: true,
+      main: symbolInner,
+      instances: [innerCmp, innerCmp2],
+      isMain: false,
+      isInstance: true,
+      relatives: [symbolInner, innerCmp],
+    });
+
+    expect(comp.components().at(0).getInnerHTML()).toBe('Component2 ');
+    expect(symbol.components().at(0).getInnerHTML()).toBe('Component2 ');
+    expect(comp2.components().at(0).getInnerHTML()).toBe('Component2 ');
+    expect(innerCmp.getInnerHTML()).toBe('bold2');
+    expect(innerCmp2.getInnerHTML()).toBe('bold2');
+    expect(symbolInner.getInnerHTML()).toBe('bold2');
+  });
+
   test('New component added to an instance is correctly propogated to all others', () => {
     const comp = wrapper.append(compMultipleNodes)[0];
     const compLen = comp.components().length;