-
Notifications
You must be signed in to change notification settings - Fork 15
T/ckeditor5/1636: MultiCommand #179
Changes from all commits
fdb0c86
5f6039e
c7a9f7b
db2d11f
e83aaac
7833999
a53d8c5
efc9fa0
2c35198
c93c280
618b31a
900aeaf
79a6aa4
569eba3
b1e2f8d
7e7b4c7
dd39d16
94f0edb
ccb9c2c
e970541
d531bb9
2ef389a
7340ad2
dafe54d
e1661d0
f2296c8
401ac7a
75b3176
3849a8c
f23cd70
d8517ce
0b23e78
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
/** | ||
* @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. | ||
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license | ||
*/ | ||
|
||
import Command from './command'; | ||
|
||
/** | ||
* @module core/multicommand | ||
*/ | ||
|
||
/** | ||
* A CKEditor command that aggregates other commands. | ||
* | ||
* This command is used to proxy multiple commands. The multi-command is enabled when | ||
* at least one of its registered child commands is enabled. | ||
* When executing a multi-command the first command that is enabled will be executed. | ||
* | ||
* const multiCommand = new MultiCommand( editor ); | ||
* | ||
* const commandFoo = new Command( editor ); | ||
* const commandBar = new Command( editor ); | ||
* | ||
* // Register child commands. | ||
* multiCommand.registerChildCommand( commandFoo ); | ||
* multiCommand.registerChildCommand( commandBar ); | ||
* | ||
* // Enable one of the commands. | ||
* commandBar.isEnabled = true; | ||
* | ||
* multiCommand.execute(); // Will execute commandBar. | ||
* | ||
* @extends module:core/command~Command | ||
*/ | ||
export default class MultiCommand extends Command { | ||
/** | ||
* @inheritDoc | ||
*/ | ||
constructor( editor ) { | ||
super( editor ); | ||
|
||
/** | ||
* Registered child commands. | ||
* | ||
* @type {Array.<module:core/command~Command>} | ||
* @private | ||
*/ | ||
this._childCommands = []; | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
refresh() { | ||
// Override base command refresh(): the command's state is changed when one of child commands changes states. | ||
} | ||
|
||
/** | ||
* Executes the first of it registered child commands. | ||
*/ | ||
execute( ...args ) { | ||
const command = this._getFirstEnabledCommand(); | ||
|
||
command.execute( args ); | ||
} | ||
|
||
/** | ||
* Registers a child command. | ||
* | ||
* @param {module:core/command~Command} command | ||
*/ | ||
registerChildCommand( command ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although, "register" reads quite well in the docs. So I don't have a strong opinion on that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "register" implies something more than just adding it (like adding to a collection). So if it also reads well in the docs I'll leave it as is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. We actually start watching those commands. |
||
this._childCommands.push( command ); | ||
|
||
// Change multi command enabled state when one of registered commands changes state. | ||
command.on( 'change:isEnabled', () => this._checkEnabled() ); | ||
|
||
this._checkEnabled(); | ||
} | ||
|
||
/** | ||
* Checks if any of child commands is enabled. | ||
* | ||
* @private | ||
*/ | ||
_checkEnabled() { | ||
this.isEnabled = !!this._getFirstEnabledCommand(); | ||
} | ||
|
||
/** | ||
* Returns a first enabled command or undefined if none of them is enabled. | ||
* | ||
* @returns {module:core/command~Command|undefined} | ||
* @private | ||
*/ | ||
_getFirstEnabledCommand() { | ||
return this._childCommands.find( command => command.isEnabled ); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
/** | ||
* @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. | ||
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license | ||
*/ | ||
|
||
import MultiCommand from '../src/multicommand'; | ||
import ModelTestEditor from './_utils/modeltesteditor'; | ||
import Command from '../src/command'; | ||
|
||
describe( 'MultiCommand', () => { | ||
let editor, multiCommand; | ||
|
||
beforeEach( () => { | ||
return ModelTestEditor | ||
.create() | ||
.then( newEditor => { | ||
editor = newEditor; | ||
multiCommand = new MultiCommand( editor ); | ||
} ); | ||
} ); | ||
|
||
afterEach( () => { | ||
multiCommand.destroy(); | ||
|
||
return editor.destroy(); | ||
} ); | ||
|
||
describe( 'isEnabled', () => { | ||
it( 'is always falsy when no child commands are registered', () => { | ||
expect( multiCommand.isEnabled ).to.false; | ||
|
||
multiCommand.refresh(); | ||
|
||
expect( multiCommand.isEnabled ).to.false; | ||
} ); | ||
|
||
it( 'is set to true if one of registered commands is true', () => { | ||
expect( multiCommand.isEnabled ).to.false; | ||
|
||
const commandA = new Command( editor ); | ||
const commandB = new Command( editor ); | ||
|
||
multiCommand.registerChildCommand( commandA ); | ||
multiCommand.registerChildCommand( commandB ); | ||
|
||
expect( multiCommand.isEnabled ).to.false; | ||
|
||
commandA.isEnabled = true; | ||
|
||
expect( multiCommand.isEnabled ).to.be.true; | ||
|
||
commandA.isEnabled = false; | ||
|
||
expect( multiCommand.isEnabled ).to.be.false; | ||
|
||
commandB.isEnabled = true; | ||
|
||
expect( multiCommand.isEnabled ).to.be.true; | ||
} ); | ||
} ); | ||
|
||
describe( 'execute()', () => { | ||
it( 'does not call any command if all are disabled', () => { | ||
const commandA = new Command( editor ); | ||
const commandB = new Command( editor ); | ||
|
||
multiCommand.registerChildCommand( commandA ); | ||
multiCommand.registerChildCommand( commandB ); | ||
|
||
const spyA = sinon.spy( commandA, 'execute' ); | ||
const spyB = sinon.spy( commandB, 'execute' ); | ||
|
||
multiCommand.execute(); | ||
|
||
sinon.assert.notCalled( spyA ); | ||
sinon.assert.notCalled( spyB ); | ||
} ); | ||
|
||
it( 'executes enabled command', () => { | ||
const commandA = new Command( editor ); | ||
const commandB = new Command( editor ); | ||
const commandC = new Command( editor ); | ||
|
||
multiCommand.registerChildCommand( commandA ); | ||
multiCommand.registerChildCommand( commandB ); | ||
multiCommand.registerChildCommand( commandC ); | ||
|
||
const spyA = sinon.spy( commandA, 'execute' ); | ||
const spyB = sinon.spy( commandB, 'execute' ); | ||
const spyC = sinon.spy( commandC, 'execute' ); | ||
|
||
commandC.isEnabled = true; | ||
|
||
multiCommand.execute(); | ||
|
||
sinon.assert.notCalled( spyA ); | ||
sinon.assert.notCalled( spyB ); | ||
sinon.assert.calledOnce( spyC ); | ||
} ); | ||
|
||
it( 'executes first registered command if many are enabled', () => { | ||
const commandA = new Command( editor ); | ||
const commandB = new Command( editor ); | ||
const commandC = new Command( editor ); | ||
|
||
multiCommand.registerChildCommand( commandA ); | ||
multiCommand.registerChildCommand( commandB ); | ||
multiCommand.registerChildCommand( commandC ); | ||
|
||
const spyA = sinon.spy( commandA, 'execute' ); | ||
const spyB = sinon.spy( commandB, 'execute' ); | ||
const spyC = sinon.spy( commandC, 'execute' ); | ||
|
||
commandC.isEnabled = true; | ||
commandB.isEnabled = true; | ||
|
||
multiCommand.execute(); | ||
|
||
sinon.assert.notCalled( spyA ); | ||
sinon.assert.calledOnce( spyB ); | ||
sinon.assert.notCalled( spyC ); | ||
} ); | ||
} ); | ||
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I became unsure about this. I'll report a followup, though.