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

Commit

Permalink
Other: Aligned the implementation to the new Command API (see https:/…
Browse files Browse the repository at this point in the history 
…/github.com/ckeditor/ckeditor5-core/issues/88).

BREAKING CHANGES: The command API has been changed.
  • Loading branch information
@szymonkups
szymonkups committed Jun 13, 2017
2 parents c3fa9f0 + 897e390 commit 9a7d596
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 65 deletions.
10 changes: 5 additions & 5 deletions src/converters.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,18 +715,18 @@ function _fixItemsType( changePosition, fixPrevious, document, batch ) {
* <listItem type="bulleted" indent=2>Y/listItem>
* <listItem type="bulleted" indent=2>C</listItem>
*
* @param {module:engine/model/document~Document} document Document to observe.
* @returns {Function} Callback to be attached to {@link module:engine/model/document~Document#event:change document change event}.
* @param {module:utils/eventinfo~EventInfo} evt Event info object.
* @param {Array} args Arguments of {@link module:engine/controller/datacontroller~DataController#insertContent}.
*/
export function modelIndentPasteFixer( evt, data ) {
export function modelIndentPasteFixer( evt, [ content, selection ] ) {
// Check whether inserted content starts from a `listItem`. If it does not, it means that there are some other
// elements before it and there is no need to fix indents, because even if we insert that content into a list,
// that list will be broken.
let item = data.content.getChild( 0 );
let item = content.getChild( 0 );

if ( item.is( 'listItem' ) ) {
// Get a reference list item. Inserted list items will be fixed according to that item.
const pos = data.selection.getFirstPosition();
const pos = selection.getFirstPosition();
let refItem = null;

if ( pos.parent.is( 'listItem' ) ) {
Expand Down
24 changes: 16 additions & 8 deletions src/indentcommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
* @module list/indentcommand
*/

import Command from '@ckeditor/ckeditor5-core/src/command/command';
import Command from '@ckeditor/ckeditor5-core/src/command';
import first from '@ckeditor/ckeditor5-utils/src/first';

/**
* The list indent command. It is used by the {@link module:list/list~List list feature}.
*
* @extends core.command.Command
* @extends module:core/command~Command
*/
export default class IndentCommand extends Command {
/**
Expand All @@ -34,16 +34,21 @@ export default class IndentCommand extends Command {
* @member {Number}
*/
this._indentBy = indentDirection == 'forward' ? 1 : -1;

this.listenTo( editor.document, 'changesDone', () => {
this.refreshState();
} );
}

/**
* @inheritDoc
*/
_doExecute() {
refresh() {
this.isEnabled = this._checkEnabled();
}

/**
* Indents or outdents (depends on {@link #constructor}'s `indentDirection` parameter) selected list items.
*
* @fires execute
*/
execute() {
const doc = this.editor.document;
const batch = doc.batch();
let itemsToChange = Array.from( doc.selection.getSelectedBlocks() );
Expand Down Expand Up @@ -99,7 +104,10 @@ export default class IndentCommand extends Command {
}

/**
* @inheritDoc
* Checks whether the command can be enabled in the current context.
*
* @private
* @returns {Boolean} Whether the command should be enabled.
*/
_checkEnabled() {
// Check whether any of position's ancestor is a list item.
Expand Down
42 changes: 25 additions & 17 deletions src/listcommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
* @module list/listcommand
*/

import Command from '@ckeditor/ckeditor5-core/src/command/command';
import Command from '@ckeditor/ckeditor5-core/src/command';
import Position from '@ckeditor/ckeditor5-engine/src/model/position';
import first from '@ckeditor/ckeditor5-utils/src/first';

/**
* The list command. It is used by the {@link module:list/list~List list feature}.
*
* @extends module:core/command/command~Command
* @extends module:core/command~Command
*/
export default class ListCommand extends Command {
/**
Expand All @@ -38,25 +38,17 @@ export default class ListCommand extends Command {
* Flag indicating whether the command is active, which means that selection starts in a list of the same type.
*
* @observable
* @readonly
* @member {Boolean} #value
*/
this.set( 'value', false );

// Listen on selection and document changes and set the current command's value.
this.listenTo( editor.document, 'changesDone', () => {
this.refreshValue();
this.refreshState();
} );
}

/**
* Sets command's value based on the document selection.
* @inheritDoc
*/
refreshValue() {
// Check whether closest `listItem` ancestor of the position has a correct type.
const listItem = first( this.editor.document.selection.getSelectedBlocks() );

this.value = listItem && listItem.is( 'listItem' ) && listItem.getAttribute( 'type' ) == this.type;
refresh() {
this.value = this._getValue();
this.isEnabled = this._checkEnabled();
}

/**
Expand All @@ -67,7 +59,7 @@ export default class ListCommand extends Command {
* @param {module:engine/model/batch~Batch} [options.batch] Batch to collect all the change steps.
* New batch will be created if this option is not set.
*/
_doExecute( options = {} ) {
execute( options = {} ) {
const document = this.editor.document;
const blocks = Array.from( document.selection.getSelectedBlocks() );

Expand Down Expand Up @@ -226,7 +218,23 @@ export default class ListCommand extends Command {
}

/**
* @inheritDoc
* Checks the command's {@link #value}.
*
* @private
* @returns {Boolean} The current value.
*/
_getValue() {
// Check whether closest `listItem` ancestor of the position has a correct type.
const listItem = first( this.editor.document.selection.getSelectedBlocks() );

return !!listItem && listItem.is( 'listItem' ) && listItem.getAttribute( 'type' ) == this.type;
}

/**
* Checks whether the command can be enabled in the current context.
*
* @private
* @returns {Boolean} Whether the command should be enabled.
*/
_checkEnabled() {
// If command value is true it means that we are in list item, so the command should be enabled.
Expand Down
8 changes: 4 additions & 4 deletions src/listengine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ export default class ListEngine extends Plugin {
data.on( 'insertContent', modelIndentPasteFixer, { priority: 'high' } );

// Register commands for numbered and bulleted list.
editor.commands.set( 'numberedList', new ListCommand( editor, 'numbered' ) );
editor.commands.set( 'bulletedList', new ListCommand( editor, 'bulleted' ) );
editor.commands.add( 'numberedList', new ListCommand( editor, 'numbered' ) );
editor.commands.add( 'bulletedList', new ListCommand( editor, 'bulleted' ) );

// Register commands for indenting.
editor.commands.set( 'indentList', new IndentCommand( editor, 'forward' ) );
editor.commands.set( 'outdentList', new IndentCommand( editor, 'backward' ) );
editor.commands.add( 'indentList', new IndentCommand( editor, 'forward' ) );
editor.commands.add( 'outdentList', new IndentCommand( editor, 'backward' ) );
}
}

Expand Down
32 changes: 16 additions & 16 deletions tests/indentcommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,13 @@ describe( 'IndentCommand', () => {
} );
} );

describe( '_doExecute', () => {
describe( 'execute()', () => {
it( 'should increment indent attribute by 1', () => {
doc.enqueueChanges( () => {
doc.selection.collapse( root.getChild( 5 ) );
} );

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand All @@ -148,7 +148,7 @@ describe( 'IndentCommand', () => {
doc.selection.collapse( root.getChild( 1 ) );
} );

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand All @@ -169,7 +169,7 @@ describe( 'IndentCommand', () => {
) ] );
} );

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand All @@ -191,7 +191,7 @@ describe( 'IndentCommand', () => {
'<listItem indent="1" type="bulleted">[]d</listItem>'
);

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand All @@ -213,7 +213,7 @@ describe( 'IndentCommand', () => {
'<listItem indent="0" type="bulleted">f</listItem>'
);

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand Down Expand Up @@ -266,13 +266,13 @@ describe( 'IndentCommand', () => {
} );
} );

describe( '_doExecute', () => {
describe( 'execute()', () => {
it( 'should decrement indent attribute by 1 (if it is bigger than 0)', () => {
doc.enqueueChanges( () => {
doc.selection.collapse( root.getChild( 5 ) );
} );

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand All @@ -290,7 +290,7 @@ describe( 'IndentCommand', () => {
doc.selection.collapse( root.getChild( 0 ) );
} );

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<paragraph indent="0" type="bulleted">a</paragraph>' +
Expand All @@ -308,7 +308,7 @@ describe( 'IndentCommand', () => {
doc.selection.collapse( root.getChild( 1 ) );
} );

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand All @@ -329,7 +329,7 @@ describe( 'IndentCommand', () => {
) ] );
} );

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand All @@ -350,7 +350,7 @@ describe( 'IndentCommand', () => {
'<listItem indent="2" type="numbered">[]c</listItem>'
);

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand All @@ -369,7 +369,7 @@ describe( 'IndentCommand', () => {
'<listItem indent="1" type="numbered">d</listItem>'
);

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand All @@ -389,7 +389,7 @@ describe( 'IndentCommand', () => {
'<listItem indent="1" type="numbered">d]</listItem>'
);

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand All @@ -407,7 +407,7 @@ describe( 'IndentCommand', () => {
'<listItem indent="0" type="numbered">c</listItem>'
);

command._doExecute();
command.execute();

// Another possible behaviour would be that "b" list item becomes first list item of a top level
// numbered list (so it does not change it's type) but it seems less correct from UX standpoint.
Expand All @@ -430,7 +430,7 @@ describe( 'IndentCommand', () => {
'<listItem indent="2" type="bulleted">e</listItem>'
);

command._doExecute();
command.execute();

expect( getData( doc, { withoutSelection: true } ) ).to.equal(
'<listItem indent="0" type="bulleted">a</listItem>' +
Expand Down
Loading

0 comments on commit 9a7d596

Please sign in to comment.