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 for payment 2024-12-30] [$250] Android - Chat - When context menu is closed, keyboard doesn´t open again #52390

Closed
3 of 8 tasks
IuliiaHerets opened this issue Nov 12, 2024 · 49 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Nov 12, 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.60-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5204110&group_by=cases:section_id&group_order=asc&group_id=292106
Issue reported by: Applause Internal Team

Action Performed:

  1. Open the Expensify app.
  2. Open any chat.
  3. Tap on compose box to open the keyboard.
  4. Long tap on any message to open context menu.
  5. Verify that keyboard is closed when context menu is opened.
  6. Tap outside of the context menu to close it.
  7. Verify that keyboard is opened again.

Expected Result:

When the context menu is closed, keyboard should be displayed again.

Actual Result:

After the context menu is closed, keyboard is not opened again, despite compose box being focused.

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6662106_1731387510202.Context.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021856600731292480706
  • Upwork Job ID: 1856600731292480706
  • Last Price Increase: 2024-11-27
Issue OwnerCurrent Issue Owner: @Christinadobrzyn
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 12, 2024
Copy link

melvin-bot bot commented Nov 12, 2024

Triggered auto assignment to @Christinadobrzyn (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.

@Christinadobrzyn Christinadobrzyn added the External Added to denote the issue can be worked on by a contributor label Nov 13, 2024
Copy link

melvin-bot bot commented Nov 13, 2024

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

@melvin-bot melvin-bot bot changed the title Android - Chat - When context menu is closed, keyboard doesn´t open again [$250] Android - Chat - When context menu is closed, keyboard doesn´t open again Nov 13, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 13, 2024
Copy link

melvin-bot bot commented Nov 13, 2024

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

@Christinadobrzyn
Copy link
Contributor

Oh this is a good one - I think this can be external since it's affecting multiple platforms.

@Pholenk
Copy link

Pholenk commented Nov 13, 2024

Edited by proposal-police: This proposal was edited at 2024-11-15 11:50:54 UTC.

Proposal

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

Android - Chat - When context menu is closed, keyboard doesn´t open again

What is the root cause of that problem?

The keyboard is dismissed because the popup menu's appearance does not cause the input to lose focus; when the popup menu disappears, the input is still focused.

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

const [isMenuActive, setIsMenuActive] = useState(false);
const isPrevMenuActive = usePrevious(isMenuActive);
const [isReportInputFocused, setIsReportInputFocused] = useState(false);
const keyboardState = useKeyboardState();
useEffect(() => {
        if (!keyboardState.isKeyboardShown) {
            return;
        }
        setIsReportInputFocused(true);
}, [keyboardState.isKeyboardShown]);
  • Add this useEffect to track whether pop up menu is shown
useEffect(() => {
        setIsMenuActive(isContextMenuActive || !!(isEmojiPickerActive ?? isPaymentMethodPopoverActive));
}, [isContextMenuActive, isEmojiPickerActive, isPaymentMethodPopoverActive]);
  • After all, add this useEffect to execute the logic
useEffect(() => {
        if (!isPrevMenuActive && isMenuActive) {
            ReportActionComposeFocusManager.composerRef.current?.blur();
        }

        if (isPrevMenuActive && !isMenuActive && isReportInputFocused) {
            ReportActionComposeFocusManager.composerRef.current?.focus();
            setIsReportInputFocused(ReportActionComposeFocusManager.composerRef.current?.isFocused() ?? true);
        }
}, [isMenuActive, isPrevMenuActive, isReportInputFocused]);

What alternative solutions did you explore? (Optional)

Result

screencast-Genymotion-2024-11-14_02.29.56.567.mp4

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@Christinadobrzyn
Copy link
Contributor

hi @jjcoffee can you check on the above proposal when you have a moment? TY!

@jjcoffee
Copy link
Contributor

@Pholenk Thanks for your proposal! Your RCA is plausible, but I'm missing a bit more detail on why this only occurs on Android native.

For your solution, it would also be helpful for you to add where you propose to make your changes, ideally by linking to a relevant code snippet.

@Pholenk
Copy link

Pholenk commented Nov 15, 2024

Proposal

Updated

@Pholenk
Copy link

Pholenk commented Nov 15, 2024

@Pholenk Thanks for your proposal! Your RCA is plausible, but I'm missing a bit more detail on why this only occurs on Android native.

For your solution, it would also be helpful for you to add where you propose to make your changes, ideally by linking to a relevant code snippet.

I have checked that this issue behaves the same way as when the user presses the physical back button while the keyboard is open/shown and, based on react native official documentation, it is a known issue for Android.

@jjcoffee
Copy link
Contributor

I actually can't reproduce on latest main, are you still able to reproduce @Pholenk?

issue-52390-test-2024-11-15_14.25.11.mp4

@Christinadobrzyn Can we ask for a retest?

@Pholenk
Copy link

Pholenk commented Nov 15, 2024

I am still able to reproduce after syncing my fork and using the fresh install version @jjcoffee
Here is the commit ID that I used 4964af5b1fbb0225a52f560c73dc9660769bae28

screencast-Genymotion-2024-11-15_22.52.48.168.mp4

@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Nov 18, 2024

@Christinadobrzyn Can we ask for a retest?

I tested again on Google Pixel 6 Pro 13.0 NewDot version 9.0.60-0. it looks like the keyboard is not opening on Step 7 (of the OP)

screenRecording-18-11-2024-10-3.mp4

@melvin-bot melvin-bot bot added the Overdue label Nov 18, 2024
@jjcoffee
Copy link
Contributor

Hmm strange, I can't reproduce it at all (using Medium Phone API 34)! Are there any steps I might be missing? Have you tried with a small(er) chat, e.g. with a brand new user?

@Pholenk I don't think I can accept the RCA in your proposal without it dealing with why the bug is not happening consistently.

@melvin-bot melvin-bot bot removed the Overdue label Nov 18, 2024
@Pholenk
Copy link

Pholenk commented Nov 18, 2024

Hmm strange, I can't reproduce it at all (using Medium Phone API 34)! Are there any steps I might be missing? Have you tried with a small(er) chat, e.g. with a brand new user?

@Pholenk I don't think I can accept the RCA in your proposal without it dealing with why the bug is not happening consistently.

If that's the case, have you tried with the lower API such as 33 or lower @jjcoffee?
I'm afraid it's not reproducible only for API 34 because I and Christina used the same API (API 33 for Android 13)

I tried with a newly created account, latest version of main branch, and this issue is still there.
https://github.com/user-attachments/assets/0a321075-d68b-47fd-a250-6cbf149f932c

@muttmuure muttmuure moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Nov 19, 2024
@jjcoffee
Copy link
Contributor

@Pholenk Ah yes, I can reproduce it now on API 33, thanks for the tip! Can you explain why that is?

@Pholenk
Copy link

Pholenk commented Nov 19, 2024

You're most welcome @jjcoffee.

I have checked that this issue behaves the same way as when the user presses the physical back button while the keyboard is open/shown and, based on react native official documentation, it is a known issue for Android.

As far as I know, this happened because of this known issue of the text input focus.
That's why I propose to make the text input lose its focus programmatically before the context menu shows up and re-focus it again after the context menu disappears.

Copy link

melvin-bot bot commented Nov 20, 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 Weekly KSv2 label Dec 11, 2024
Copy link

melvin-bot bot commented Dec 13, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@aldo-expensify
Copy link
Contributor

PR merged

Copy link

melvin-bot bot commented Dec 23, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 23, 2024
@melvin-bot melvin-bot bot changed the title [$250] Android - Chat - When context menu is closed, keyboard doesn´t open again [HOLD for payment 2024-12-30] [$250] Android - Chat - When context menu is closed, keyboard doesn´t open again Dec 23, 2024
Copy link

melvin-bot bot commented Dec 23, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Dec 23, 2024
Copy link

melvin-bot bot commented Dec 23, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.77-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-12-30. 🎊

For reference, here are some details about the assignees on this issue:

  • @kirillzyusko does not require payment (Contractor)
  • @jjcoffee requires payment through NewDot Manual Requests

Copy link

melvin-bot bot commented Dec 23, 2024

@jjcoffee @Christinadobrzyn @jjcoffee The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@garrettmknight garrettmknight moved this from Bugs and Follow Up Issues to Hold for Payment in [#whatsnext] #expense Dec 23, 2024
@Christinadobrzyn
Copy link
Contributor

Preparing for payment - @jjcoffee do we need a regression test for this?

@jjcoffee
Copy link
Contributor

jjcoffee commented Dec 26, 2024

BugZero Checklist:

  • [Contributor] Classify the bug:
Bug classification

Source of bug:

  • 1a. Result of the original design (eg. a case wasn't considered)
  • 1b. Mistake during implementation
  • 1c. Backend bug
  • 1z. Other:

Where bug was reported:

  • 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
  • 2b. Reported on staging (eg. found during regression or PR testing)
  • 2d. Reported on a PR
  • 2z. Other:

Who reported the bug:

  • 3a. Expensify user
  • 3b. Expensify employee
  • 3c. Contributor
  • 3d. QA
  • 3z. Other:
  • [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

    Link to comment: N/A - bug/inconsistency with Android itself

  • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

    Link to discussion: N/A

  • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

  • [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

    Link to issue: https://github.com/Expensify/Expensify/issues/456375

Regression Test Proposal

Test:

  1. Open any chat.
  2. Tap on compose box to open the keyboard.
  3. Long tap on any message to open context menu.
  4. Verify that keyboard is closed when context menu is opened.
  5. Tap outside of the context menu to close it.
  6. Verify that keyboard is opened again (iOS native & mWeb, Android native).

Do we agree 👍 or 👎

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Dec 30, 2024
Copy link

melvin-bot bot commented Dec 30, 2024

Payment Summary

Upwork Job

  • Contributor: @kirillzyusko is from an agency-contributor and not due payment
  • Reviewer: @jjcoffee owed $250 via NewDot

BugZero Checklist (@Christinadobrzyn)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1856600731292480706/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@Christinadobrzyn
Copy link
Contributor

Payment date:

Contributor+: @jjcoffee owed $250 via NewDot

Regression test https://github.com/Expensify/Expensify/issues/456375

@melvin-bot melvin-bot bot added the Overdue label Jan 2, 2025
@Christinadobrzyn
Copy link
Contributor

Closing this out - @jjcoffee make sure you submit a ND payment request!

@garrettmknight
Copy link
Contributor

$250 approved for @jjcoffee

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
Status: Done
Development

No branches or pull requests

7 participants