Skip to content

Commit

Permalink
prevent drop a symbol in itself or similar recursive issues
Browse files Browse the repository at this point in the history
  • Loading branch information
lexoyo committed Aug 14, 2023
1 parent f6e21b6 commit 992fccc
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 21 deletions.
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module.exports = {
transform: {},
preset: "ts-jest",
testEnvironment: "jsdom",
}

19 changes: 14 additions & 5 deletions src/SymbolsCommands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@ import Backbone from 'backbone'

import { jest } from '@jest/globals'

// Mock module lit-html
// This is needed because lit-html fails to load in jest (Must use import to load ES Module: node_modules/lit-html/lit-html.js)
jest.mock('lit-html', () => {
const render = jest.fn()
const html = jest.fn()
return { render, html }
})

import { addSymbol } from './SymbolsCommands'
import {
createSymbolInstance,
Expand All @@ -11,13 +19,11 @@ import {
import { getTestSymbols } from './test-utils'
import { Symbols } from './model/Symbols'

let commands, editor

test('Command symbols:add', () => {
const { editor, s1, s2 } = getTestSymbols()
editor.Symbols = new Backbone.Collection() as Symbols
const sender = {}, label = 'label', icon = 'icon'
expect(() => addSymbol(editor, sender, {label, icon, component: null})).toThrow('missing param component')
expect(() => addSymbol(editor, sender, {label, icon, component: undefined})).toThrow('missing param component')
expect(editor.Symbols).toHaveLength(0)
const [component] = editor.addComponents([{
tagName: 'div',
Expand Down Expand Up @@ -68,13 +74,16 @@ test('Command symbols:unlink', () => {
})

test('Command symbols:create', () => {
const { s1, comp1, editor } = getTestSymbols()
const { s1, comp1, s2, editor } = getTestSymbols()
const sender = {},
component = comp1,
pos = {},
target = { getAttribute: jest.fn((name) => comp1.getId()) } as any as HTMLElement
expect(() => createSymbolInstance(editor, sender, {} as any)).toThrow('missing param symbol')
expect(() => createSymbolInstance(editor, sender, {symbol: s1, pos, target})).not.toThrow()
expect(createSymbolInstance(editor, sender, {symbol: s1, pos, target}).get('symbolId')).toBe(s1.id)
// Add a symbol to the target
expect(createSymbolInstance(editor, sender, {symbol: s2, pos, target})?.get('symbolId')).toBe(s2.id)
// Try to add a symbol to a target that already has the symbol (S1)
expect(createSymbolInstance(editor, sender, {symbol: s1, pos, target})?.get('symbolId')).toBeUndefined()
expect(component.attributes.symbolId).not.toBeUndefined()
})
49 changes: 37 additions & 12 deletions src/SymbolsCommands.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// exported plugin
import { Editor, Component } from 'grapesjs'
import Symbol, { createSymbol, getSymbolId } from './model/Symbol'
import { setDirty } from './utils'
import { allowDrop, setDirty } from './utils'
import { SymbolEditor } from './model/Symbols'
import { html, render } from 'lit-html'

export const cmdAdd = 'symbols:add'
export const cmdRemove = 'symbols:remove'
Expand All @@ -19,6 +19,19 @@ export default function({ editor, options }: { editor: Editor, options: any}) {
// Symbol management functions
// These are exported for unit tests

export function displayError(editor: Editor, title: string, message: string) {
const content = document.createElement('div')
editor.Modal.open({
title,
content,
})
render(html`<main>
<p>${message}</p>
</main><footer>
<button class="gjs-btn-prim" @click=${() => editor.Modal.close()}>Close</button>
</footer>`, content)
}

/**
* Create a new symbol
* @param options.component - the component which will become the first instance of the symbol
Expand Down Expand Up @@ -92,18 +105,30 @@ export function createSymbolInstance(
editor: SymbolEditor,
sender: any,
{ symbol, pos, target }: { symbol: Symbol, pos: any, target: HTMLElement },
) {
pos = pos || { }
if(symbol && pos && target) {
): Component | null {
pos = pos || {}
if (symbol && pos && target) {
const parentId = target ? target.getAttribute('id') : undefined
if(!parentId) throw new Error('Can not create the symbol: missing param id')
if (!parentId) throw new Error('Can not create the symbol: missing param id')
const parent = target ? editor.Components.allById()[parentId!] : undefined
// create the new component
const [c] = parent ? parent.append([symbol.createInstance()], { at: pos.index }) : []
// Select the new component
// Break unit tests? editor.select(c, { scroll: true })
return c
// Check that it is a valid parent
if (parent) {
if(!allowDrop({target: symbol.get('model'), parent})) {
// Cancel and notify the user
displayError(editor, 'Error: can not create the symbol', 'One of the parent is in the symbol.</p><p>Please remove the parent from the symbol and try again.')
return null
} else {
// create the new component
const [c] = parent ? parent.append([symbol.createInstance()], { at: pos.index }) : []
// Select the new component
// Break unit tests? editor.select(c, { scroll: true })
return c
}
} else {
console.error('Can not create the symbol: parent not found')
return null
}
} else {
throw new Error('Can not create the symbol: missing param symbol, pos or target')
throw new Error('Can not create the symbol: missing param symbol or pos or target')
}
}
19 changes: 17 additions & 2 deletions src/model/Symbols.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Backbone, { ViewOptions } from 'backbone'

import { closestInstance, wait } from '../utils'
import Symbol, { getSymbolId, cleanup } from './Symbol'
import { allowDrop, closestInstance, wait } from '../utils'
import Symbol, { getSymbolId } from './Symbol'
import { Editor, Component, CssRule } from 'grapesjs'

// Editor with the symbols plugin
Expand Down Expand Up @@ -30,6 +30,7 @@ export class Symbols extends Backbone.Collection<Symbol> {
this.editor.on('component:update:classes', c => this.onUpdateClasses(c))
this.editor.on('component:input', c => this.onUpdateContent(c))
this.editor.on('styleable:change', cssRule => this.onStyleChanged(cssRule))
this.editor.on('component:drag', ({target, parent}) => this.onDrag({target, parent}))
}

/**
Expand All @@ -43,6 +44,20 @@ export class Symbols extends Backbone.Collection<Symbol> {
components.forEach(c => this.onAdd(c))
}

/**
* Prevent drop on a symbol into itself or things similar
*/
onDrag({target, parent}) {
if(parent?.get('droppable') && !allowDrop({target, parent})) {
// Prevent drop
parent.set('droppable', false)
// Reset after drop
this.editor.once('component:drag:end', () => {
parent.set('droppable', true)
})
}
}

/**
* Add a component to a symbol
* This is useful only when loading new HTML content
Expand Down
39 changes: 37 additions & 2 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Component } from 'grapesjs'
import { SymbolEditor } from './model/Symbols'
import { getSymbolId } from './model/Symbol'

/**
* set editor as dirty
Expand Down Expand Up @@ -76,10 +77,9 @@ export function find(c: Component, symbolChildId: string): Component | null {

/**
* find the first symbol in the parents (or the element itself)
* exported for unit tests
* @private
*/
export function closestInstance(c: Component) {
export function closestInstance(c: Component): Component | undefined {
let ptr: Component | undefined = c
while(ptr && !hasSymbolId(ptr)) {
ptr = ptr.parent()
Expand Down Expand Up @@ -160,5 +160,40 @@ export function setCaret(el: HTMLElement, { path, pos }: { path: number[], pos:
} else {
console.error('Could not keep the caret position', {el, path})
}
}

/**
* find the all the symbols in the parents (or the element itself)
* @private
*/
function allParentInstances(c: Component, includeSelf: boolean): Component[] {
let result = []
let ptr: Component | undefined = includeSelf ? c : c.parent()
while(ptr) {
if(hasSymbolId(ptr)) result.push(ptr)
ptr = ptr.parent()
}
return result
}

/**
* find the all the symbols in the children (or the element itself)
*/
function allChildrenInstances(c: Component, includeSelf: boolean): Component[] {
const children = c.components().toArray()
const result = []
if(includeSelf && hasSymbolId(c)) result.push(c)
return [c]
.concat(children
.flatMap(child => allChildrenInstances(child, true)) // include self only for the subsequent levels
)
}

/**
* Find if a parent is also a child of the symbol
*/
export function allowDrop({target, parent}): boolean {
const allParents = allParentInstances(parent, true)
const allChildren = allChildrenInstances(target, false)
return !allParents.find(p => allChildren.find(c => getSymbolId(c) === getSymbolId(p)))
}
1 change: 1 addition & 0 deletions src/view/SymbolsView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export default class extends Backbone.View {
protected lastPos: Position | null = null
protected lastTarget: HTMLElement | null = null
//initialize(model, { editor, options }) {
// FIXME: why is editor in options?
constructor(protected options: SymbolsViewOptions) {
super(options)
// listen to redraw UI
Expand Down

0 comments on commit 992fccc

Please sign in to comment.