[$1000] Auto focus is broken on New room page

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Login with any account
2. Tap or click FAB menu > New room

Expected Result:

Auto focus on room name input and keyboard shows

Actual Result:

Room name input is not focused and "Create room" button bounces

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [ ] Android / native
• [ ] Android / Chrome
• [x] iOS / native
• [ ] iOS / Safari
• [ ] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.3..0 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/43996225/232314025-3d269f41-d613-4604-a379-99dd9cadb0a7.MP4

https://user-images.githubusercontent.com/43996225/232314049-26306250-f0f1-4a49-81b9-1746731c39ac.mp4

Expensify/Expensify Issue URL: Issue reported by: @situchan Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681493406622809

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0160117b7c7096e58e
• Upwork Job ID: 1651569523514560512
• Last Price Increase: 2023-04-27

[MelvinBot]

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [x] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• [x] This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• [x] This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[MelvinBot]

@johncschuster Whoops! This issue is 2 days overdue. Let's get this updated quick!

[johncschuster]

I'm not sure how to "tab" the FAB menu on mobile. I've asked for clarity on the repro steps in the Slack thread above.

[MelvinBot]

@johncschuster Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[johncschuster]

I've reproduced the behavior. Triaging!

[MelvinBot]

Triggered auto assignment to @techievivek (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[johncschuster]

Umm... this isn't overdue, Melv.

[techievivek]

This can be worked externally.

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~0160117b7c7096e58e

[MelvinBot]

Current assignee @johncschuster is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External)

[MelvinBot]

Triggered auto assignment to @PauloGasparSv (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Balabhadrad]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Auto focus is broken on New room page

What is the root cause of that problem?

I added a breakpoint in RCTBaseTextInputView.textInputDidEndEditing, and I found that it was because RCTModalHostViewManager.dismissModalHostView called RootView.becomeFirstResponder(the View that will be shown below Modal got the first responder), and then called UITextField.resignFirstResponder(lost the first responder)

What changes do you think we should make in order to solve the problem?

Since there is a lot of cross-threading here, and react-native-modal doesn't use onDismiss, and only iOS platform has this problem, I don't want to change the upstream source code, because it maybe cause a lot of regressions

Solution A:

• We can rermove WorkspaceNewRoomPage.shouldDelayFocus and WorkspaceNewRoomPage.autoFocus
• Then we can add ScreenWrapper.onEntryTransitionEnd to WorkspaceNewRoomPage, We call this.roomInput.focus()

Solution B:

• We can remove WorkspaceNewRoomPage.shouldDelayFocus,
• Then we can add ScreenWrapper.didScreenTransitionEnd to WorkspaceNewRoomPage, just like the NewChatPage

What alternative solutions did you explore? (Optional)

[huzaifa-99]

Proposal

Please re-state the problem that we are trying to solve in this issue.

We are trying to auto focus on room name input so that keyboard shows

What is the root cause of that problem?

This is a regression from https://github.com/Expensify/App/issues/15111, the android auto focus works but not for ios.

What changes do you think we should make in order to solve the problem?

We need to delay the input focus on room name input by passing shouldDelayFocus={true} in WorkspaceNewRoomPage. The default value of shouldDelayFocus is false and the one we currently pass (shouldDelayFocus={shouldDelayFocus}) also has false as value

Android Demo

https://user-images.githubusercontent.com/68777211/234888486-03f5fb7a-03cb-4f03-a102-34e85091b96c.mp4

IOS Demo

https://user-images.githubusercontent.com/68777211/234889462-bdef18fb-01b6-4914-95bb-8110b1a66b66.mov

What alternative solutions did you explore? (Optional)

N/A

[thesahindia]

This is a regression from https://github.com/Expensify/App/issues/15111, the android auto focus works but not for ios.

It's was working fine without shouldDelayFocus (check the videos in PR). shouldDelayFocus is only for android. autofocus is working correctly on ios without the shouldDelayFocus prop in other places. You can verify this at BankAccountManualStep

Can you guys check what really caused this?

[kuluruvineeth]

@thesahindia i am able to run web and desktop applications but while i am running npm run ios after running pod install in ios folder, i am not able to run ios(because of version compatibility issue) or android apps....according to local development environment setup there is SO link provided but looks like that is accessible only through expensify domain email which i dont have.....could you please provide further guidelines for me to setup and run ios or android app. Your assistance would really mean a lot for me to start contributing to this project, thanks.

[MelvinBot]

📣 @kuluruvineeth! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[kuluruvineeth]

Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~0164ed01f32c785903

