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

Hotfix: unnecessary Discard Changes dialog for RTEs #17692

Merged
merged 10 commits into from
Dec 2, 2024

Conversation

madsrasmussen
Copy link
Contributor

@madsrasmussen madsrasmussen commented Dec 2, 2024

Fixes: #17670

The PR aims to clean up how we set/update the RTE property value so we don't get the "Discard Changes" dialog unless there has been user updates to the value.

How I went about this:

Allow the internal property _value to be undefined.
The property value can be undefined but the internal _value could only be the full value object with blocks. By aligning these it is easier to keep them in sync. The new code clears all internal the values and only sets the block data if we get a markup value.

Handles the Tip Tap empty <p></p> tag (ueberdosis/tiptap#154)
This issue wasn't directly linked to this issue but for us to be able to check for an empty tip tap editor I encountered this issue. Tip Tap has a property on their element to check if it's empty. It ignores the

and returns the value you would expect. I have added a public method on our element to proxy their value.

What to test:

  • Please ensure that you only get the "Discard Changes"-dialog if you have done any changes to an RTE.
  • Test both Tiny MCE and Tip Tap

@madsrasmussen madsrasmussen marked this pull request as ready for review December 2, 2024 13:15
@madsrasmussen madsrasmussen changed the title Hotfix: unnecessary RTE value changes Hotfix: unnecessary Discard Changes dialog for RTEs Dec 2, 2024
Copy link
Contributor

@iOvergaard iOvergaard left a comment

Choose a reason for hiding this comment

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

Tested it out and looks like it is working.

@NguyenThuyLan
Copy link
Contributor

Hi, I realize it also happens on Content pages that have BlockList/BlockGrid inside, is this PR fixed that issue also? @madsrasmussen

@engern
Copy link
Contributor

engern commented Dec 11, 2024

I got the same issue @NguyenThuyLan and created a new issue for it: #17784

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

Successfully merging this pull request may close these issues.

4 participants