PATCH: Android - Fix caret resetting to the beginning of the input#52734
Conversation
|
@shawnborton @mananjadhav One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
Reviewing |
|
Videos look pretty good to me 👍 |
|
It does look like there is a slight delay once the cursor is shown right after the currency amount and then the cursor goes to the end of the input when the amount shows up. I'm not sure exactly what to make of that, maybe that's just an artifact of using RN here. cc @Expensify/design for input, and Jon curious if this is something that feels common across Android devices. To be clear, this is a huge improvement from what we had before so I'm happy to move forward with what is shown here. |
|
Note: The videos were taken on Android emulator which is pretty slow UI / UX wise compared to real Android devices. I will perform a build of this on my OnePlus 7T running Android 14 tomorrow to show how it looks on an actual device 🙌 Edit: I tested on real device and there's no difference in this case.
I think this comes from the fact that the amount input's width is dynamic (grows with the input) which causes that glitchy effect on mount: App/src/components/AmountTextInput.tsx Lines 61 to 63 in a1196a6
From my side we're good to go move forward here with merging the PR. |
Nothing I've come across, but agree that it looks like a slow device/RN artefact. I would say we're still likely to see this on real devices though given lots of people out there don't have the newest technology. But still this feels a lot better so I'm okay with it too. |
|
Left with the final testing, will finish the checklist today. |
|
🚧 @mountiny has triggered a test build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
|
@mananjadhav what is your ETA for the checklist? |
|
I have been trying to run the build locally and it's failing. I just finished a checklist for another PR and I didn't have the issue. Will delete all node_modules again and update the checklist in max few hours. |
Reviewer Checklist
Screenshots/VideosAndroid: Nativeandroid-send-invoice-amount-caret.mp4android-card-limt-amount-caret.mp4Android: mWeb Chromemweb-chrome-amount-caret.moviOS: Nativeios-card-amount-caret.moviOS: mWeb Safarimweb-safari-amount-caret.movMacOS: Chrome / SafariMacOS: Desktop |
|
All good on my end, feel free to merge 👍 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/aldo-expensify in version: 9.0.65-0 🚀
|
|
🚀 Deployed to production by https://github.com/chiragsalian in version: 9.0.65-5 🚀
|
Explanation of Change
We're applying a patch which includes two
react-nativefixes (1) and (2) which were merged but will take some time to reach production. The patch is fixing an issue based in the Android native code which caused the selection caret to be reset to the beginning of the input instead at the end when the input is not empty. More details in the issue / proposal.Fixed Issues
$ #52029
$ #50055
PROPOSAL: #52029 (comment)
Tests
Note
This PR fix only applies to Android: Native, none of the other platforms will be affected or work differently.
Precondition: Make sure to perform a clean install of
node_modulesand ensure the new patch is applied.Steps for issue #52029:
Steps for issue #50055:
Offline tests
N/A.
QA Steps
Note
This PR fix only applies to Android: Native, none of the other platforms will be affected or work differently.
Steps for issue #52029:
Steps for issue #50055:
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Note
This PR fix only applies to Android: Native, none of the other platforms will be affected or work differently.
android-card-limit.webm
android-invoice-amount.webm
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop