-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[RNMobile] add selectionStart and selectionEnd to transientEdits #22492
Conversation
Size Change: +205 B (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
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.
I tested this on Pixel 3a and this PR works as described. I confirmed too that the issues mentioned are present in develop
, and not introduced in this PR. I also observed another issue (which is also present on develop
) where the caret did not "land" in the correct place after undo and redo actions:
Nice work on this PR @dratwas , and thanks for documenting your findings with great detail! 😄
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.
Tested on iOS iPhone 11 works as described for me as well 👍
👋 It looks like this change has introduced a regression with respect to saving a post when someone is typing quickly: wordpress-mobile/gutenberg-mobile#2349. |
Description
Gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#2285
In this PR i added a
selectionStart
andselectionEnd
to transient Edits. The motivation is described here: wordpress-mobile/gutenberg-mobile#2192 (comment)I also added a debounce function to create an undo level when the input (rich-text) stops for over a second to be consistent with the web:
https://github.com/WordPress/gutenberg/blob/master/packages/rich-text/src/component/index.js#L501
It changes how the undo mechanism works on mobile.
Known Issues:
These issues are not introduced by this PR
Sometimes the redo level is removed in rich-text - After testing, I can say that this PR doesn't introduce that issue but it is easier to reproduce. (GIFS in the testing section) I found that it happens only when the paragraph block has the break line at the very beginning. It's really edge case because as far as I know, it is possible by adding it to the code editor.
We create unnecessary undo levels on the editor init - [RNMobile] add selectionStart and selectionEnd to transientEdits #22492 (comment)
How has this been tested?
Undo/redo block actions
Undo/redo text
NOTE: This test case doesn't work as before. The whole text is removed since we do not create the undo level when we change the same attribute in the same block. Before changes from this PR the undo level was created after each sign in the rich-text. After these changes, the undo works the same as on the web.
Edit: I observed a weird behavior. Sometimes when i remove text by pressing on the undo button the redo level is not created - see the gif below (the redo button is disabled after pressing undo)
Undo/redo text format
In that case, i have the same issue as in the previous one - the redo level is removed in rich-text sometimes
I've tested that and it seems like these issues i mentioned above also appear w/o changes from this PR
Types of changes
Add
selectionStart
andselectionEnd
to transientEdits to make undo mechanism work the same as on the webChecklist: