Skip to content
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

[$250] keyboard hides unexpectedly when adding a new task on the report screen #50442

Closed
1 of 6 tasks
m-natarajan opened this issue Oct 8, 2024 · 31 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@m-natarajan
Copy link

m-natarajan commented Oct 8, 2024

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


Version Number: 9.0.46-3
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
Expensify/Expensify Issue URL:
Issue reported by: @jayeshmangwani
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1728308186934659

Action Performed:

  1. Go to any chat and add a new task. (keyboard should open at this point)
  2. Tap on the newly created task to view task details.
  3. Check the composer.

Expected Result:

Keyboard should remain open when viewing the task details.

Actual Result:

Composer is shown as active, but the keyboard is hidden.

Workaround:

unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Android.keyboard.mp4
az_recorder_20241008_131240.1.mp4
Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021844835682397684717
  • Upwork Job ID: 1844835682397684717
  • Last Price Increase: 2024-11-08
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 8, 2024
Copy link

melvin-bot bot commented Oct 8, 2024

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@QichenZhu
Copy link
Contributor

QichenZhu commented Oct 9, 2024

Proposal

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

Tapping a task on the report screen while the soft keyboard is open will close the keyboard.

What is the root cause of that problem?

The soft keyboard is closed during the screen transition by the two pieces of code below.

https://github.com/react-navigation/react-navigation/blob/4caf3cb849a8708a36d0dbf22dac432d2ca780be/packages/stack/src/utils/useKeyboardManager.tsx#L51

Screenshot 2024-10-10 at 1 54 19 AM

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

We don't need to pass the autoFocus attribute to the Android native input since useAutoFocusInput will take care of it after the screen transaction ends.

const isAndroidNative = getPlatform() === CONST.PLATFORM.ANDROID;

autoFocus={autoFocus}

autoFocus={isAndroidNative ? undefined : autoFocus}

What alternative solutions did you explore? (Optional)

We could consider disabling auto focus for the task details screen, just like we did for the money request details screen below.

const shouldAutoFocus =
!modal?.isVisible &&
Modal.areAllModalsHidden() &&
isFocused &&
(shouldFocusInputOnScreenFocus || (isEmptyChat && !ReportActionsUtils.isTransactionThread(parentReportAction))) &&
shouldShowComposeInput;

@melvin-bot melvin-bot bot added the Overdue label Oct 10, 2024
Copy link

melvin-bot bot commented Oct 11, 2024

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

@johncschuster johncschuster added the External Added to denote the issue can be worked on by a contributor label Oct 11, 2024
Copy link

melvin-bot bot commented Oct 11, 2024

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

@melvin-bot melvin-bot bot changed the title keyboard hides unexpectedly when adding a new task on the report screen [$250] keyboard hides unexpectedly when adding a new task on the report screen Oct 11, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 11, 2024
Copy link

melvin-bot bot commented Oct 11, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Oct 11, 2024
@QichenZhu
Copy link
Contributor

QichenZhu commented Oct 12, 2024

UPDATE: My proposal doesn’t apply to the latest main.

showSoftInputOnFocus is set to false on purpose to fix #10731, so it’s expected that the soft keyboard doesn’t show.

UPDATE 2: My proposal is valid again since PR #50279 is reverted.

Copy link

melvin-bot bot commented Oct 15, 2024

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

@johncschuster
Copy link
Contributor

Looks like we're still waiting on proposals

@johncschuster
Copy link
Contributor

Bump for Melv

@melvin-bot melvin-bot bot removed the Overdue label Oct 16, 2024
Copy link

melvin-bot bot commented Oct 18, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Oct 21, 2024
@johncschuster
Copy link
Contributor

@allgandalf can you take a look at @QichenZhu's proposal and let me know if it's sufficient? If not, I'll drum up more proposals!

@melvin-bot melvin-bot bot removed the Overdue label Oct 22, 2024
@mollfpr
Copy link
Contributor

mollfpr commented Oct 22, 2024

@johncschuster Do you mean me? 😅

I'll review the latest proposal!

Copy link

melvin-bot bot commented Oct 22, 2024

@johncschuster @mollfpr this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Oct 24, 2024
@johncschuster
Copy link
Contributor

@allgandalf

@johncschuster Do you mean me? 😅

.... 🤦 .... yes. Sorry about that! Did you get a chance to review the latest proposal?

@melvin-bot melvin-bot bot removed the Overdue label Oct 24, 2024
@mollfpr
Copy link
Contributor

mollfpr commented Oct 25, 2024

@QichenZhu to be clear the autoFocus from the text input conflict with the blur/dismiss from the library and our component?

