Skip to content

Commit

Permalink
Merge pull request #15287 from ckeditor/ck/15246
Browse files Browse the repository at this point in the history 
Feature (editor-multi-root): Added `MultiRootEditor#registerRootAttribute()`. All roots attributes used by features should now be registered. Roots attributes passed in editor config are now automatically registered. Closes #15246.

MINOR BREAKING CHANGE (editor-multi-root): If you use a custom plugin that uses roots attributes, it is recommended to use newly added `MultiRootEditor#registerRootAttribute()` to register the custom root attribute.
  • Loading branch information
@scofalik
scofalik authored Nov 6, 2023
2 parents 64312d8 + e385d23 commit 5404c1a
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 23 deletions.
63 changes: 41 additions & 22 deletions packages/ckeditor5-editor-multi-root/src/multirooteditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ export default class MultiRootEditor extends DataApiMixin( Editor ) {
}

for ( const key of Object.keys( attributes ) ) {
this._registeredRootsAttributesKeys.add( key );
this.registerRootAttribute( key );
}
}

Expand Down Expand Up @@ -386,9 +386,10 @@ export default class MultiRootEditor extends DataApiMixin( Editor ) {
* editor.addRoot( 'myRoot', { attributes: { isCollapsed: true, index: 4 } } );
* ```
*
* See also {@link module:core/editor/editorconfig~EditorConfig#rootsAttributes `rootsAttributes` configuration option}.
* Note that attributes added together with a root are automatically registered.
*
* Note that attributes keys of attributes added in `attributes` option are also included in {@link #getRootsAttributes} return value.
* See also {@link ~#registerRootAttribute `MultiRootEditor#registerRootAttribute()`} and
* {@link module:core/editor/editorconfig~EditorConfig#rootsAttributes `config.rootsAttributes` configuration option}.
*
* By setting `isUndoable` flag to `true`, you can allow for detaching the root using the undo feature.
*
Expand All @@ -414,26 +415,23 @@ export default class MultiRootEditor extends DataApiMixin( Editor ) {
rootName: string,
{ data = '', attributes = {}, elementName = '$root', isUndoable = false }: AddRootOptions = {}
): void {
const dataController = this.data;
const registeredKeys = this._registeredRootsAttributesKeys;

if ( isUndoable ) {
this.model.change( _addRoot );
} else {
this.model.enqueueChange( { isUndoable: false }, _addRoot );
}

function _addRoot( writer: Writer ) {
const _addRoot = ( writer: Writer ) => {
const root = writer.addRoot( rootName, elementName );

if ( data ) {
writer.insert( dataController.parse( data, root ), root, 0 );
writer.insert( this.data.parse( data, root ), root, 0 );
}

for ( const key of Object.keys( attributes ) ) {
registeredKeys.add( key );
this.registerRootAttribute( key );
writer.setAttribute( key, attributes[ key ], root );
}
};

if ( isUndoable ) {
this.model.change( _addRoot );
} else {
this.model.enqueueChange( { isUndoable: false }, _addRoot );
}
}

Expand Down Expand Up @@ -550,6 +548,11 @@ export default class MultiRootEditor extends DataApiMixin( Editor ) {
*
* This method is {@link module:utils/observablemixin~Observable#decorate decorated}.
*
* Note that attributes loaded together with a root are automatically registered.
*
* See also {@link ~#registerRootAttribute `MultiRootEditor#registerRootAttribute()`} and
* {@link module:core/editor/editorconfig~EditorConfig#rootsAttributes `config.rootsAttributes` configuration option}.
*
* When this method is used in real-time collaboration environment, its effects become asynchronous as the editor will first synchronize
* with the remote editing session, before the root is added to the editor.
*
Expand All @@ -572,7 +575,7 @@ export default class MultiRootEditor extends DataApiMixin( Editor ) {
}

for ( const key of Object.keys( attributes ) ) {
this._registeredRootsAttributesKeys.add( key );
this.registerRootAttribute( key );

writer.setAttribute( key, attributes[ key ], root );
}
Expand Down Expand Up @@ -606,8 +609,8 @@ export default class MultiRootEditor extends DataApiMixin( Editor ) {
/**
* Returns attributes for all attached roots.
*
* Note: only attributes specified in {@link module:core/editor/editorconfig~EditorConfig#rootsAttributes `rootsAttributes`}
* configuration option will be returned.
* Note: all and only {@link ~#registerRootAttribute registered} roots attributes will be returned. If a registered root attribute
* is not set for a given root, `null` will be returned.
*
* @returns Object with roots attributes. Keys are roots names, while values are attributes set on given root.
*/
Expand All @@ -624,10 +627,8 @@ export default class MultiRootEditor extends DataApiMixin( Editor ) {
/**
* Returns attributes for the specified root.
*
* Note: only attributes specified in {@link module:core/editor/editorconfig~EditorConfig#rootsAttributes `rootsAttributes`}
* configuration option will be returned.
*
* @param rootName
* Note: all and only {@link ~#registerRootAttribute registered} roots attributes will be returned. If a registered root attribute
* is not set for a given root, `null` will be returned.
*/
public getRootAttributes( rootName: string ): RootAttributes {
const rootAttributes: RootAttributes = {};
Expand All @@ -640,6 +641,24 @@ export default class MultiRootEditor extends DataApiMixin( Editor ) {
return rootAttributes;
}

/**
* Registers given string as a root attribute key. Registered root attributes are added to
* {@link module:engine/model/schema~Schema schema}, and also returned by
* {@link ~#getRootAttributes `getRootAttributes()`} and {@link ~#getRootsAttributes `getRootsAttributes()`}.
*
* Note: attributes passed in {@link module:core/editor/editorconfig~EditorConfig#rootsAttributes `config.rootsAttributes`} are
* automatically registered as the editor is initialized. However, registering the same attribute twice does not have any negative
* impact, so it is recommended to use this method in any feature that uses roots attributes.
*/
public registerRootAttribute( key: string ): void {
if ( this._registeredRootsAttributesKeys.has( key ) ) {
return;
}

this._registeredRootsAttributesKeys.add( key );
this.editing.model.schema.extend( '$root', { allowAttributes: key } );
}

/**
* Switches given editor root to the read-only mode.
*
Expand Down
89 changes: 88 additions & 1 deletion packages/ckeditor5-editor-multi-root/tests/multirooteditor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,12 +484,17 @@ describe( 'MultiRootEditor', () => {
} );

it( 'should add a model root with given attributes', () => {
sinon.spy( editor, 'registerRootAttribute' );

editor.addRoot( 'bar', { attributes: { order: 20, isLocked: true } } );

const root = editor.model.document.getRoot( 'bar' );

expect( root.getAttribute( 'order' ) ).to.equal( 20 );
expect( root.getAttribute( 'isLocked' ) ).to.be.true;

expect( editor.registerRootAttribute.calledWithExactly( 'order' ) );
expect( editor.registerRootAttribute.calledWithExactly( 'isLocked' ) );
} );

it( 'should add a model root which can be undone by undo feature if `isUndoable` is set to `true`', () => {
Expand Down Expand Up @@ -619,11 +624,15 @@ describe( 'MultiRootEditor', () => {
} );

it( 'should set not-loaded root as loaded and set initial data and attributes', () => {
sinon.spy( editor, 'registerRootAttribute' );

editor.loadRoot( 'foo', { data: '<p>Foo</p>', attributes: { order: 100 } } );

expect( root._isLoaded ).to.be.true;
expect( editor.getData( { rootName: 'foo' } ) ).to.equal( '<p>Foo</p>' );
expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { order: 100 } );

expect( editor.registerRootAttribute.calledWithExactly( 'order' ) );
} );

it( 'should load an empty root', () => {
Expand Down Expand Up @@ -1055,6 +1064,25 @@ describe( 'MultiRootEditor', () => {
await editor.destroy();
} );

it( 'should register all root attributes passed in the config', async () => {
editor = await MultiRootEditor.create( { foo: '', bar: '' }, {
rootsAttributes: {
foo: { order: 10 },
bar: { isLocked: false }
}
} );

expect( editor.getRootsAttributes() ).to.deep.equal( {
foo: { order: 10, isLocked: null },
bar: { order: null, isLocked: false }
} );

expect( editor.editing.model.schema.checkAttribute( '$root', 'order' ) ).to.be.true;
expect( editor.editing.model.schema.checkAttribute( '$root', 'isLocked' ) ).to.be.true;

await editor.destroy();
} );

it( 'should throw when trying to set an attribute on non-existing root', done => {
MultiRootEditor.create( { foo: '', bar: '' }, {
rootsAttributes: {
Expand Down Expand Up @@ -1100,7 +1128,34 @@ describe( 'MultiRootEditor', () => {
await editor.destroy();
} );

it( 'should not return roots attributes that were not specified in config', async () => {
it( 'should return roots attributes that were registered', async () => {
editor = await MultiRootEditor.create( { foo: '', bar: '' }, {
rootsAttributes: {
foo: { order: 10 },
bar: {}
}
} );

editor.registerRootAttribute( 'isLocked' );

editor.model.change( writer => {
writer.setAttribute( 'isLocked', true, editor.model.document.getRoot( 'bar' ) );
} );

expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( {
isLocked: null,
order: 10
} );

expect( editor.getRootAttributes( 'bar' ) ).to.deep.equal( {
isLocked: true,
order: null
} );

await editor.destroy();
} );

it( 'should not return roots attributes that were not registered', async () => {
editor = await MultiRootEditor.create( { foo: '', bar: '' }, {
rootsAttributes: {
foo: { order: 10, isLocked: true },
Expand Down Expand Up @@ -1209,6 +1264,38 @@ describe( 'MultiRootEditor', () => {
await editor.destroy();
} );

it( 'should return all and only roots attributes that were registered', async () => {
editor = await MultiRootEditor.create( { foo: '', bar: '' }, {
rootsAttributes: {
foo: { order: 10 },
bar: {}
}
} );

editor.registerRootAttribute( 'isLocked' );

editor.model.change( writer => {
writer.setAttribute( 'isLocked', true, editor.model.document.getRoot( 'bar' ) );

// Not registered:
writer.setAttribute( 'abc', true, editor.model.document.getRoot( 'foo' ) );
writer.setAttribute( 'abc', false, editor.model.document.getRoot( 'bar' ) );
} );

expect( editor.getRootsAttributes( 'foo' ) ).to.deep.equal( {
foo: {
isLocked: null,
order: 10
},
bar: {
isLocked: true,
order: null
}
} );

await editor.destroy();
} );

it( 'should return attributes for all and only currently attached roots', async () => {
editor = await MultiRootEditor.create( { foo: '', bar: '' }, {
rootsAttributes: {
Expand Down
15 changes: 15 additions & 0 deletions packages/ckeditor5-engine/tests/model/schema.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2063,6 +2063,21 @@ describe( 'Schema', () => {
.to.equal( '<div><$text a="1">foo</$text>bar<$text a="1">biz</$text></div>' );
} );
} );

// Related to https://github.com/ckeditor/ckeditor5/issues/15246.
it( 'should filter out only non-allowed root attributes', () => {
schema.extend( '$root', { allowAttributes: 'allowed' } );

model.change( writer => {
writer.setAttribute( 'allowed', 'value', root );
writer.setAttribute( 'other', true, root );

schema.removeDisallowedAttributes( [ root ], writer );
} );

expect( root.getAttribute( 'allowed' ) ).to.equal( 'value' );
expect( root.getAttribute( 'other' ) ).to.be.undefined;
} );
} );

describe( 'getAttributesWithProperty()', () => {
Expand Down

0 comments on commit 5404c1a

Please sign in to comment.