fix: mobile doesn't need manual focus anymore for autoFocus#54755
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
|
@allroundexperts Please 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] |
|
Added a follow up ticket as well: |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeScreen.Recording.2025-01-08.at.11.08.39.PM.moviOS: NativeScreenRecording_01-08-2025.21-07-16_1.MP4ScreenRecording_01-08-2025.21-10-24_1.MP4ScreenRecording_01-08-2025.21-14-10_1.MP4iOS: mWeb SafariScreen.Recording.2025-01-08.at.10.56.54.PM.movMacOS: Chrome / SafariScreen.Recording.2025-01-08.at.10.52.57.PM.movMacOS: DesktopScreen.Recording.2025-01-08.at.10.55.35.PM.mov |
|
@hannojg Although the field is focused, but the keyboard does not appear. Is that expected? ScreenRecording_01-08-2025.22-41-03_1.MP4 |
I know that there are certain conditions in which we will display the keyboard in the chat. We don't display it per se for every chat. I would need to lookup the conditions for that, hold on … |
|
This is happening on dev as well btw, so it shouldn't block the approval. |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #54759 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #54759 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
🎀 👀 🎀 |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #54759 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
mountiny
left a comment
There was a problem hiding this comment.
Thanks, changes look good to me
|
Is this issue happening in dev too? #54755 (comment) I think we should open the keyboard everytime |
|
I think there is custom behavior to when the keyboard should open when opening a chat (I never understood why though, but there were some reasons for that). On production the keyboard doesn't open for me for every chat. |
|
😂 😂 when I have time I will find the specific code lines that control that behaviour |
|
✋ 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/mountiny in version: 9.0.89-0 🚀
|
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.0.89-8 🚀
|

Explanation of Change
There is a bug in react native where calling
textInputRef.focus()doesn't focus the text input. That is the reason for the bug which this PR wants to fix. This kind of bug doesn't happen when we use theautoFocusnative prop on theTextInput.I figured, instead of going down the rabbit hole and analyzing why
.focus()became broken in certain conditions, we should start to rely on theautoFocusprop again.In the past we use to manually call
textInputRef.focus()to circumvent a bunch of bugs. However, now that we are:those previous problems don't really exist anymore, and it seems safe to remove our manually calling code. (I think it's also cleaner to remove our "custom" code and instead rely on the build in APIs such as the
autoFocusprop).Note
For now I have kept the
shouldDelayFocusprop. I think we should be able to remove this prop as well as it was only helping us to circumvent certain bugs, but:autoFocusprop and we need to investigate why that happensOpened a follow up ticket here:
shouldDelayFocusprop #54759Fixed Issues
$ #54658
PROPOSAL: #54658
Tests
Additionally make sure all other text inputs when navigating through the app are still being auto focused.
Offline tests
n/a
QA Steps
Same as testing steps
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
Android: Native
Screen_Recording_20250102_141359_New.Expensify.Dev.mp4
Android: mWeb Chrome
Screen_Recording_20250102_144524_Samsung.Internet.mp4
iOS: Native
ios_after.MP4
iOS: mWeb Safari
ios_mWeb.MP4
MacOS: Chrome / Safari
CleanShot.2025-01-02.at.14.39.03.mp4
MacOS: Desktop
CleanShot.2025-01-02.at.14.48.53.mp4