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

History stack is not empty (and broke) on a fresh start of the editor, and on new RichText block #887

Closed
daniloercoli opened this issue Apr 18, 2019 · 3 comments · Fixed by #892
Assignees
Labels
[Type] Bug Something isn't working Writing Flow
Milestone

Comments

@daniloercoli
Copy link
Contributor

daniloercoli commented Apr 18, 2019

History stack is not empty on a clean start of the editor.

Problem 1:

  • Start the demo app
  • Tap on a RichText block
  • Boom ---> you will see the Undo button available
  • 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 first seconds of the GIF attached below

Problem 2:
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. (The GIF below does show this behavior at the end).

history mess up

Investigated a bit, and I think 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 @mkevins

@daniloercoli
Copy link
Contributor Author

The first problem reported here, where the history is not empty on a fresh start of the editor, tapping into a RichText powered block, is due to this call here: https://github.com/WordPress/gutenberg/blob/7e878b791b8a24648da3a7d5f11b9dc2a9397d8e/packages/block-editor/src/components/rich-text/index.native.js#L487

I guess we can avoid that call.

@mkevins
Copy link
Contributor

mkevins commented Apr 19, 2019

I'm able to reproduce this, but it only seems to happen when I tap on the block containing all italicized text: "Gutenberg is available... ...plugin if needed.". It seems that once I tap that block in particular, the undo button becomes "enabled" and never goes away, even after tapping on other RichText blocks that did not previously exhibit the issue.

I also noticed a related issue: when I undo the creation of a new paragraph block, the caret "tries" to go 2 blocks back instead of one. What I mean is:
let's say:

[R] = some RichText block (e.g. paragraph, heading, etc.)
[N] = some non-RichText block (e.g. page break, unsupported, etc.)
[U] = the paragraph block being removed as a result of the undo
* = the selected block

If we start with:

[R]
[R]
[U]*

After undo, we have:

[R]*
[R]

If we start with:

[N]
[R]
[U]*

After undo we have:

[N]
[R]*

If the block being removed by the "undo block" is preceded by two RichText blocks, the caret will land on the first of those two blocks, but if the "undo block" is preceded by only one RichText block, and non-RichText block type precedes that, the caret seems to "stop in the right place" and ends up in the RichText block that preceded the "undo block".

@daniloercoli
Copy link
Contributor Author

Yes, @mkevins , the caret that goes to another block is a known issue, and it's also present when you move the block UP/Down. It's due to the way we call focus/blur and the Android Platform behaviour.

#311

@daniloercoli daniloercoli changed the title History stack is not empty on a fresh start of the editor, and on new RichText block History stack is not empty (and broke) on a fresh start of the editor, and on new RichText block Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug Something isn't working Writing Flow
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants