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

Store caret position in history and use this info on Undo/Redo #884

Closed
wants to merge 5 commits into from

Conversation

daniloercoli
Copy link
Contributor

Opened this PR to discuss a couple of problems I found with RichText text based blocks and history.

Trying to fix #303 - Cursor positioning on Rich Text-based blocks after undoing is pos+1 - found out it is a problem with the caret position not kept retained in history.
We're only storing the content in history, then when the user taps on the undo/redo buttons the text is changed but the caret stays in the same position of before if possible. (it's often moved to the end of the block in case undo removes some text and caret-position > newText.length).

I then started working on adding the position of the caret, together with the content, in the history stack, and made it "kind of working", even if it feels a bit of an hack - for ref the GB side branch is available here.

While trying to fix #303 I've encountered lot of problems with the history, and decided to switch back to develop to check if I can replicate them there, since was having hard time on figuring out if it was my code the culprit of the bad behavior.
Below is a list of problems I see with history on develop:

  • History in not empty on a clean start of the editor!? As soon as you tap in a block the Back button will be available, and if you tap on it "an action" is actually executed, the "redo" button slightly visible for a fraction of second, and the the "undo" is available again. See the GIF I attached below

  • If you add a new empty block it's hard to get rid of hit by tapping on Undo. For the same reason of above, it seems two actions are executed in a sequence in a very small time.
    If you want to get rid of the block you need to tap "undo" many times, very fast.

Investigated a bit, and it seems to me that the problem is due to the onSelectionChange called at init and every time the user adds new text - onSelectionChange is called right after the onChange callback. So basically we're updating the JS side lot of times 🤔
Not quite sure why we're calling this.props.onChange( this.lastContent ); when the selection does change. cc @marecar3

history mess up

I think it's better to fix the problems with the history before starting storing caret position in it. Otherwise we're messing things up a little more.
Thoughts?

@Tug
Copy link
Contributor

Tug commented Apr 17, 2019

Since this is missing on the web as well I think this is to be discussed with the Gutenberg team. We probably also want cursor history there so it would be useful (maybe?) to have that information as part of the store.
cc @gziolo @ellatrix Do you know if you have any plans for this?

@ellatrix
Copy link
Collaborator

ellatrix commented Apr 17, 2019

This is being worked on in WordPress/gutenberg#14640 and follow-up PRs.

@daniloercoli
Copy link
Contributor Author

So let's wait the GB side PRs getting merged, and then we can re-iterate on this on mobile.

In the meanwhile I will open new issues for the problems we've with history on develop.

@daniloercoli
Copy link
Contributor Author

Reported problems with history here: #887 and closing this PR for the moment.

@SergioEstevao SergioEstevao deleted the issue/303-undo-redo-caret-position-lost branch November 8, 2019 11:50
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.

Undo/redo: cursor positioning on Rich Text-based blocks after undoing is pos+1
3 participants