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

[HOLD #41128] [$500] mWeb - Chat - The keyboard appears for a moment when leaving the thread #35494

Closed
1 of 6 tasks
kbecciv opened this issue Jan 31, 2024 · 85 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@kbecciv
Copy link

kbecciv commented Jan 31, 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: v1.4.34-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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Issue found when executing PR #35202

Action Performed:

  1. Go to URL https://staging.new.expensify.com/ on Android
  2. Login with any account
  3. Navigate to any conversation
  4. Start a thread and send some messages
  5. Tap and hold on a message and choose Reply in Thread
  6. Click on the three-dot menu and hit "Leave thread"

Expected Result:

The keyboard doesn't appear for a moment when leaving the thread

Actual Result:

The keyboard appears for a moment when leaving the thread

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

Add any screenshot/video evidence

Bug6362602_1706721121608.video_2024-01-31_17-29-22.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01278478c2e7ed2136
  • Upwork Job ID: 1752743923246346240
  • Last Price Increase: 2024-03-15
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 31, 2024
@melvin-bot melvin-bot bot changed the title mWeb - Chat - The keyboard appears for a moment when leaving the thread [$500] mWeb - Chat - The keyboard appears for a moment when leaving the thread Jan 31, 2024
Copy link

melvin-bot bot commented Jan 31, 2024

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

Copy link

melvin-bot bot commented Jan 31, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 31, 2024
Copy link

melvin-bot bot commented Jan 31, 2024

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

@kbecciv
Copy link
Author

kbecciv commented Jan 31, 2024

We think that this bug might be related to #vip-vsb
CC @quinthar

@Christinadobrzyn
Copy link
Contributor

I can reproduce this based on the steps in the OP. I think this can be External so waiting for proposals.

@samilabud
Copy link
Contributor

samilabud commented Jan 31, 2024

Proposal

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

The keyboard appears for a moment when leaving the thread

What is the root cause of that problem?

When we are in a thread and the composer is focused the keyboard going to be displayed, but when we click on the three dots the keyboard get hide (as part of the behavior of this component), here no matter what option we select the keyboard going to be displayed again after three dots menu get hide, because of this when we leave the thread the keyboard seems to appear (expected behavior) and then disappears when the leave action makes we navigates back to main thread because the composer loose the focus.

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

We can keep the focus on the composer when navigating backwards and the keyboard is displayed.

To do this we should modify the ReportScreen Component to add a variable that determine if the user got to the screen using the navigation component with path (this let us know if the user is not getting to the Report from LHN), like:

const isReportOpenedFromNavigationPath = Boolean(lodashGet(route, 'path') || false);

And pass this to ReportFooter as a prop:

<ReportFooter
                                       report={report}
                                       pendingAction={addWorkspaceRoomOrChatPendingAction}
                                       isComposerFullSize={isComposerFullSize}
                                       listHeight={listHeight}
                                       isEmptyChat={isEmptyChat}
                                       lastReportAction={lastReportAction}
                                       isReportOpenedFromNavigationPath={isReportOpenedFromNavigationPath}
                                   />

With the prop isReportOpenedFromNavigationPath we can now determinate when to force the input focus, to do this we can pass as a prop the result of this variable on ReportActionCompose, like:

 const forceFocusComposer = props.isReportOpenedFromNavigationPath;
.
.
.
<ReportActionCompose
                            onSubmit={onSubmitComment}
                            reportID={props.report.reportID}
                            report={props.report}
                            isEmptyChat={props.isEmptyChat}
                            lastReportAction={props.lastReportAction}
                            pendingAction={props.pendingAction}
                            isComposerFullSize={props.isComposerFullSize}
                            listHeight={props.listHeight}
                            isReportReadyForDisplay={props.isReportReadyForDisplay}
                            shouldForceComposerFocus={forceFocusComposer}
                        />

Finally we should receive the new prop and use it to focus the composer when is true inside of ReportActionCompose component, like:

useEffect(() => {
        if (!shouldForceComposerFocus) {
            return;
        }
        focus();
    }, [shouldForceComposerFocus]);

Result:

Leaving.thread.keyboard.issue.mp4

@Christinadobrzyn
Copy link
Contributor

collecting proposals!

@melvin-bot melvin-bot bot added the Overdue label Feb 5, 2024
@sobitneupane
Copy link
Contributor

sobitneupane commented Feb 5, 2024

@samilabud Thanks for your proposal

onMouseDown={(e) => {
/* Keep the focus state on mWeb like we did on the native apps. */
if (!Browser.isMobile()) {
return;
}
e.preventDefault();
}}

Above code block was introduced by this PR. As per the discussion in the PR, the change was made to refocus the text input after closing the modal.

I was testing it and in any other case, if the composer was focused and we open any modal or menu, it get's refocused on going back, so we should do the same with the 3 dots!

We refocus the text input after closing the modal in Native and mWeb. If we remove the code, the text input will not get refocused even if we close the modal without taking any action.

@melvin-bot melvin-bot bot removed the Overdue label Feb 5, 2024
@Christinadobrzyn
Copy link
Contributor

Still accepting proposals!

@samilabud
Copy link
Contributor

Proposal

Updated

@sobitneupane I have added the details of alternative proposal, please check it out.

Copy link

melvin-bot bot commented Feb 7, 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 Feb 8, 2024
@Christinadobrzyn
Copy link
Contributor

@sobitneupane can you review this proposal when possible? #35494 (comment)

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Feb 8, 2024
Copy link

melvin-bot bot commented Feb 12, 2024

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

@sobitneupane
Copy link
Contributor

I will review the proposal asap.

@melvin-bot melvin-bot bot removed the Overdue label Feb 12, 2024
@sobitneupane
Copy link
Contributor

sobitneupane commented Feb 13, 2024

@samilabud Thanks for the update.

But I am quite confused about the solution. It looks like a work around to me.

What we want is to prevent the keyboard to close and reopen after leaving the thread. Can you please re-sate the problem, the root cause and how your solution is going to deal with the root cause. Please do explain your solution as well.

@samilabud
Copy link
Contributor

Proposal

Updated

@sobitneupane I have explained the solution better, please let me know if it makes sense to you.  🙏🏼

@melvin-bot melvin-bot bot added the Overdue label Apr 17, 2024
Copy link

melvin-bot bot commented Apr 18, 2024

@Julesssss, @sobitneupane, @Christinadobrzyn Whoops! This issue is 2 days overdue. Let's get this updated quick!

@Christinadobrzyn
Copy link
Contributor

hi sorry @sobitneupane or @Julesssss can you provide an update, I'm not sure if we looking for proposals for the issue reported #35494 (comment) or if that should be a different issue. I'm wondering if that is an issue at all...

@melvin-bot melvin-bot bot removed the Overdue label Apr 19, 2024
@Christinadobrzyn
Copy link
Contributor

Dmd Jules to see if I can get some guidance on what to do next for this

@Julesssss
Copy link
Contributor

Hey @Christinadobrzyn I was ooo.

But the behavior now is little different. Keyboard does not disappear when leaving the chat.

As the user is going back to a parent thread I'm not sure this is a big issue. The pre-fix keyboard issue was far worse in my opinion, but lets see if we can further improve. Some questions first though:

  • Is this only occurring on Android?
  • Does this only occur when navigating from a child thread back to a parent thread?
  • What about being linked from one chat to another?
  • When going from a top level chat to the LHN does this occur?

@melvin-bot melvin-bot bot added the Overdue label Apr 24, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

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

@Christinadobrzyn
Copy link
Contributor

Sorry for the delay here - I'll test these on Monday unless someone @sobitneupane you have time.

  1. Is this only occurring on Android?
  2. Does this only occur when navigating from a child thread back to a parent thread?
  3. What about being linked from one chat to another?
  4. When going from a top level chat to the LHN does this occur?

@melvin-bot melvin-bot bot removed the Overdue label Apr 26, 2024
@sobitneupane
Copy link
Contributor

  1. Is this only occurring on Android?

I tested it on IOS. But it looks like there is some Issue in IOS with navigation. Each time I leave the thread instead of going back to the parent thread, it opens a new report(parent report). When I go back, I will end up in the same report(parent report). This issue can only be tested once the issue gets resolved.

RPReplay_Final1714374664.MP4

I could not test it on android though. I am having some issue while trying to build android.

@Julesssss
Copy link
Contributor

Hey @sobitneupane, the issue you mention sounds like this issue. Maybe we can wait for this one to be resolved

@sobitneupane
Copy link
Contributor

Hey @sobitneupane, the issue you mention sounds like this issue. Maybe we can wait for this one to be resolved

Agreed. It seems like there might be a connection there.

@Christinadobrzyn Christinadobrzyn changed the title [$500] mWeb - Chat - The keyboard appears for a moment when leaving the thread [HOLD #41128] [$500] mWeb - Chat - The keyboard appears for a moment when leaving the thread May 1, 2024
@Christinadobrzyn
Copy link
Contributor

added Hold for #41128 to the subject. Let me know if that doesn't seem like the best next move.

@Christinadobrzyn Christinadobrzyn added Weekly KSv2 and removed Daily KSv2 labels May 1, 2024
@Christinadobrzyn
Copy link
Contributor

watching - #41128

@Julesssss
Copy link
Contributor

Hey @Christinadobrzyn, I'm going to try and move that linked issue forward.

@Christinadobrzyn
Copy link
Contributor

thanks @Julesssss! I'll keep an eye on #41128

@Christinadobrzyn
Copy link
Contributor

monitoring - #41128

1 similar comment
@Christinadobrzyn
Copy link
Contributor

monitoring - #41128

@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented May 28, 2024

PR - #41128 is done and I think this is resolved.

Testing again and the keyboard shows after leaving a thread.

screenRecording-28-5-2024-16-8.mp4

Gonna ask QA to test again. https://expensify.slack.com/archives/C9YU7BX5M/p1716883797729199

@isagoico
Copy link

Issue is still reproducible on Android mWeb

1953885561340247456az_recorder_20240528_103525.mp4

@Christinadobrzyn
Copy link
Contributor

hum @sobitneupane can you restest? I'm not able to reproduce as is above #35494 (comment)

@Christinadobrzyn Christinadobrzyn added Daily KSv2 and removed Weekly KSv2 labels Jun 5, 2024
@sobitneupane
Copy link
Contributor

@Christinadobrzyn I could not reproduce the issue in Android/Chrome.

@Christinadobrzyn
Copy link
Contributor

Awesome - let's close! thanks for checking @sobitneupane!

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 Engineering External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Archived in project
Development

No branches or pull requests

10 participants