-
Notifications
You must be signed in to change notification settings - Fork 111
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
Introduce the dataChangeEventDisabled
property
#143
Conversation
Hi, @Ruud-cb! Can you tell us more about what problem does this PR actually solve? Some use–case or maybe a demo? |
Hi @oleq, follow the reproduction steps in the issue, use the demo-form component. You will notice extreme lag after inserting a lot of text. Then change |
Thanks @Ruud-cb for the PR.
It's something we can do nothing about, the |
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.
Some necessary changes are described in the above post.
What do you mean? you can get the data with |
I understand that
Sorry for the confusion, It would be favorable if you can adjust the unit tests as it will take you significant less time. |
…ixed the disability mechanism.
dataChangeEventDisabled
property that will enable performance boost for large documents
dataChangeEventDisabled
property that will enable performance boost for large documentsdataChangeEventDisabled
property
After a while I think about this, such change can be dangerous for big documents as they might be not saved for a long time, as now the saving is done only during the |
True, although it is deliberately set by the developer, so one should know. Currently the problem is that no text can be typed in big documents smoothly. And to solve the 'losing data' problem, I would suggest developers to make use of |
But I think that using the |
Alright, that's also possible. Only argument against that is that it might occur the user starts typing again exactly at the moment the debounce is fired off. Maybe both, whatever you want :). |
Hi @ma2ciek, the build is failing on this due to unit test coverage being under 100%, so your changes have not been merged, the large document problem was not solved. |
I think that this change is no longer required as the ckeditor5-angular/src/ckeditor/ckeditor.component.ts Lines 383 to 393 in bdfabf4
|
Suggested merge commit message (convention)
Feature: Introduced the
dataChangeEventDisabled
property that will enable performance boost for large documents. Closes #141.Additional information
For example – encountered issues, assumptions you had to make, other affected tickets, etc.