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

Commit

Permalink
Merge pull request #33 from ckeditor/t/32
Browse files Browse the repository at this point in the history
Other: Made the list feature use the model.Selection#getSelectedBlocks() instead of implementing the same logic itself. Closes #32. Closes #31.
  • Loading branch information
scofalik committed Feb 15, 2017
2 parents 3071908 + 0129bb3 commit d04eab5
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 144 deletions.
18 changes: 14 additions & 4 deletions src/listcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
*/

import Command from '@ckeditor/ckeditor5-core/src/command/command';
import { getClosestListItem, getSelectedBlocks, getPositionBeforeBlock } from './utils';
import Position from '@ckeditor/ckeditor5-engine/src/model/position';
import { getClosestListItem } from './utils';

/**
* The list command. It is used by the {@link module:list/list~List list feature}.
Expand Down Expand Up @@ -72,7 +73,7 @@ export default class ListCommand extends Command {
*/
_doExecute( options = {} ) {
const document = this.editor.document;
const blocks = getSelectedBlocks( document.selection, document.schema );
const blocks = Array.from( document.selection.getSelectedBlocks() );

// Whether we are turning off some items.
const turnOff = this.value === true;
Expand Down Expand Up @@ -199,9 +200,18 @@ export default class ListCommand extends Command {

const selection = this.editor.document.selection;
const schema = this.editor.document.schema;
const position = getPositionBeforeBlock( selection.getFirstPosition(), schema );

const firstBlock = selection.getSelectedBlocks().next().value;

if ( !firstBlock ) {
return false;
}

// Otherwise, check if list item can be inserted at the position start.
return schema.check( { name: 'listItem', inside: position, attributes: [ 'type', 'indent' ] } );
return schema.check( {
name: 'listItem',
attributes: [ 'type', 'indent' ],
inside: Position.createBefore( firstBlock )
} );
}
}
55 changes: 0 additions & 55 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
* @module list/utils
*/

import Position from '@ckeditor/ckeditor5-engine/src/model/position';

/**
* For given {@link module:engine/model/position~Position position}, returns the closest ancestor of that position which is a
* `listItem` element.
Expand All @@ -22,56 +20,3 @@ import Position from '@ckeditor/ckeditor5-engine/src/model/position';
export function getClosestListItem( position ) {
return Array.from( position.getAncestors() ).find( ( parent ) => parent.name == 'listItem' ) || null;
}

/**
* For given {@link module:engine/model/selection~Selection selection} and {@link module:engine/model/schema~Schema schema},
* returns an array with all elements that are in the selection and are extending `$block` schema item.
*
* @param {module:engine/model/selection~Selection} selection Selection from which blocks will be taken.
* @param {module:engine/model/schema~Schema} schema Schema which will be used to check if a model element extends `$block`.
* @returns {Array.<module:engine/model/element~Element>} All blocks from the selection.
*/
export function getSelectedBlocks( selection, schema ) {
let position = getPositionBeforeBlock( selection.getFirstPosition(), schema );

const endPosition = selection.getLastPosition();
const blocks = [];

// Traverse model from the first position before a block to the end position of selection.
// Store all elements that were after the correct positions.
while ( position !== null && position.isBefore( endPosition ) ) {
blocks.push( position.nodeAfter );

position.offset++;
position = getPositionBeforeBlock( position, schema );
}

return blocks;
}

/**
* For given {@link module:engine/model/position~Position position}, finds a model element extending `$block` schema item which is
* closest element to that position. First node after the position is checked and then the position's ancestors. `null`
* is returned if such element has not been found or found element is a root element.
*
* @param position
* @param schema
* @returns {*}
*/
export function getPositionBeforeBlock( position, schema ) {
// Start from the element right after the position. Maybe it is already a `$block` element.
let element = position.nodeAfter;

// If the position is not before an element, check the parent.
if ( !element ) {
element = position.parent;
}

// If proper element is still not found, check the ancestors.
while ( element !== null && !schema.itemExtends( element.name || '$text', '$block' ) ) {
element = element.parent;
}

// If proper element has been found, return position before it, otherwise return null;
return element !== null && element.parent !== null ? Position.createBefore( element ) : null;
}
22 changes: 16 additions & 6 deletions tests/listcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,29 @@ describe( 'ListCommand', () => {
expect( command.isEnabled ).to.be.true;
} );

it( 'should be true if selection first position is in a place where listItem can be inserted', () => {
doc.selection.collapse( doc.getRoot(), 2 );
it( 'should be true if entire selection is in a list', () => {
setData( doc, '<listItem type="bulleted" indent="0">[a]</listItem>' );
expect( command.isEnabled ).to.be.true;
} );

doc.selection.collapse( doc.getRoot().getChild( 0 ) );
it( 'should be true if entire selection is in a block which can be turned into a list', () => {
setData( doc, '<paragraph>[a]</paragraph>' );
expect( command.isEnabled ).to.be.true;
} );

doc.selection.collapse( doc.getRoot().getChild( 2 ) );
it( 'should be true if selection first position is in a block which can be turned into a list', () => {
setData( doc, '<paragraph>[a</paragraph><widget>b]</widget>' );
expect( command.isEnabled ).to.be.true;
} );

it( 'should be false if selection first position is in a place where listItem cannot be inserted', () => {
doc.selection.collapse( doc.getRoot().getChild( 4 ) );
it( 'should be false if selection first position is in an element which cannot be converted to a list item', () => {
setData( doc, '<widget><paragraph>[a</paragraph></widget><paragraph>b]</paragraph>' );
expect( command.isEnabled ).to.be.false;
} );

it( 'should be false in a root which does not allow blocks at all', () => {
doc.createRoot( 'paragraph', 'inlineOnlyRoot' );
setData( doc, 'a[]b', { rootName: 'inlineOnlyRoot' } );
expect( command.isEnabled ).to.be.false;
} );
} );
Expand Down
80 changes: 1 addition & 79 deletions tests/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@
* For licensing, see LICENSE.md.
*/

import { getClosestListItem, getSelectedBlocks, getPositionBeforeBlock } from '../src/utils';
import { getClosestListItem } from '../src/utils';

import Element from '@ckeditor/ckeditor5-engine/src/model/element';
import Text from '@ckeditor/ckeditor5-engine/src/model/text';
import Position from '@ckeditor/ckeditor5-engine/src/model/position';
import Schema from '@ckeditor/ckeditor5-engine/src/model/schema';
import Selection from '@ckeditor/ckeditor5-engine/src/model/selection';

describe( 'getClosestListItem', () => {
const item = new Element( 'listItem', null, 'foobar' );
Expand All @@ -23,78 +20,3 @@ describe( 'getClosestListItem', () => {
expect( getClosestListItem( Position.createAt( root ) ) ).to.be.null;
} );
} );

describe( 'getSelectedBlocks', () => {
const paragraph1 = new Element( 'paragraph', null, '---' );
const item1 = new Element( 'listItem', null, '---' );
const item2 = new Element( 'listItem', null, '---' );
const item3 = new Element( 'listItem', null, '---' );
const paragraph2 = new Element( 'paragraph', null, '---' );

const root = new Element( '$root', null, [
paragraph1, item1, item2, item3, paragraph2
] );

const schema = new Schema();
schema.registerItem( 'paragraph', '$block' );
schema.registerItem( 'listItem', '$block' );

const selection = new Selection();

it( 'should return just one block if selection is over one block', () => {
selection.collapse( root, 2 );
selection.setFocus( root, 3 );

expect( getSelectedBlocks( selection, schema ) ).to.deep.equal( [ item2 ] );
} );

it( 'should return ancestor block if selection is collapsed and not before a block', () => {
selection.collapse( paragraph1, 2 );

expect( getSelectedBlocks( selection, schema ) ).to.deep.equal( [ paragraph1 ] );
} );

it( 'should return empty array for collapsed selection before a block, in a root', () => {
selection.collapse( root, 1 );

expect( getSelectedBlocks( selection, schema ) ).to.deep.equal( [] );
} );

it( 'should return all blocks "touched" by the selection if it spans over multiple blocks', () => {
selection.collapse( item1, 3 );
selection.setFocus( root, 4 );

expect( getSelectedBlocks( selection, schema ) ).to.deep.equal( [ item1, item2, item3 ] );
} );
} );

describe( 'getPositionBeforeBlock', () => {
const paragraph = new Element( 'paragraph', null, 'foo' );
const item = new Element( 'listItem', null, 'bar' );
const text = new Text( 'xyz' );

const root = new Element( '$root' );
root.appendChildren( [ paragraph, item, text ] );

const schema = new Schema();
schema.registerItem( 'paragraph', '$block' );
schema.registerItem( 'listItem', '$block' );

it( 'should return same position if position is already before a block', () => {
const position = Position.createBefore( paragraph );

expect( getPositionBeforeBlock( position, schema ).isEqual( position ) ).to.be.true;
} );

it( 'should return position before position parent if position is inside a block', () => {
const position = Position.createAt( item );

expect( getPositionBeforeBlock( position, schema ).isEqual( Position.createBefore( item ) ) ).to.be.true;
} );

it( 'should return null if position is not next to block and is not in a block other than root', () => {
const position = Position.createBefore( text );

expect( getPositionBeforeBlock( position, schema ) ).to.be.null;
} );
} );

0 comments on commit d04eab5

Please sign in to comment.