[MelvinBot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[thesahindia]

Hi @kuluruvineeth, have you joined our slack channel yet? If not, you can request an invitation by sending an email to [email protected]. Once you're in the channel, you can search for solutions related to any problem that you might be facing while developing your app. In case you don't find any relevant posts, feel free to ask for assistance in the #expensify-open-source channel.

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

New room page does not auto focus in iOS. (new task page also suffer the same issue)

What is the root cause of that problem?

I don't really have a deep explanation on why this issue happens. But here is what I found so far:

The autofocus issue only happens when we navigate to the page while a modal opens. When a modal opens and we do a navigation, the input will be auto focused, but also will be auto blurred for some reason (that's why we can see the validation error when we open the new room page). It doesn't happen with BankAccountManualStep because there is no modal there. It is also don't happen when we go to New chat page because we delayed the render of the input and the list and show it when didScreenTransitionEnd becomes true. However, the initial intention of using didScreenTransitionEnd is to fix animation related issue here https://github.com/Expensify/App/issues/2335, which I can't reproduce so I don't think we should use this solution.

What changes do you think we should make in order to solve the problem?

We previously already had a similar issue here https://github.com/Expensify/App/issues/15276 on web but I also didn't find the root cause really explain the cause deeply. The point of the solution is to navigate after a modal is completely closed. We can use Modal.close() and pass a function (which is the navigate) as the parameter that will be executed after a modal is completely closed.

https://github.com/Expensify/App/blob/f8acfae37e181c66ab9dbf232772bb293c9b159b/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js#L190

Note:

1. We should always delay the focus for Android (which we already did, no changes)
2. ~For iOS and Web~actually for all platforms, navigate after a modal is completely closed

To solve it globally, I think we can put every code of navigate inside Modal.close https://github.com/Expensify/App/blob/f8acfae37e181c66ab9dbf232772bb293c9b159b/src/libs/Navigation/Navigation.js#L139-L172

[MelvinBot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[aimane-chnaif]

I suggest to put this on hold since popover including FAB menu will be completely refactored in #15289

[hellohublot]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Auto focus is broken on New room page

What is the root cause of that problem?

In PopoverMenu, when we select an item, we will execute this.selectItem -> this.props.onItemSelected -> hideCreateMenu, when the Modal closing animation is executed, it will passively call onModalHide -> resetFocusAndHideModal -> Navigation.navigate(), we navigate to a new page after the Modal is completely hidden, this is indeed the right logic

However, on iOS, when we hide the Modal, RCTModalHostViewManager.dismissModalHostView will execute dismissViewControllerAnimated, which is a system method of iOS, which will call UIViewController for the upcoming rootView(the rootView under the Modal).becomeFirstResponder(), which will make other UITextField lose focus, will call UITextField.resignFirstResponder

What changes do you think we should make in order to solve the problem?

So, we should wait for dismissViewControllerAnimated to execute before executing onModalHide -> Navigation.navigate

Since only iOS has an onDismiss callback, react-native-modal has not been maintained for many years, so I have to use react-native-modal.patch, I also hope you can fork this repository https://github.com/react-native-modal/react-native-modal/blob/6624b5abb326470820f9fedec8b3ecdb35520869/src/modal.tsx#L684-L686

+   // We will handle `onModalHide` in the `onDismiss` event (iOS only)
+   if (Platform.OS === 'ios') {
+       return;
+   }
+   this.props.onModalHide();

...


- return (React.createElement(Modal, Object.assign({ transparent: true, animationType: 'none', 
-    visible: this.state.isVisible, onRequestClose: onBackButtonPress }, otherProps),
+ return (React.createElement(Modal, Object.assign({ transparent: true, animationType: 'none', 
+    visible: this.state.isVisible, onRequestClose: onBackButtonPress, 
+    onDismiss: Platform.OS === 'ios' ? this.props.onModalHide : undefined }, otherProps)

https://user-images.githubusercontent.com/20135674/235093018-71b6ab5d-0307-4b8d-88af-5a4ea75102d2.mp4

[techievivek]

Based on @aimane-chnaif's comment here, I am going to put this on HOLD. We can revisit the issue to see if this is reproducible or not once the PR from the linked issue is merged. Thanks

[johncschuster]

Thanks, @techievivek! Should we downgrade the priority label as well while we wait, or should we keep it on Daily?

[techievivek]

Good point, I am updating it to weekly for now. Thanks

[johncschuster]

@techievivek, are we still held on this one?

[techievivek]

@johncschuster Yeah, it looks the PR review is still in progress, so we will need to HOLD this for some more time.