[HOLD for payment 2024-09-10] [$250] iOS - Composer - Add attachment modal is hidden behind the keyboard

IuliiaHerets commented 1 week ago

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: 9.0.28-0 Reproducible in staging?: Y Reproducible in production?: N Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Launch New Expensify app
Go to any chat
Focus on the composer (important)
Tap +
Tap Add attachment

Expected Result:

Add attachment modal will not be hidden behind the keyboard

Actual Result:

Add attachment modal is hidden behind the keyboard

Workaround:

Unknown

Platforms:

[ ] Android: Native
[ ] Android: mWeb Chrome
[x] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/dcc06c05-1d90-4a66-a5bc-bd1d16108a74

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01929e53bf6e5a45ee
Upwork Job ID: 1831044805586591004
Last Price Increase: 2024-09-03
Automatic offers:

ishpaul777 | Contributor | 103803636

Issue Owner

Current Issue Owner: @stephanieelliott

melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @stephanieelliott (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] commented 1 week ago

Triggered auto assignment to @danieldoglas (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

github-actions[bot] commented 1 week ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

danieldoglas commented 1 week ago

It looks like a glitch; I can't simulate that after several tries.

https://github.com/user-attachments/assets/8d2fd8ea-f754-4a1c-8750-8cd6248e97e4

roryabraham commented 1 week ago

@danieldoglas looks like you're testing on iOS web, not iOS native

melvin-bot[bot] commented 1 week ago

Current assignee @stephanieelliott is eligible for the Bug assigner, not assigning anyone new.

melvin-bot[bot] commented 1 week ago

Current assignee @danieldoglas is eligible for the DeployBlockerCash assigner, not assigning anyone new.

github-actions[bot] commented 1 week ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

danieldoglas commented 1 week ago

This is not happening in production, so we need to check what happened.

ishpaul777 commented 1 week ago

i found the Root cause :

Here when we close all modals https://github.com/Expensify/App/blob/227aa1372a342b27493dc08de627fe23e95c1d76/src/components/PopoverMenu.tsx#L143-L146 this onClose is triggered https://github.com/Expensify/App/blob/227aa1372a342b27493dc08de627fe23e95c1d76/src/components/PopoverMenu.tsx#L232 which triggers this which triggers restoreKeyboardState here https://github.com/Expensify/App/blob/227aa1372a342b27493dc08de627fe23e95c1d76/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.tsx#L312

Solution: we have this case covered before in onItemSelected, now its just not called before the modal close https://github.com/Expensify/App/blob/227aa1372a342b27493dc08de627fe23e95c1d76/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx#L273-L275

so we have to modify this

https://github.com/Expensify/App/blob/227aa1372a342b27493dc08de627fe23e95c1d76/src/components/PopoverMenu.tsx#L143-L146

to

            onItemSelected(selectedItem, index);
            Modal.close(() => {
                selectedItem.onSelected?.();
            });

so onItemSelected is called before we close modal just like before

danieldoglas commented 1 week ago

@ishpaul777 can you spin up a PR for that?

melvin-bot[bot] commented 1 week ago

📣 @ishpaul777 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

situchan commented 1 week ago

Looks like regression from https://github.com/Expensify/App/pull/47991

roryabraham commented 1 week ago

cherry-picking the fix here: https://github.com/Expensify/App/actions/runs/10691504641

roryabraham commented 1 week ago

Verified this is fixed on staging ✅

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

https://github.com/Expensify/App/pull/48510

If no regressions arise, payment will be issued on 2024-09-10. :confetti_ball:

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

@parasharrajat requires payment through NewDot Manual Requests
@ishpaul777 requires payment automatic offer (Contributor)

melvin-bot[bot] commented 1 week ago

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[ ] [@parasharrajat / @ishpaul777] The PR that introduced the bug has been identified. Link to the PR:
[ ] [@parasharrajat / @ishpaul777] 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:
[ ] [@parasharrajat / @ishpaul777] A discussion in #expensify-bugs 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:
[ ] [@parasharrajat / @ishpaul777] Determine if we should create a regression test for this bug.
[ ] [@parasharrajat / @ishpaul777] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[ ] [@stephanieelliott] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

danieldoglas commented 1 week ago

Removing @parasharrajat

stephanieelliott commented 5 days ago

Summarizing payment on this issue:

Contributor: @ishpaul777 $250 via Upwork -- PAID
Contributor+: n/a internal

Upwork job is here: https://www.upwork.com/jobs/~01929e53bf6e5a45ee

Expensify / App