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

BUGFIX: 3839 ckeditor only save changes if (really) dirty #3846

Merged
merged 3 commits into from
Sep 9, 2024

Conversation

mhsdesign
Copy link
Member

Resolves: #3839
Alternative fix for: #3832

What I did

The onChange callback is since also fired after defocusing an editor (#3751). A followup introduced a check to compare the contents if the change is indeed new (#3833).

The comparison is fragile and not always in the users intention. (#3839)

This pr reverts the check and introduces a more stable isDirty handling.

How I did it

While i researched the isDirty tracking i stumbled upon ckeditor/ckeditor5#996 (comment), naively i did a little tunnel through space and had to frizzle a lot with state handling.
But when trying to solve the problem of deduplicating the debounce after the focus was lost i stumbled upon .flush which is EXACTLY what we need here. No further logic needed. And it guarantees a stable, simple to read and best possible behaviour.

How to verify it

…is unchanged"

This reverts commit 180906d.

The change introduced with pr #3833 is not needed when onChange is only triggerd on real actual changes.
... and thus we can avoid the isDirty state and second change:data callback

> a flush method to immediately invoke them
Copy link
Member

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, works great and reduces save operations even further. Love it.

@mhsdesign mhsdesign merged commit 8db53a3 into 8.3 Sep 9, 2024
10 checks passed
@mhsdesign mhsdesign deleted the bugfix/3839-ckeditor-only-save-changes-if-dirty branch September 9, 2024 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.3 Bug Label to mark the change as bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants