-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Classic block: save content bug fix #23408
Conversation
Size Change: +67 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
Looks good to me |
@@ -112,6 +121,20 @@ export default class ClassicEdit extends Component { | |||
bookmark = null; | |||
} ); | |||
|
|||
editor.on( | |||
'Paste Change input Undo Redo', | |||
debounce( () => { |
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.
Why are you using debounce
here? Just not to fire more times when these events are triggered?
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.
We figured that this could be a very noisy call as someone is typing, making lots of changes, etc., and we don't need every tiny change detected to go through this cycle.
We didn't actually start to see any visible performance degradation until you got upwards of 50,000 words in a post, and adding a debounce made it performant again even with a massive post.
Although people are never likely to have huge posts like that we didn't think it would hurt to add this in anyway, and the debounce time could probably even be raised. Open to feedback on whether we keep this, increase time, etc.
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.
we didn't think it would hurt to add this in anyway
I believe that too and I also think that 250
is a fair debounce time.
It's great that you have made some tests checking performance! 👍
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.
Good fix @glendaviesnz ! Thank you!
…eloading editor content
f5d45f5
to
edb8ec1
Compare
Thanks @ntsekouras, did you have any thoughts on this one #23376? |
Description
Fixes: #22935
Fixes: #15577
Uses the TinyMCE change events to push changed content back out to gutenberg instead of just relying on the onblur event
How has this been tested?
Manually
Screenshots
Before:
After:
Types of changes
Checklist: