Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Added some missing tests and made small code simplications.
Browse files Browse the repository at this point in the history
  • Loading branch information
Reinmar committed Dec 31, 2017
1 parent 26b3806 commit 4f4ead2
Show file tree
Hide file tree
Showing 10 changed files with 185 additions and 60 deletions.
2 changes: 1 addition & 1 deletion src/model/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ export default class Document {
const type = ( data.walker == backwardWalker ? 'elementEnd' : 'elementStart' );
const value = data.value;

if ( value.type == type && schema.isObject( value.item.name ) ) {
if ( value.type == type && schema.isObject( value.item ) ) {
return Range.createOn( value.item );
}

Expand Down
2 changes: 1 addition & 1 deletion src/model/documentselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ export default class DocumentSelection extends Selection {
// ...look for a first character node in that range and take attributes from it.
for ( const value of range ) {
// If the item is an object, we don't want to get attributes from its children.
if ( value.item.is( 'element' ) && schema.isObject( value.item.name ) ) {
if ( value.item.is( 'element' ) && schema.isObject( value.item ) ) {
break;
}

Expand Down
10 changes: 3 additions & 7 deletions src/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,9 @@ export default class Model {
this.schema.register( '$text', {
allowIn: '$block'
} );

// TODO review
// Create an "all allowed" context in the schema for processing the pasted content.
// Read: https://github.com/ckeditor/ckeditor5-engine/issues/638#issuecomment-255086588

this.schema.register( '$clipboardHolder', {
allowContentOf: '$root'
allowContentOf: '$root',
isLimit: true
} );
this.schema.extend( '$text', { allowIn: '$clipboardHolder' } );
}
Expand Down Expand Up @@ -317,7 +313,7 @@ export default class Model {

for ( const item of rangeOrElement.getItems() ) {
// Remember, `TreeWalker` returns always `textProxy` nodes.
if ( item.is( 'textProxy' ) || this.schema.isObject( item.name ) ) {
if ( item.is( 'textProxy' ) || this.schema.isObject( item ) ) {
return true;
}
}
Expand Down
63 changes: 42 additions & 21 deletions src/model/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,24 @@
* For licensing, see LICENSE.md.
*/

/**
* @module engine/model/schema
*/

import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
import mix from '@ckeditor/ckeditor5-utils/src/mix';

import Range from './range';

/**
* @module engine/model/schema
*/

/**
* @mixes module:utils/emittermixin~ObservableMixin
*/
export default class Schema {
constructor() {
this._sourceRules = {};

// TODO events
// TODO docs
this.decorate( 'checkChild' );
this.decorate( 'checkAttribute' );

Expand All @@ -36,7 +36,9 @@ export default class Schema {
register( itemName, rules ) {
if ( this._sourceRules[ itemName ] ) {
// TODO docs
throw new CKEditorError( 'schema-cannot-register-item-twice: A single item cannot be registered twice in the schema.' );
throw new CKEditorError( 'schema-cannot-register-item-twice: A single item cannot be registered twice in the schema.', {
itemName
} );
}

this._sourceRules[ itemName ] = [
Expand All @@ -47,11 +49,11 @@ export default class Schema {
}

extend( itemName, rules ) {
// TODO it should not throw if we want to allow e.g. adding attrs before element is registered
// (which may be done by another feature).
if ( !this._sourceRules[ itemName ] ) {
// TODO docs
throw new CKEditorError( 'schema-cannot-extend-missing-item: Cannot extend an item which was not registered yet.' );
throw new CKEditorError( 'schema-cannot-extend-missing-item: Cannot extend an item which was not registered yet.', {
itemName
} );
}

this._sourceRules[ itemName ].push( Object.assign( {}, rules ) );
Expand Down Expand Up @@ -86,30 +88,42 @@ export default class Schema {
return this.getRules()[ itemName ];
}

isRegistered( itemName ) {
return !!this.getRule( itemName );
/**
* @param {module:engine/model/item~Item|SchemaContextItem|String} item
*/
isRegistered( item ) {
return !!this.getRule( item );
}

isBlock( itemName ) {
const rule = this.getRule( itemName );
/**
* @param {module:engine/model/item~Item|SchemaContextItem|String} item
*/
isBlock( item ) {
const rule = this.getRule( item );

return !!( rule && rule.isBlock );
}

isLimit( itemName ) {
const rule = this.getRule( itemName );
/**
* @param {module:engine/model/item~Item|SchemaContextItem|String} item
*/
isLimit( item ) {
const rule = this.getRule( item );

return !!( rule && rule.isLimit );
}

isObject( itemName ) {
const rule = this.getRule( itemName );
/**
* @param {module:engine/model/item~Item|SchemaContextItem|String} item
*/
isObject( item ) {
const rule = this.getRule( item );

return !!( rule && rule.isObject );
}

/**
* @param {module:engine/model/element~Element|module:engine/model/position~Position|Array.<String>} context
* @param {SchemaContextDefinition} context
* @param {module:engine/model/node~Node|String}
*/
checkChild( context, child ) {
Expand All @@ -123,7 +137,7 @@ export default class Schema {
}

/**
* @param {module:engine/model/node~Node} context
* @param {SchemaContextDefinition} context
* @param {String}
*/
checkAttribute( context, attributeName ) {
Expand Down Expand Up @@ -154,7 +168,7 @@ export default class Schema {
return node.getCommonAncestor( range.getCommonAncestor() );
}, null );

while ( !this.isLimit( element.name ) ) {
while ( !this.isLimit( element ) ) {
if ( element.parent ) {
element = element.parent;
} else {
Expand Down Expand Up @@ -329,7 +343,7 @@ export class SchemaContext {
}

/**
* Returns an iterator that iterates over all context items
* Returns an iterator that iterates over all context items.
*
* @returns {Iterator.<TODO>}
*/
Expand All @@ -350,6 +364,13 @@ export class SchemaContext {
}
}

/**
* TODO
*
* @typedef {module:engine/model/node~Node|module:engine/model/position~Position|
* Array.<String|module:engine/model/node~Node>} SchemaContextDefinition
*/

function compileBaseItemRule( sourceItemRules, itemName ) {
const itemRule = {
name: itemName,
Expand Down
2 changes: 1 addition & 1 deletion src/model/selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ function isUnvisitedBlockContainer( element, visited ) {

visited.add( element );

return element.document.model.schema.isBlock( element.name ) && element.parent;
return element.document.model.schema.isBlock( element ) && element.parent;
}

// Finds the lowest element in position's ancestors which is a block.
Expand Down
2 changes: 1 addition & 1 deletion src/model/utils/deletecontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ function checkCanBeMerged( leftPos, rightPos, schema ) {
const rangeToCheck = new Range( leftPos, rightPos );

for ( const value of rangeToCheck.getWalker() ) {
if ( schema.isObject( value.item.name ) || schema.isLimit( value.item.name ) ) {
if ( schema.isObject( value.item ) || schema.isLimit( value.item ) ) {
return false;
}
}
Expand Down
15 changes: 3 additions & 12 deletions src/model/utils/insertcontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ class Insertion {
// Let's handle object in a special way.
// * They should never be merged with other elements.
// * If they are not allowed in any of the selection ancestors, they could be either autoparagraphed or totally removed.
if ( this._checkIsObject( node ) ) {
if ( this.schema.isObject( node ) ) {
this._handleObject( node, context );

return;
Expand Down Expand Up @@ -258,7 +258,7 @@ class Insertion {
livePos.detach();

// The last inserted object should be selected because we can't put a collapsed selection after it.
if ( this._checkIsObject( node ) && !this.schema.checkChild( this.position, '$text' ) ) {
if ( this.schema.isObject( node ) && !this.schema.checkChild( this.position, '$text' ) ) {
this.nodeToSelect = node;
} else {
this.nodeToSelect = null;
Expand Down Expand Up @@ -355,7 +355,7 @@ class Insertion {

while ( allowedIn != this.position.parent ) {
// If a parent which we'd need to leave is a limit element, break.
if ( this.schema.isLimit( this.position.parent.name ) ) {
if ( this.schema.isLimit( this.position.parent ) ) {
return false;
}

Expand Down Expand Up @@ -402,13 +402,4 @@ class Insertion {

return null;
}

/**
* Checks whether according to the schema this is an object type element.
*
* @param {module:engine/model/node~Node} node The node to check.
*/
_checkIsObject( node ) {
return this.schema.isObject( node.is( 'text' ) ? '$text' : node.name );
}
}
4 changes: 2 additions & 2 deletions src/model/utils/modifyselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function tryExtendingTo( data, value ) {
// Entering an element.
if ( value.type == ( data.isForward ? 'elementStart' : 'elementEnd' ) ) {
// If it's an object, we can select it now.
if ( data.schema.isObject( value.item.name ) ) {
if ( data.schema.isObject( value.item ) ) {
return Position.createAt( value.item, data.isForward ? 'after' : 'before' );
}

Expand All @@ -94,7 +94,7 @@ function tryExtendingTo( data, value ) {
// Leaving an element.
else {
// If leaving a limit element, stop.
if ( data.schema.isLimit( value.item.name ) ) {
if ( data.schema.isLimit( value.item ) ) {
// NOTE: Fast-forward the walker until the end.
data.walker.skip( () => true );

Expand Down
31 changes: 28 additions & 3 deletions tests/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,32 @@ describe( 'Model', () => {
changes = '';
} );

describe( 'change & enqueueChange', () => {
describe( 'constructor()', () => {
it( 'registers $root to the schema', () => {
expect( schema.isRegistered( '$root' ) ).to.be.true;
expect( schema.isLimit( '$root' ) ).to.be.true;
} );

it( 'registers $block to the schema', () => {
expect( schema.isRegistered( '$block' ) ).to.be.true;
expect( schema.isBlock( '$block' ) ).to.be.true;
expect( schema.checkChild( [ '$root' ], '$block' ) ).to.be.true;
} );

it( 'registers $text to the schema', () => {
expect( schema.isRegistered( '$text' ) ).to.be.true;
expect( schema.checkChild( [ '$block' ], '$text' ) ).to.be.true;
} );

it( 'registers $clipboardHolder to the schema', () => {
expect( schema.isRegistered( '$clipboardHolder' ) ).to.be.true;
expect( schema.isLimit( '$clipboardHolder' ) ).to.be.true;
expect( schema.checkChild( [ '$clipboardHolder' ], '$text' ) ).to.be.true;
expect( schema.checkChild( [ '$clipboardHolder' ], '$block' ) ).to.be.true;
} );
} );

describe( 'change() & enqueueChange()', () => {
it( 'should execute changes immediately', () => {
model.change( () => {
changes += 'A';
Expand Down Expand Up @@ -318,7 +343,7 @@ describe( 'Model', () => {
} );
} );

describe( 'applyOperation', () => {
describe( 'applyOperation()', () => {
it( 'should execute provided operation end return the result of operation', () => {
const returnValue = { foo: 'bar' };

Expand All @@ -334,7 +359,7 @@ describe( 'Model', () => {
} );
} );

describe( 'transformDeltas', () => {
describe( 'transformDeltas()', () => {
it( 'should use deltaTransform.transformDeltaSets', () => {
sinon.spy( deltaTransform, 'transformDeltaSets' );

Expand Down
Loading

0 comments on commit 4f4ead2

Please sign in to comment.