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

[Awaiting Test from Applause] [$500] Android - Compose-In edit comment mode, compose box is shown after tapping header and returning back. #33466

Closed
1 of 6 tasks
lanitochka17 opened this issue Dec 21, 2023 · 43 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Dec 21, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 1.4.15-5
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:

Action Performed:

  1. Launch app
  2. Tap on a report
  3. Long press any sent message and select edit comment
  4. Note, compose box is not visible
  5. Tap header
  6. Tap back
  7. Note compose box is visible
  8. Send a message

Expected Result:

When user in edit comment mode, compose box should not be shown after tapping header and returning back

Actual Result:

When user in edit comment mode, compose box shown after tapping header and returning back

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

Bug6323208_1703193415742.pppp.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019035bbdbdc062b24
  • Upwork Job ID: 1737949296231571456
  • Last Price Increase: 2023-12-21
  • Automatic offers:
    • tienifr | Contributor | 28094075
Issue OwnerCurrent Issue Owner: @garrettmknight
@lanitochka17 lanitochka17 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 Dec 21, 2023
@melvin-bot melvin-bot bot changed the title Android - Compose-In edit comment mode, compose box is shown after tapping header and returning back. [$500] Android - Compose-In edit comment mode, compose box is shown after tapping header and returning back. Dec 21, 2023
Copy link

melvin-bot bot commented Dec 21, 2023

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

Copy link

melvin-bot bot commented Dec 21, 2023

