Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added MultiRootEditor#registerRootAttribute() #15287

Merged
merged 3 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 40 additions & 22 deletions packages/ckeditor5-editor-multi-root/src/multirooteditor.ts
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,23 @@ 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`}.
scofalik marked this conversation as resolved.
Show resolved Hide resolved
*
* 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 ) ) {
this._registeredRootsAttributesKeys.add( key );
}

this.editing.model.schema.extend( '$root', { allowAttributes: key } );
scofalik marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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
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
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