Also, why do we still need the autoFocus property for iOS even though the useAutoFocusInput also focuses on the input?

@mollfpr
Copy link
Contributor

mollfpr commented Oct 25, 2024

I repro the issue a while ago, but after pulling the latest main is not reproducing.

Screen_Recording_20241025_201335_New.Expensify.Dev.mov

Copy link

melvin-bot bot commented Oct 25, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@QichenZhu
Copy link
Contributor

after pulling the latest main is not reproducing

Still reproducible on my end.

1000000765-2024-10-25.21_10_10.201.mp4

to be clear the autoFocus from the text input conflict with the blur/dismiss from the library and our component?

Also, why do we still need the autoFocus property for iOS even though the useAutoFocusInput also focuses on the input?

@mollfpr I'll look into it.

@QichenZhu
Copy link
Contributor

QichenZhu commented Oct 28, 2024

to be clear the autoFocus from the text input conflict with the blur/dismiss from the library and our component?

@mollfpr Both factors could contribute to the issue. Addressing either one will make the other take effect.

Also, why do we still need the autoFocus property for iOS even though the useAutoFocusInput also focuses on the input?

The useAutoFocusInput hook was initially introduced for the web and recently applied to the native component to address an Android issue (#45008). I'm not sure why it doesn't work on iOS, but I believe it was never intended to.

Update: The reason useAutoFocusInput doesn't work on iOS is because at first updateMultilineInputRange tries to focus the input,

but fails siliently becuase at that time componentView is nullptr.

https://github.com/facebook/react-native/blob/1611a5e8a1a9a9cb7d6d9ddb4d7316d8e634e6e3/packages/react-native/React/Fabric/Mounting/RCTMountingManager.mm#L334

Screenshot 2024-10-29 at 12 17 44 PM

However, the input is optimistically marked as focused regardless,

https://github.com/facebook/react-native/blob/0705eb8b91b6c3688a73f8481c6b067899ac6eec/packages/react-native/Libraries/Components/TextInput/TextInputState.js#L62

Screenshot 2024-10-29 at 12 26 12 PM

so future attempts to programmatically focus it will be ignored.

https://github.com/facebook/react-native/blob/0705eb8b91b6c3688a73f8481c6b067899ac6eec/packages/react-native/Libraries/Components/TextInput/TextInputState.js#L106

Screenshot 2024-10-29 at 12 27 03 PM

@melvin-bot melvin-bot bot added the Overdue label Oct 28, 2024
Copy link

melvin-bot bot commented Oct 28, 2024

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

@johncschuster
Copy link
Contributor

@mollfpr bump on the conversation above!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 28, 2024
@johncschuster
Copy link
Contributor

Bumped in Slack

@melvin-bot melvin-bot bot removed the Overdue label Oct 31, 2024
@mollfpr
Copy link
Contributor

mollfpr commented Nov 1, 2024

The issue is not reproducible because we disabled auto focus for task reports recently #50544.

Copy link

melvin-bot bot commented Nov 1, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@QichenZhu
Copy link
Contributor

I don't think the root cause has been fixed. I can still reproduce it with other steps.

Record_2024-11-02-09-13-37.mp4

@melvin-bot melvin-bot bot added the Overdue label Nov 4, 2024
@mollfpr
Copy link
Contributor

mollfpr commented Nov 4, 2024

@QichenZhu Thanks, I'm able to reproduce the issue with the above step.

It's still not clear to me why the issue only happens on Android, and for the solution even though it's working I feel this is like a workaround.

@melvin-bot melvin-bot bot removed the Overdue label Nov 4, 2024
@QichenZhu
Copy link
Contributor

@mollfpr

It's still not clear to me why the issue only happens on Android

I believe it's clear why it doesn't work on Android, isn't it?

iOS has its own issues (#50442 (comment), #51482) but it just happens to work in this senario for now. I'd love to explore more if it's a meaningful problem.

for the solution even though it's working I feel this is like a workaround

Focusing with a delay is a common solution in our repo, used in similar issues like #51010 (comment) and #51279 (comment).

Copy link

melvin-bot bot commented Nov 5, 2024

@johncschuster @mollfpr this issue is now 4 weeks old, please consider:

  • Finding a contributor to fix the bug
  • Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

Copy link

melvin-bot bot commented Nov 8, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Nov 8, 2024
@johncschuster
Copy link
Contributor

@mollfpr bump on the above comment!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Nov 8, 2024
@johncschuster
Copy link
Contributor

Bump!

@melvin-bot melvin-bot bot removed the Overdue label Nov 11, 2024
@github-project-automation github-project-automation bot moved this from LOW to Done in [#whatsnext] #quality Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
Development

No branches or pull requests

5 participants