Triggered auto assignment to @garrettmknight (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 Dec 21, 2023
Copy link

melvin-bot bot commented Dec 21, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • 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
  • 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.
  • 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.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

Copy link

melvin-bot bot commented Dec 21, 2023

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

@ikevin127
Copy link
Contributor

ikevin127 commented Dec 21, 2023

Proposal

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

When user is in edit comment mode, the main composer is shown after tapping header and returning back.
The issue happens on both native platforms (Android / iOS).

What is the root cause of that problem?

TLDR: As soon as the edit composer loses its focus, the main composer is being shown. When the edit composer re-gains its focus the main composer is hidden.

When we go to a different screen while the edit composer is focused, the composer loses focus which causes the main composer to be shown. If we manually re-focus the edit composer, the main composer will be hidden.

This issue happens because within ReportActionItemMessageEdit.js there's no logic that focuses the edit composer input when returning back from another screen.

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

Similar to the ComposerWithSuggestions.js component, we should add logic that focuses the edit composer input when returning from another screen to the screen where we are in edit comment mode.

Code changes

    /**
     * Focus the composer text input
     */
    const focus = focusComposerWithDelay(textInputRef.current);

    // ^ - existing code

    // new code:

    useEffect(() => {
        const unsubscribeFocus = navigation.addListener('focus', () => focus());
        return () => unsubscribeFocus();
    }, [navigation, focus]);

Videos

iOS: Native
Screen.Recording.2023-12-22.at.01.16.36.mov
Android: Native
Screen.Recording.2023-12-22.at.01.11.30.mov

@tienifr
Copy link
Contributor

tienifr commented Dec 22, 2023

Proposal

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

Compose-In edit comment mode, compose box is shown after tapping header and returning back

What is the root cause of that problem?

When users focus on the edit composer, the main composer will be hidden. If they blur the edit composer, both composer will be shown. And we don't have the logic to refocus on the input if users open other page and come back

-> That why we can see 2 composers

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

Focusing on the edit composer whenever users back to report screen doesn't make sense if this composer is not focused before

We should detect if the composer is blurred when the page is not focus, so when users back to this page, we can automatically focus again

In Composer/index.ios.js

    const isFocused = useIsFocused();
    const shouldResetFocus = useRef(false);

    useEffect(()=>{
        if(!isFocused || !shouldResetFocus.current){
            return;
        }
        textInput.current?.focus(); // focus input again
        shouldResetFocus.current=false
    },[isFocused])

<RNTextInput

...
            onBlur={()=>{
                if(!isFocused){
                    shouldResetFocus.current=true // detect the input is blurred when the page is hidden
                }
                props.onBlur()
            }}

Apply the same logic in index.android.js. To avoid duplicated code we can create the new hook for that.

What alternative solutions did you explore? (Optional)

We can apply the above logic in ReportActionItemMessageEdit (edit composer) and ComposerWithSuggestions (main composer)

Result

Screen.Recording.2023-12-22.at.11.23.08.mov

@allroundexperts
Copy link
Contributor

Taking over!

@allroundexperts
Copy link
Contributor

Hi @tienifr!

Focusing on the edit composer whenever users back to report screen doesn't make sense if this composer is not focused before

Can you show me a video of this happening?

@melvin-bot melvin-bot bot added the Overdue label Dec 24, 2023
@tienifr
Copy link
Contributor

tienifr commented Dec 25, 2023

@allroundexperts here you are

Screen.Recording.2023-12-25.at.10.46.51.mov

Copy link

melvin-bot bot commented Dec 25, 2023

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

@tienifr
Copy link
Contributor

tienifr commented Dec 27, 2023

@allroundexperts what do you think ^?

Copy link

melvin-bot bot commented Dec 27, 2023

@garrettmknight, @narefyev91 Eep! 4 days overdue now. Issues have feelings too...

@garrettmknight
Copy link
Contributor

Updating C+

@melvin-bot melvin-bot bot removed the Overdue label Dec 27, 2023
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 27, 2023
@garrettmknight
Copy link
Contributor

@allroundexperts thoughts on video?

@melvin-bot melvin-bot bot added the Overdue label Jan 1, 2024
Copy link

melvin-bot bot commented Jan 1, 2024

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

@garrettmknight
Copy link
Contributor

@allroundexperts can you give this one an update when you get a chance?

@melvin-bot melvin-bot bot removed the Overdue label Jan 2, 2024
@garrettmknight garrettmknight added Weekly KSv2 and removed Daily KSv2 labels Feb 8, 2024
@garrettmknight
Copy link
Contributor

Dropping to weekly while we figure it out and @allroundexperts submits for payment

@parasharrajat
Copy link
Member

@garrettmknight Unfortunately, I don't have the steps handy but QA team could help us get those quickly.

@melvin-bot melvin-bot bot added the Overdue label Feb 16, 2024
@MonilBhavsar
Copy link
Contributor

Looks like we're still waiting on QA steps here @parasharrajat ?

@melvin-bot melvin-bot bot removed the Overdue label Feb 21, 2024
@parasharrajat
Copy link
Member

@MonilBhavsar as I said earlier, it would be great if someone familiar like QA team could get the related regression steps. I don't know how to get the exact ones.

@MonilBhavsar
Copy link
Contributor

I checked Testrail but couldn't find exact steps for related Composer test. @lanitochka17 can you please link to the regression test suite if this issue is coming from regression run. thank you!

@JmillsExpensify
Copy link

$500 approved for @allroundexperts based on this summary.

@melvin-bot melvin-bot bot added the Overdue label Mar 15, 2024
@garrettmknight
Copy link
Contributor

@lanitochka17 I know it's been a while since we opened this one up, but could you point us to the test that triggered this one if you can find it? Thanks!

@melvin-bot melvin-bot bot removed the Overdue label Mar 15, 2024
@garrettmknight garrettmknight changed the title [HOLD for payment 2024-02-05] [$500] Android - Compose-In edit comment mode, compose box is shown after tapping header and returning back. [Awaiting Test from Applause] [$500] Android - Compose-In edit comment mode, compose box is shown after tapping header and returning back. Mar 15, 2024
@lanitochka17
Copy link
Author

The issue was found while checking this test case https://expensify.testrail.io/index.php?/tests/view/4074970

@garrettmknight garrettmknight added Daily KSv2 and removed Weekly KSv2 labels Mar 22, 2024
@melvin-bot melvin-bot bot added the Overdue label Mar 25, 2024
@garrettmknight
Copy link
Contributor

Cool, I think this once is covered in the existing test case.

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. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants