Skip to content

Commit

Permalink
Merge pull request #7650 from ckeditor/i/4060
Browse files Browse the repository at this point in the history
Fix (undo): Undo/redo stacks should be cleared on `DataController#set()`. Closes #4060.
Feature (engine): The method `DataController#set()` is now decorated so plugins can listen to `editor.setData()` calls.

MAJOR BREAKING CHANGE: The method `editor.setData()` clears the undo and redo stacks.
  • Loading branch information
jodator authored Jul 22, 2020
2 parents bbd0d6d + ba6565b commit 4a12d38
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 0 deletions.
11 changes: 11 additions & 0 deletions packages/ckeditor5-engine/src/controller/datacontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ export default class DataController {
this.upcastDispatcher.on( 'documentFragment', convertToModelFragment(), { priority: 'lowest' } );

this.decorate( 'init' );
this.decorate( 'set' );

// Fire `ready` event when initialisation has completed. Such low level listener gives possibility
// to plug into initialisation pipeline without interrupting the initialisation flow.
Expand Down Expand Up @@ -317,6 +318,7 @@ export default class DataController {
*
* dataController.set( { main: '<p>Foo</p>', title: '<h1>Bar</h1>' } ); // Sets data on the `main` and `title` roots.
*
* @fires set
* @param {String|Object.<String,String>} data Input data as a string or an object containing `rootName` - `data`
* pairs to set data on multiple roots at once.
*/
Expand Down Expand Up @@ -452,6 +454,15 @@ export default class DataController {
*
* @event init
*/

/**
* Event fired after {@link #set set() method} has been run.
*
* The `set` event is fired by decorated {@link #set} method.
* See {@link module:utils/observablemixin~ObservableMixin#decorate} for more information and samples.
*
* @event set
*/
}

mix( DataController, ObservableMixin );
Expand Down
10 changes: 10 additions & 0 deletions packages/ckeditor5-engine/tests/controller/datacontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,16 @@ describe( 'DataController', () => {
} );

describe( 'set()', () => {
it( 'should be decorated', () => {
const spy = sinon.spy();

data.on( 'set', spy );

data.set( 'foo bar' );

sinon.assert.calledWithExactly( spy, sinon.match.any, [ 'foo bar' ] );
} );

it( 'should set data to default main root', () => {
schema.extend( '$text', { allowIn: '$root' } );
data.set( 'foo' );
Expand Down
2 changes: 2 additions & 0 deletions packages/ckeditor5-undo/src/basecommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ export default class BaseCommand extends Command {

// Refresh state, so the command is inactive right after initialization.
this.refresh();

this.listenTo( editor.data, 'set', () => this.clearStack() );
}

/**
Expand Down
8 changes: 8 additions & 0 deletions packages/ckeditor5-undo/tests/redocommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,5 +288,13 @@ describe( 'RedoCommand', () => {
expect( undoSpy.firstCall.args[ 1 ] ).to.equal( redoingBatch );
} );
} );

it( 'should clear stack on DataController set()', () => {
const spy = sinon.stub( redo, 'clearStack' );

editor.setData( 'foo' );

sinon.assert.called( spy );
} );
} );
} );
8 changes: 8 additions & 0 deletions packages/ckeditor5-undo/tests/undocommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,5 +347,13 @@ describe( 'UndoCommand', () => {
expect( getCaseText( root ) ).to.equal( 'adbcef' );
expect( editor.model.document.selection.getFirstRange().isEqual( r( 1, 4 ) ) ).to.be.true;
} );

it( 'should clear stack on DataController set()', () => {
const spy = sinon.stub( undo, 'clearStack' );

editor.setData( 'foo' );

sinon.assert.called( spy );
} );
} );
} );

0 comments on commit 4a12d38

Please sign in to comment.