fix:67907: autofocus inputs in PerDiem and Amount page - #69380
Conversation
|
@jayeshmangwani 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] |
|
PR is ready! I removed autofocus from mobile devices. From the thread we had on Slack, I realized that the main problem was with the keyboard, and someone also mentioned not focusing on phones, so I realized that it was not necessary to focus only on phones. If you think we should remove the focus altogether (on PCs), let me know and I'll update the PR. Thank you! Here is the link to Slack cc: @jayeshmangwani |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid.mp4Android: mWeb Chromemweb-chrome.mp4iOS: HybridAppios.mp4iOS: mWeb Safarimweb-safari.movMacOS: Chrome / Safariweb.movMacOS: Desktopdesktop.mov |
I’m okay with keeping the focus on web and desktop while removing it from mobile devices due to the keyboard pop-ups. I’m looping in the @Expensify/design to see if they have a different perspective. |
I think that was the general consensus from the threads. @Expensify/design @Expensify/product-pr for a gut check! |
|
Mhm, not sure how I feel about this. Most people using Per Diem have a ton of destination/rates, so focusing the input to search for it right away seems snappier than forcing the click or encouraging scrolling a long list of rates. 🤔 I don't see it as being any different to the participant selector step in |
|
I feel like I go back and forth depending on what input field it's talking about. For this one, I don't mind auto-focusing it tbh especially if the list is gonna be super long like you mention, Tom |
|
Okay, just let me know so i can make a proper fix |
|
Yeah, my vote would be this one.
|
|
Okay, can I start working on the changes, or are we waiting for someone else's recommendation? |
|
@dannymcclain @shawnborton you cool with this too? |
|
I definitely won't stand in the way of that route, though it's not my preference haha. I feel like I'm in the minority in terms of preference, so I'm down to go with what you all think is right.
My spicy take preference is that I would rather have to click to focus on mobile (in all of these cases) than have the keyboard automatically pop up. I don't see it as being slower or less snappy—but again, I realize not everyone / maybe most people don't share that perspective so I'm happy to go with what y'all think is best! |
|
Personally I think I generally agree with Danny's preference as a broad rule, but I also kinda agree with Jon that if we are fairly positive the user is going to need to search for something, then I don't mind autofocusing. But if we don't know for sure, I would fallback on not autofocusing. Here I am, just sitting on the fence I guess! |
|
Uff, I'm with Danny. I think with where we're at with RN keyboard controls, I think the auto-focus gets particularly rough on mobile. I'm also not confident we know that a user is going to want to search on this page, so I'd lean more to not doing it until it comes up as a pain point. |
|
I've never known someone not have a lot of per diem rates given the nature of them, so I guess I feel more confident and lean on leaving the status quo as is until someone complains the keyboard is focused to choose their per diem rates. 😅 |
|
Oops, we’re circling back again, and it seems like we still have mixed opinions on the expected result. What’s the final call 😅, are we going with auto-focus or not? |
|
My vote is yes, autoFocus the per diem rate selector page. Every other select list of sorts in |
|
No strong feelings here, works for me! |
|
Okay, so @Eskalifer1 , it seems we are going with auto-focusing. Please update the changes accordingly. |
|
Got it! I'll work on it today! @jayeshmangwani Do I also need to fix the page with the amount? The focus is also lost there, but it's not noticeable because it's not a regular field and the keyboard doesn't appear, but the bug is also present |
|
@Eskalifer1 I think we only want to fix the Per Diem tab here. I’m not sure what other bug you’re referring to - if you can share a video, I can confirm whether we should fix it here or not |
|
@jayeshmangwani Here is a video showing that there is a caret on the first entry, but it is missing on the second entry. This is the same RCA as for the perDiem page, so I am asking if this needs to be done. 2025-09-10.15.19.03.mov |
|
Okay, cool - let’s solve that one here too if the RCA is the same. |
|
Sorry for the delay, I’ll get to this PR soon. My local iOS tabs have been acting flaky, so I’m working on fixing that first. |
|
✋ 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/madmax330 in version: 9.2.16-0 🚀
|
|
We decided to revert changes for manual tab since this is out of scope, and this approach adds delay (as on all pages where there is focus when changing tabs with the approach used here), which negatively affects UX in this case, and if we want to fix this in the future, it is better to create a separate task for this. As a result, this PR only fixes the focus on the PerDiem page. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.2.16-14 🚀
|
|
🚀 Deployed to staging by https://github.com/madmax330 in version: 9.2.17-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.2.17-6 🚀
|
Explanation of Change
This PR fixes autofocus inputs on the Amount and PerDiem pages when creating an expense, as focus is lost on iOS.
Fixed Issues
$#67907
PROPOSAL:#67907 (comment)
Tests
Preconditions: Have a workspace with Per Diem enabled.
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
67907-android-native.mp4
Android: mWeb Chrome
67907-android-web.mp4
iOS: Native
67907-ios-native.mp4
iOS: mWeb Safari
67907-ios-web.mp4
MacOS: Chrome / Safari
67907-web.mp4
MacOS: Desktop
67907-desktop.mp4