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

More detailed commandStack events #486

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
31 changes: 26 additions & 5 deletions lib/command/CommandStack.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
uniqueBy,
isArray
isArray,
assign,
} from 'min-dash';


Expand Down Expand Up @@ -107,10 +108,16 @@ export default function CommandStack(eventBus, injector) {
* Current active commandStack execution
*
* @type {Object}
* @property {Array} actions
* @property {Array} dirty
* @property {boolean|undefined} atomic
* @property {string|undefined} method Is this an execution, undo, or redo?
* @property {Array} executed List of actions who finished executing
*/
this._currentExecution = {
actions: [],
dirty: []
dirty: [],
executed: [],
};


Expand Down Expand Up @@ -141,6 +148,8 @@ CommandStack.prototype.execute = function(command, context) {
throw new Error('command required');
}

var execution = this._currentExecution;
execution.method || (execution.method = 'execute');
var action = { command: command, context: context };

this._pushAction(action);
Expand Down Expand Up @@ -201,7 +210,8 @@ CommandStack.prototype.clear = function(emit) {
this._stackIdx = -1;

if (emit !== false) {
this._fire('changed');
this._fire('changed', { method: 'clear', actions: [] });
delete this._currentExecution.method;
}
};

Expand All @@ -211,8 +221,10 @@ CommandStack.prototype.clear = function(emit) {
*/
CommandStack.prototype.undo = function() {
var action = this._getUndoAction(),
execution = this._currentExecution,
next;

execution.method = 'undo';
if (action) {
this._pushAction(action);

Expand All @@ -237,11 +249,14 @@ CommandStack.prototype.undo = function() {
*/
CommandStack.prototype.redo = function() {
var action = this._getRedoAction(),
execution = this._currentExecution,
next;

if (action) {
this._pushAction(action);

execution.method = 'redo';

while (action) {
this._internalExecute(action, true);
next = this._getRedoAction();
Expand Down Expand Up @@ -446,17 +461,21 @@ CommandStack.prototype._pushAction = function(action) {

CommandStack.prototype._popAction = function() {
var execution = this._currentExecution,
method = execution.method,
actions = execution.actions,
dirty = execution.dirty;

actions.pop();

if (!actions.length) {
this._eventBus.fire('elements.changed', { elements: uniqueBy('id', dirty.reverse()) });
var context = { elements: uniqueBy('id', dirty.reverse()) };
this._eventBus.fire('elements.changed', assign({}, context));

dirty.length = 0;

this._fire('changed');
this._fire('changed', assign({ method: method, actions: execution.executed.concat() }, { context: context }));
delete execution.method;
execution.executed.length = 0;
}
};

Expand All @@ -479,12 +498,14 @@ CommandStack.prototype._executedAction = function(action, redo) {

if (!redo) {
this._stack.splice(stackIdx, this._stack.length, action);
this._currentExecution.executed.push(action);
}
};


CommandStack.prototype._revertedAction = function(action) {
this._stackIdx--;
this._currentExecution.executed.push(action);
};


Expand Down
96 changes: 96 additions & 0 deletions test/spec/command/CommandStackSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {


import cmdModule from 'lib/command';
import { expect } from 'chai';

// example commands

Expand Down Expand Up @@ -876,4 +877,99 @@ describe('command/CommandStack', function() {

});

describe('change-event', function() {

var testSetup = function(eventBus, commandStack) {
var eventData = {};

commandStack.registerHandler('complex-command', ComplexCommand);
commandStack.registerHandler('pre-command', PreCommand);
commandStack.registerHandler('post-command', PostCommand);
eventBus.on('commandStack.changed', function(event) {
eventData.changeEvent = event.method;
eventData.actions = event.actions.map(function(action) {return action.command;});
});

return eventData;
};

describe('should indicate details about a change', function() {

it('with method <execute>', inject(function(eventBus, commandStack) {

// given
var eventData = testSetup(eventBus, commandStack);

// when
commandStack.execute('complex-command', { element: { trace: [] } });

// then
var changeEvent = eventData.changeEvent,
actions = eventData.actions;

expect(changeEvent).to.equal('execute');
expect(actions).to.eql(['pre-command', 'complex-command', 'post-command']);

}));

it('with method <undo>', inject(function(eventBus, commandStack) {

// given
var eventData = testSetup(eventBus, commandStack);

// when
commandStack.execute('complex-command', { element: { trace: [] } });
commandStack.undo();

// then
var changeEvent = eventData.changeEvent,
actions = eventData.actions;

expect(changeEvent).to.equal('undo');
expect(actions).to.eql(['post-command', 'complex-command', 'pre-command']);

}));

it.skip('with method <redo>', inject(function(eventBus, commandStack) {

// given
var eventData = testSetup(eventBus, commandStack);

// when
commandStack.execute('complex-command', { element: { trace: [] } });
commandStack.undo();
commandStack.redo();

// then
var changeEvent = eventData.changeEvent,
actions = eventData.actions;

expect(changeEvent).to.equal('redo');

// Need to figure out a way to indicate the executed actions
expect(actions).to.eql(['pre-command', 'complex-command', 'post-command']);
Comment on lines +949 to +950
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one has an issue though, that I discovered when making tests (I skipped the relevant test because it fails). The redo action doesn't indicate the executed actions. I can't figure out a graceful way to do this, so I would appreciate a nudge in the right direction!


}));

it('with method <clear>', inject(function(eventBus, commandStack) {

// given
var eventData = testSetup(eventBus, commandStack);

// when
commandStack.clear();

// then
var changeEvent = eventData.changeEvent,
actions = eventData.actions;

expect(changeEvent).to.equal('clear');
expect(actions).to.eql([]);

}));

});

});

});