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

The component works slow on large documents #141

Closed
Ruud-cb opened this issue Oct 6, 2019 · 12 comments · Fixed by #346
Closed

The component works slow on large documents #141

Ruud-cb opened this issue Oct 6, 2019 · 12 comments · Fixed by #346
Assignees
Labels
bc:major squad:devops Issue to be handled by the Devops team. type:improvement
Milestone

Comments

@Ruud-cb
Copy link

Ruud-cb commented Oct 6, 2019

Similar issues:
ckeditor/ckeditor5-vue#42
And:
ckeditor/ckeditor4#449

But then for CKEditor 5 and angular. I am experiencing the same issues and wonder how to fix this? I guess this has been brought up before but I can't seem to find any related topics on this.

This can easily be re-produced by opening the default template for angular ckeditor:
https://stackblitz.com/edit/angular-ckeditor
And paste large amount of text in it. Go to https://www.lipsum.com/, put paragraphs on 150 and just start copy-pasting that text into the edtior. Around 5 or 6 times it becomes slower. The more lines you have, the more slower it becomes.

I have tried to remove the [data] property so that there is no two-way binding, but I guess, just like in the vue project, there is an issue with the on-change detection.

@Mgsy
Copy link
Member

Mgsy commented Oct 7, 2019

cc @ma2ciek

@ma2ciek
Copy link
Contributor

ma2ciek commented Oct 7, 2019

I've checked it and the reason, I think, is that the editor's change event triggers the this.ngZone.run( () => {} ) fn with the emitter called inside. However, there's no editor.getData() call or sth like this inside that function, so this is strictly connected with the performance of the ngZone.run() function.

I've checked and nothing breaks if I remove this callback. Although, according to docs this fn should be called to reenter the angular zone and to emit the event inside it. 🤷‍♂

@ma2ciek ma2ciek added pending:feedback This issue is blocked by necessary feedback. status:confirmed type:improvement and removed pending:feedback This issue is blocked by necessary feedback. labels Oct 7, 2019
@ma2ciek ma2ciek added this to the next milestone Oct 7, 2019
@Ruud-cb
Copy link
Author

Ruud-cb commented Oct 7, 2019

Thanks for the reply!
You are referring to this line?

What solution would you like to see as an PR? Introduce a new property to disable the on-change function all together and manually call editor.getData() when needed (by the component using this component)?

@ma2ciek
Copy link
Contributor

ma2ciek commented Oct 8, 2019

I guess, that this line can actually be removed, but I need to be sure that nothing breaks after this change.

@Ruud-cb
Copy link
Author

Ruud-cb commented Oct 9, 2019

Although the PR is not finished, I did do some checking to make sure this actually resolves the issue. It certainly does make a huge difference when working with large text files, a delay is still noticeable when blurred because that is the moment the ngModel now gets updated. Did the demo edits and the component edits in separate commits so you can cherry pick if needed.

@ma2ciek ma2ciek changed the title CKEditor very slow with large text The component works slow on large documents Oct 15, 2019
@ma2ciek ma2ciek modified the milestones: next, iteration 29 Nov 26, 2019
@ma2ciek ma2ciek modified the milestones: iteration 29, nice-to-have Feb 27, 2020
@Reinmar Reinmar added the squad:collaboration Issue to be handled by the Collaboration team. label May 25, 2020
@ma2ciek ma2ciek removed the squad:collaboration Issue to be handled by the Collaboration team. label Oct 12, 2020
@numabilis
Copy link

I encounter the same issue... I search more informations on this but nothing help me tio solve this issue.
Is there any solution ?

Thanks

@ma2ciek
Copy link
Contributor

ma2ciek commented Feb 2, 2021

Perhaps the only solution currently is to limit the data loaded in the editor and split it into chapters or some other smaller things.

@numabilis
Copy link

Thanks ma2ciek for reply.
I found a workaround. The problem comes from the editor.getData() call in the two-way data binding mechanism that is long. I replace ['nhModel)]="text" with [data]="text" for initializing the editor and when we want to save the data, I call editor.getData().

@lost-carrier
Copy link

Same issue here... (...except that without two-way binding it will be very very cumbersome here...)

@ma2ciek
Copy link
Contributor

ma2ciek commented Jun 25, 2021

This issue is still blocked by the upstream issue - improving the editor.getData() performance - ckeditor/ckeditor5#5812

@Reinmar Reinmar added squad:devops Issue to be handled by the Devops team. and removed squad:integrations labels Oct 28, 2021
@pomek pomek removed this from the nice-to-have milestone Feb 21, 2022
@pomek
Copy link
Member

pomek commented Nov 29, 2022

Could we remove two-way data binding in favor of synchronous changes using the onChange() callback?

@pomek pomek added the bc:major label Jan 25, 2023
@CKEditorBot CKEditorBot added the status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. label Jan 27, 2023
@CKEditorBot CKEditorBot added status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Jan 30, 2023
@pomek pomek closed this as completed in #346 Feb 6, 2023
pomek added a commit that referenced this issue Feb 6, 2023
Feature: Added an optional option called `disableTwoWayDataBinding` that allows disabling the two-way data binding. It increases performance when working with large documents. Closes #141.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Feb 6, 2023
@pomek
Copy link
Member

pomek commented Feb 6, 2023

While the upstream issue is still open, we wanted to improve the developer experience using CKEditor 5 and the Angular integration. I would like to announce we added a new option called disableTwoWayDataBinding. It increases performance when working with large documents. However, an integrator must synchronize data manually, e.g., using the ready() callback to store the editor instance and call editor.getData() whenever needed.

Changes: #346

We will release these changes this week. I will make sure to keep you posted.

@CKEditorBot CKEditorBot added this to the iteration 61 milestone Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment