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

Big document block typing #82

Closed
dkrahn opened this issue Apr 1, 2019 · 4 comments
Closed

Big document block typing #82

dkrahn opened this issue Apr 1, 2019 · 4 comments
Assignees
Milestone

Comments

@dkrahn
Copy link
Contributor

dkrahn commented Apr 1, 2019

For performance issues, specially in big documents, I think the code below should be modified:

modelDocument.on( 'change:data', ( evt: CKEditor5.EventInfo<'change:data'> ) => {
const data = editor.getData();
this.ngZone.run( () => {
if ( this.cvaOnChange ) {
this.cvaOnChange( data );
}
this.change.emit( { event: evt, editor } );
} );
} );

I would suggest the following:

modelDocument.on( 'change:data', ( evt: CKEditor5.EventInfo<'change:data'> ) => {
 	this.ngZone.run( () => {
 		if ( this.cvaOnChange ) {
 			const data = editor.getData(); 
 			this.cvaOnChange( data );
 		}
  
 		this.change.emit( { event: evt, editor } );
 	} );
 } );

Getting the data(getData()) only when it is necessary.

@dkrahn
Copy link
Contributor Author

dkrahn commented Apr 2, 2019

Hi @ma2ciek, I found this performance issue and did the PR #83 to fix it. Did I do it correctly?

@ma2ciek
Copy link
Contributor

ma2ciek commented Apr 2, 2019

H1 @dkrahn

Thanks for the detailed issue and PR! This change makes sense indeed, it's a pity that I overlooked it. I'll add one test to your PR and will merge it.

@ma2ciek ma2ciek self-assigned this Apr 2, 2019
@ma2ciek ma2ciek added this to the iteration 23 milestone Apr 2, 2019
ma2ciek added a commit that referenced this issue Apr 2, 2019
Other: Improved performance by processing data only when effectively needed. Closes #82. Closes #83.
@ma2ciek
Copy link
Contributor

ma2ciek commented Apr 2, 2019

I'll try to release it soon.

@dkrahn
Copy link
Contributor Author

dkrahn commented Apr 2, 2019

Thanks for the great work you all are doing!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants