App icon indicating copy to clipboard operation
App copied to clipboard

[$250] Android - Chat - When context menu is closed, keyboard doesn´t open again

Open IuliiaHerets opened this issue 1 year ago • 34 comments

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:

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

Screenshots/Videos

https://github.com/user-attachments/assets/6e8fea20-14ca-44b8-beb5-c12cdab1d4dd

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: @kirillzyusko

IuliiaHerets avatar Nov 12 '24 13:11 IuliiaHerets

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.

melvin-bot[bot] avatar Nov 12 '24 13:11 melvin-bot[bot]

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

melvin-bot[bot] avatar Nov 13 '24 07:11 melvin-bot[bot]

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

melvin-bot[bot] avatar Nov 13 '24 07:11 melvin-bot[bot]

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

Christinadobrzyn avatar Nov 13 '24 07:11 Christinadobrzyn

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?

  • We need to add these lines to track whether the input is focused and popup menu is active at https://github.com/Expensify/App/blob/9228354caa0f3731ec7d05241fd8363c13477c5b/src/pages/home/report/ReportActionItem.tsx#L190
const [isMenuActive, setIsMenuActive] = useState(false);
const isPrevMenuActive = usePrevious(isMenuActive);
const [isReportInputFocused, setIsReportInputFocused] = useState(false);
const keyboardState = useKeyboardState();
  • To track that input has been focused once, add this useEffect at https://github.com/Expensify/App/blob/9228354caa0f3731ec7d05241fd8363c13477c5b/src/pages/home/report/ReportActionItem.tsx#L272
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

https://github.com/user-attachments/assets/056bf0f3-89d8-44db-b4b8-7f1460ec9625

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.

Pholenk avatar Nov 13 '24 19:11 Pholenk

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

Christinadobrzyn avatar Nov 15 '24 02:11 Christinadobrzyn

@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.

jjcoffee avatar Nov 15 '24 09:11 jjcoffee

Proposal

Updated

Pholenk avatar Nov 15 '24 11:11 Pholenk

@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.

Pholenk avatar Nov 15 '24 11:11 Pholenk

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

https://github.com/user-attachments/assets/c0318f94-ff65-4750-a425-71977030c854

@Christinadobrzyn Can we ask for a retest?

jjcoffee avatar Nov 15 '24 13:11 jjcoffee

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

https://github.com/user-attachments/assets/73b7dc7c-c035-4347-afff-63bd79f8ccc2

Pholenk avatar Nov 15 '24 15:11 Pholenk

@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)

https://github.com/user-attachments/assets/7dddc592-9952-4ef3-a38c-f0bba8dc3eb6

Christinadobrzyn avatar Nov 18 '24 02:11 Christinadobrzyn

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.

jjcoffee avatar Nov 18 '24 12:11 jjcoffee

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

Pholenk avatar Nov 18 '24 17:11 Pholenk

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

jjcoffee avatar Nov 19 '24 14:11 jjcoffee

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.

Pholenk avatar Nov 19 '24 15:11 Pholenk

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

melvin-bot[bot] avatar Nov 20 '24 16:11 melvin-bot[bot]

@Pholenk I guess my problem here is that the RCA in your proposal is still quite vague (maybe it's related to a known issue), and also that if it is a known issue then we would have seen this before. I'd be interested to see why this started happening recently (this was caught during regression testing, which means it must have worked previously).

jjcoffee avatar Nov 20 '24 17:11 jjcoffee

Pholenk I guess my problem here is that the RCA in your proposal is still quite vague (maybe it's related to a known issue), and also that if it is a known issue then we would have seen this before. I'd be interested to see why this started happening recently (this was caught during regression testing, which means it must have worked previously).

I'm not sure what makes you vague about the RCA in my proposal. The only thing I know is that this issue is there and was found during the regression test, but I don't know what issues came in before the regression test that then made this a regression. If you can find what issues came in before the regression test, you can start tracking down those issues and finding what the root cause is.

Pholenk avatar Nov 23 '24 16:11 Pholenk

@Pholenk The RCA you have implies that this should always have been a problem, but that can't be the case if a regression test was created for it. Your solution may be correct, but it may also be just a workaround, so I'd like to be sure that we're tackling the underlying cause.

I've bumped on Slack to get more eyes on this.

jjcoffee avatar Nov 25 '24 08:11 jjcoffee

@jjcoffee @Christinadobrzyn 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[bot] avatar Nov 26 '24 09:11 melvin-bot[bot]

Hello, I'm Kiryl from Margelo expert agency and I'd like to work on that issue (if @Pholenk is not against, because I can see he was actively involved into solving that problem)

kirillzyusko avatar Nov 26 '24 18:11 kirillzyusko

Thanks @kirillzyusko! I think if we end up using @Pholenk's solution in the end, we can offer partial compensation.

jjcoffee avatar Nov 27 '24 09:11 jjcoffee

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

melvin-bot[bot] avatar Nov 27 '24 16:11 melvin-bot[bot]

@jjcoffee, @Christinadobrzyn Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] avatar Dec 02 '24 09:12 melvin-bot[bot]

@kirillzyusko Any update on this?

jjcoffee avatar Dec 02 '24 09:12 jjcoffee

Hi @kirillzyusko I'm going to add you to this GH for tracking - can you provide an update for us? TY!

Christinadobrzyn avatar Dec 02 '24 17:12 Christinadobrzyn

@jjcoffee no, not yet, sorry. I was busy (and I'm still busy) with resolving some urgent deploy blockers 😔 Hopefully we can squash all of them soon and I'll get back to all issues that I was assigned to!

kirillzyusko avatar Dec 02 '24 18:12 kirillzyusko

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

melvin-bot[bot] avatar Dec 06 '24 09:12 melvin-bot[bot]

Just checking in on this @kirillzyusko - if you have an update on the status of working on this that'd be great! TY!

Christinadobrzyn avatar Dec 06 '24 15:12 Christinadobrzyn