Skip to content

Commit

Permalink
Prevent nesting instances of the same symbol (#6164)
Browse files Browse the repository at this point in the history
* Prevent nesting instances of the same symbol

* Fix a typo, format
  • Loading branch information
mohamedsalem401 authored Sep 24, 2024
1 parent 1c16e59 commit b044455
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 1 deletion.
15 changes: 14 additions & 1 deletion packages/core/src/dom_components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ import {
isSymbolInstance,
detachSymbolInstance,
isSymbolRoot,
isSymbol as isSymbolComponent,
getSymbolTop,
} from './model/SymbolUtils';
import { ComponentsEvents, SymbolInfo } from './types';
import Symbols from './model/Symbols';
Expand Down Expand Up @@ -840,9 +842,20 @@ export default class ComponentManager extends ItemManagerModule<DomComponentsCon
target,
source: null,
};

if (!source || !target) return result;

// Check if the target and source belong to the same root symbol
if (isSymbolComponent(target) && source instanceof Component && isSymbolComponent(source)) {
const targetRootSymbol = getSymbolTop(target);
const targetMain = isSymbolMain(targetRootSymbol) ? targetRootSymbol : getSymbolMain(targetRootSymbol);
const sourceRootSymbol = getSymbolTop(source as Component);
const sourceMain = isSymbolMain(sourceRootSymbol) ? sourceRootSymbol : getSymbolMain(sourceRootSymbol);

const sameRoot = targetMain === sourceMain;
const differentInstance = targetRootSymbol !== sourceRootSymbol;
if (sameRoot && differentInstance) return { ...result, reason: CanMoveReason.TargetReject };
}

let srcModel = isComponent(source) ? source : null;

if (!srcModel) {
Expand Down
72 changes: 72 additions & 0 deletions packages/core/test/specs/dom_components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Component } from '../../../src';
import ComponentWrapper from '../../../src/dom_components/model/ComponentWrapper';
import { flattenHTML, setupTestEditor } from '../../common';
import { ProjectData } from '../../../src/storage_manager';
import { CanMoveReason } from '../../../src/dom_components';

describe('DOM Components', () => {
describe('Main', () => {
Expand Down Expand Up @@ -47,6 +48,7 @@ describe('DOM Components', () => {
var setEm = () => {
config.em = editorModel;
};
const createSymbol = (component: Component) => obj.addSymbol(component)!;

beforeEach(() => {
const editor = new Editor({
Expand Down Expand Up @@ -314,6 +316,76 @@ describe('DOM Components', () => {
expect(rule.getStyle()).toEqual(newStyle);
});
});

describe('Custom components with styles', () => {
test('canMove returns false if source is not provided', () => {
const target = obj.addComponent({ type: 'div' }) as Component;
const result = obj.canMove(target);
expect(result.result).toBe(false);
expect(result.reason).toBe(CanMoveReason.InvalidSource);
});

test('canMove returns false if target is not provided', () => {
const source = obj.addComponent({ type: 'div' }) as Component;
const result = obj.canMove(undefined as any, source);
expect(result.result).toBe(false);
expect(result.reason).toBe(CanMoveReason.InvalidSource);
});

test('canMove returns false when source and target have the same main symbol', () => {
const component = obj.addComponent({ type: 'div' }) as Component;
const mainSymbol = obj.addSymbol(component) as Component;
const target = obj.addSymbol(mainSymbol) as Component;
const source = obj.addSymbol(mainSymbol) as Component;
expect(obj.canMove(mainSymbol, source)).toMatchObject({
result: false,
reason: CanMoveReason.TargetReject,
});

expect(obj.canMove(target, source)).toMatchObject({
result: false,
reason: CanMoveReason.TargetReject,
});
});

test('canMove returns true when source and target are the same instance', () => {
const component = obj.addComponent('<div><p>child</p></div>') as Component;
const mainSymbol = obj.addSymbol(component) as Component;
const childSymbol = mainSymbol.components().at(0);
expect(obj.canMove(mainSymbol, childSymbol).result).toBe(true);
});

test('canMove returns false when source is not draggable in the target', () => {
const target = obj.addComponent({ type: 'div', droppable: true }) as Component;
const source = obj.addComponent({ type: 'span', draggable: false }) as Component;
const result = obj.canMove(target, source);
expect(result.result).toBe(false);
expect(result.reason).toBe(CanMoveReason.SourceReject);
});

test('canMove returns false when target does not accept the source', () => {
const target = obj.addComponent({ type: 'div', droppable: false }) as Component;
const source = obj.addComponent({ type: 'span', draggable: true }) as Component;
const result = obj.canMove(target, source);
expect(result.result).toBe(false);
expect(result.reason).toBe(CanMoveReason.TargetReject);
});

test('canMove returns true when source is draggable and target is droppable', () => {
const target = obj.addComponent({ type: 'div', droppable: true }) as Component;
const source = obj.addComponent({ type: 'span', draggable: true }) as Component;
const result = obj.canMove(target, source);
expect(result.result).toBe(true);
});

test('canMove returns false when target is inside the source', () => {
const source = obj.addComponent({ type: 'div' }) as Component;
const target = source.append({ type: 'span' })[0];
const result = obj.canMove(target, source);
expect(result.result).toBe(false);
expect(result.reason).toBe(CanMoveReason.TargetReject);
});
});
});

describe('Rendered components', () => {
Expand Down

0 comments on commit b044455

Please sign in to comment.