-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Editable: Invoke onChange on every input #2758
Comments
Apologies in advance if I'm not following well. So would you propose passing changes all the time to the reducers and let them decide how to create undo levels? So TinyMCE will send the content on every keypress, and the undo reducer creates levels based on some criteria? That's what I had in mind for a while, as it would be nice to unify everything under one undo manager. We can then also disable the TinyMCE one. |
Yes, it would ideally result in a single undo history. Not clear yet how precisely this would work: If TinyMCE has an event for this, we could merely send this up through to the reducer as a signal (i.e. start recording changes into the same undo entry, split at specific intervals). Also not sure this would specifically be handled by reducers or as an effect (separate action). |
Ah, yeah, we could normally send the content, say |
Sure thing. Tagged you as assignee. |
closed by #4955 |
Related: #2750
Explored in #2418
Supersedes #1240
In an effort to keep post state in sync and dramatically simplify handling of content, we should invoke the Editable
onChange
callback on every input event which occurs within the editable field. Previously we had decided to trigger the callback only when focus leaves the field, resulting in unexpected behavior as described at #1240. The reason for this was performance concerns in serializing content on every keypress with the overhead of our value getter mapping. Since Editable content fields are used in a more granular fashion than normal for a TinyMCE instance (per paragraph, not per document), this is unlikely to pose as much of a concern, and may be helped with efforts of #2750.The current approach in #2418 implements a throttled callback. While this could remedy performance concerns, it also introduces a new set of concerns with canceling pending callbacks in response to future user actions (save, etc). See Automattic/wp-calypso#17819 as an example of where we have encountered this recently in the Calypso editor where we use this approach.
It will be important to consider how Undo / Redo operates with this approach, particularly with behaviors implemented as part of #1943 with the TinyMCE -> Redux undo handoff. For example, it would be non-ideal for Undo to pop one character at a time when used in succession.
The text was updated successfully, but these errors were encountered: