-
Notifications
You must be signed in to change notification settings - Fork 58
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
After two blocks are merged, show cursor on the end of the text field #756
After two blocks are merged, show cursor on the end of the text field #756
Conversation
… avoid block merging when placeholder is present Move cursor to the end of the text when two blocks being merged
For the Issue A wondering if we can expand the code added here https://github.com/wordpress-mobile/gutenberg-mobile/pull/629/files#diff-5693404c4c8f8d757cd298cc5aabc561R117 introduced to set the caret at the end of text when getting the focus. |
…ves-cursor-to-last-location-in-previous-block # Conflicts: # react-native-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/ReactAztecManager.java
…ves-cursor-to-last-location-in-previous-block
Put comment for else if branch so that we can remember what's the purpose of it
…nd just use current text content to detect when we need to send backspace to JS side
Hey @daniloercoli , thanks for the good point. Please take a look at new iteration :) |
Hi! I've been looking into this for a while, results of my tests here: ISSUE B:
ISSUE AThis one is still happening for me.
Question, does this (2) happen for you @marecar3 ? I'm using a Pixel 2 with Android 8.1 to test. |
Added PR (comparing branch against this one) to try and fix (2) above here #765 |
…ification-on-end Added a minimal delay to call setSelection() to enforce special focus handling cases
Hey @mzorz, I have noticed that we introduced some regression, this is the scenario :
Result -> On the newly created block cursor is at the end of the text |
🤔 maybe is it due to this change here? https://github.com/wordpress-mobile/gutenberg-mobile/pull/765/files#diff-6d32b8ead802ea7e00caa5563c2239c3L33 |
Just a FYI I was trying something here #773, still need to work more on it but open to any suggestions |
Hey @mzorz, thanks for heads up. Will look at it and I am also trying some ideas. |
…ves-cursor-to-last-location-in-previous-block
…ves-cursor-to-last-location-in-previous-block
…ves-cursor-to-last-location-in-previous-block
Hey Hey @etoledom @mzorz @daniloercoli @pinarol, I have moved the logic to the JS side so if you have time please take a look at this PR and corresponding gutenberg PR : WordPress/gutenberg#14820 |
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, code LGTM! Thanks for this @marecar3
Feel free to merge after resolving conflicts
…ves-cursor-to-last-location-in-previous-block # Conflicts: # gutenberg
Will merge this one as it seems that the same tests are also failing on the develop branch. |
Based on issue: #699
Corresponding gutenberg PR : WordPress/gutenberg#14820
There are two issues :
Issue A: After two blocks are merged, a cursor will be placed on the last remembered position in the block. (Affected both iOS and Android platform)
Steps to reproduce:
Issue B: When the user clears the text in the e.g. Paragraph/Heading block, a block will be merged right after the last letter is deleted. (Affects only Android platform) - Which should be a case, as on iOS block will be merged after one extra backspace.
Steps to reproduce:
Hey @daniloercoli , @mzorz at this point I invested some extra time and I couldn't find a better solution so I am very open to hearing your suggestions if you have something on your mind. I tried to find some common solution which would be placed in JS code, but it seems that it couldn't be doable from JS side. I will mark this one as ready for review very soon so that you can start reviewing it.