-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Group changes into one batch #14060
Group changes into one batch #14060
Conversation
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.
Functionally it's ok (solution propagated from table cell properties UI), but let's improve its architecture a bit. Tests will need to be updated probably.
it( 'should use parent batch for storing undo steps', () => { | ||
it( 'should use provided batch', () => { | ||
setData( model, '<paragraph>a[bc<$text font="foo">fo]obar</$text>xyz</paragraph>' ); | ||
const batch = model.createBatch(); | ||
const spy = sinon.spy( model, 'enqueueChange' ); | ||
|
||
model.change( writer => { | ||
expect( writer.batch.operations.length ).to.equal( 0 ); | ||
command.execute( { value: 'foo' } ); | ||
expect( writer.batch.operations.length ).to.equal( 1 ); | ||
} ); | ||
|
||
expect( getData( model ) ).to.equal( '<paragraph>a[<$text font="foo">bcfo]obar</$text>xyz</paragraph>' ); |
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 think having both tests (the old one and new one) makes sense - depending on whether you've passed the batch as a parameter.
@@ -20,7 +21,7 @@ describe( 'Integration test Font', () => { | |||
|
|||
return ClassicTestEditor | |||
.create( element, { | |||
plugins: [ Font, ArticlePluginSet ], | |||
plugins: [ Font, ArticlePluginSet, Undo ], |
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.
Undo
is included in Essentials
which are included in ArticlePluginSet
.
Suggested merge commit message (convention)
Feature: Group changes into one batch. Closes #3162.
Additional information
For example – encountered issues, assumptions you had to make, other affected tickets, etc.