[HOLD for payment 2023-05-16] [$1000] Shortcut modal shortcut bugs on native

[luacmartins]

Coming from #17499 (comment), we'll address:

Issue 5 - Shortcut modal

Android: The keyboard shortcut modal closes immediately after opening it.
iOS Safari: iOS: [Inconsistency issue] Shortcuts to open the keyboard shortcut modal also closes it.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01632c47a9bed2ee73
• Upwork Job ID: 1653408389634400256
• Last Price Increase: 2023-05-02

[MelvinBot]

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

[MelvinBot]

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

[luacmartins]

@azimgd please comment on this issue so I can assign you

[MelvinBot]

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

[MelvinBot]

Current assignee @zanyrenney is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

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

[MelvinBot]

Current assignee @luacmartins is eligible for the External assigner, not assigning anyone new.

[mananjadhav]

@luacmartins Are we going to look at the proposal here or @azimgd is directly going to work on the PR?

[azimgd]

👍 @luacmartins

[MelvinBot]

📣 @azimgd You have been assigned to this job by @luacmartins!
Please apply to this job in Upwork 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 📖

[luacmartins]

@mananjadhav no proposals needed. @azimgd will work on this!

[zanyrenney]

hey @luacmartins looks like you have hired the contributor, and applied the label, so just let me know if there is anything else you need from me.

[luacmartins]

@zanyrenney you should be good for now. Thanks for checking!

[azimgd]

I was only able to reproduce "The keyboard shortcut modal closes immediately after opening it".

@mananjadhav Could you connect a hardware keyboard and try to reproduce both, on production Android please.

[azimgd]

@mananjadhav have you had a chance to take a look?

[mananjadhav]

@azimgd here's what I found when I tested. I waited to post until I was able to record the screencast. Please check my findings. Could you share the details of Android when the modal is immediately closing?

1. Android: The keyboard shortcut modal closes immediately after opening it.

I am not able to reproduce this on the production Android native as well as Chrome. I connected an external keyboard via bluetooth and the keyboard didn't immediately close it.

android-shortcut-modal-trigger.mp4

2. iOS Safari: iOS: [Inconsistency issue] Shortcuts to open the keyboard shortcut modal also closes it.

I went to the linked PR where this was reported and it looks like this is the behavior on all platforms. The above video I am opening and closing the modal with the same shortcut. Also attaching the screencast from the staging web app.

web-shortcuts-modal-trigger.mov

ios-safari-shortcut-modal-trigger.mp4

[azimgd]

PR is ready for review.

[melvin-bot]

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

[melvin-bot]

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

• prevent closing shortcut modal by triggering open shortcut #18484

If no regressions arise, payment will be issued on 2023-05-16. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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:

• [@mananjadhav] The PR that introduced the bug has been identified. Link to the PR:
• [@mananjadhav] 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:
• [@mananjadhav] 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:
• [@mananjadhav] Determine if we should create a regression test for this bug.
• [@mananjadhav] 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.
• [@zanyrenney] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[mananjadhav]

These bugs are a part of the PR #14767 (comment) when we added the support for Keyboard Shortcuts on native apps. I don't think we need to post a message as these were reported in the original PR and moved here.

I don't think we need a regression test suite too. But I am updating a proposal anyway in case others feel it is needed.

Regression Test Suite

1. Open the app in the logged in state
2. Press CMD+J
3. Shortcuts modal should be opened
4. Press CMD+J Shortcuts modal must not be closed
5. Press Escape to close the modal

[mananjadhav]

@luacmartins @zanyrenney Quick bump on the previous comment.

[luacmartins]

I agree that we don't need a regression test either.

[azimgd]

👋 Are we good to go here?

[mananjadhav]

I think we are. Just left with @zanyrenney to take care of the payouts.

[zanyrenney]

Agree with no regression test!

[zanyrenney]

Just speaking to @luacmartins about 1 thing, and then will work on the payouts. Thanks for the patience!

[zanyrenney]

Hey @azimgd i can't find your upwork profile. can you please clarify your name please?

[zanyrenney]

@mananjadhav invited you to the job.

[zanyrenney]

No reporting bonus because report was noticed as part of another PR which we decided was out of scope.

[mananjadhav]

Applied @zanyrenney

[zanyrenney]

Brill - sent you an offer @azimgd

[zanyrenney]

@mananjadhav - paid! just waiting on @azimgd to accept offer then payout on the way!

[azimgd]

@zanyrenney accepted.

[zanyrenney]

All paid to both! thank youu 👍🏼

[azimgd]

@mananjadhav just to confirm, did we manage to get the PR merged in time or was it over 3days?

[mananjadhav]

@zanyrenney I think the timeline bonus would apply here right? We started by May 5, and finished by May 8 including weekends.

[mananjadhav]

@zanyrenney Quick follow up on this one. I think the bonus applies on this one. I just saw the Upwork contract https://www.upwork.com/nx/wm/workroom/33639919/details and it isn't paid here. Can you please help here?

[zanyrenney]

@mananjadhav @azimgd payment of bonus is complete.