App icon indicating copy to clipboard operation
App copied to clipboard

Closing right click menu with esc on request/send/cancel money request messages adds blue border around the message

Open kavimuru opened this issue 1 year ago • 5 comments

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Open the app
  2. Open any report
  3. On any request/send/cancel money request message, right click to open mark as unread and reaction popup
  4. Click on esc to close the popup and observe that blue border is added around the message
  5. Try step 3 and 4 on normal message and observe that it does not add blue border

Expected Result:

App shouldn't add blue border around any message on using esc to close the right click popup menu

Actual Result:

App adds blue border around request/send/cancel money request message when we use esc to close right click menu for that message

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • [ ] Android / native
  • [ ] Android / Chrome
  • [ ] iOS / native
  • [ ] iOS / Safari
  • [x] MacOS / Chrome / Safari
  • [ ] MacOS / Desktop

Version Number: 1.3.6 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 Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/43996225/234728407-b6fdcd66-76eb-46fb-81d5-c5780a6daa83.mp4

https://user-images.githubusercontent.com/43996225/234728417-821b9cf3-4ad0-49bf-8812-0b4ec49e15f8.mp4

Expensify/Expensify Issue URL: Issue reported by: @dhanashree-sawant Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1682523637828939

View all open jobs on GitHub

kavimuru avatar Apr 27 '23 00:04 kavimuru

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

MelvinBot avatar Apr 27 '23 00:04 MelvinBot

Bug0 Triage Checklist (Main S/O)

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

MelvinBot avatar Apr 27 '23 00:04 MelvinBot

Yep, I can replicate on v36:

https://user-images.githubusercontent.com/30815269/234737161-fee07fde-c1fa-4f29-b4c0-f453dd1498c6.mp4

jliexpensify avatar Apr 27 '23 01:04 jliexpensify

Just awaiting C+'s response in the other issue to see if these issues are linked/similar.

jliexpensify avatar Apr 27 '23 01:04 jliexpensify

Proposal

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

Closing right click menu with esc on request/send/cancel money request messages adds blue border around the message

What is the root cause of that problem?

This is dupe with so many issue posts dating to at least 2022 from what I can see and they keep getting reported over and over when it's one issue but people reporting each issue are unaware of that so I will propose a solution for the entire issue.

Let's start by clarifying that the issue is happening whenever a Pressable element is right clicked then the esc key is pressed, for most elements that will require a double press of esc to show the outline, the point is that the browser will focus on the element and when it does, it will show the outline as I will demonstrate in the below screen recording:

https://user-images.githubusercontent.com/74202751/234929124-9334baf5-464f-4a86-947b-63eae7189756.mov

Please keep in mind that if you want to replicate the issue for elements outside of the currently open report, you must double press esc to get the outline. again the point is that the browser should focus the element.

Now for the root cause for these issues, it's because for some reason we are applying box-shadow color on focus in the web's index.html:

https://github.com/Expensify/App/blob/41eec3c513ea0cb1c433091692ea289e588040e0/web/index.html#L48-L51

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

We can set the focus behaviour to none for the box-shadow

style={[
    :focus-visible {
        outline: 0;
        box-shadow: none;
    }

Result

https://user-images.githubusercontent.com/74202751/234931100-411591d4-399a-4415-a2c6-4fdd12c57b6a.mov

AmjedNazzal avatar Apr 27 '23 16:04 AmjedNazzal

@jliexpensify This seems related to #17419 , issue is same problem occurring at different places!

Santhosh-Sellavel avatar Apr 27 '23 19:04 Santhosh-Sellavel

@AmjedNazzal We need to outline when we intentionally focus on, changing there will break every where!

Santhosh-Sellavel avatar Apr 27 '23 19:04 Santhosh-Sellavel

@jliexpensify Let's add hold for now!

Santhosh-Sellavel avatar Apr 27 '23 19:04 Santhosh-Sellavel

@Santhosh-Sellavel - added HOLD, let me know when it should be taken off hold and if you want to also be assigned this one.

jliexpensify avatar Apr 27 '23 22:04 jliexpensify

@jliexpensify Huh... This is 4 days overdue. Who can take care of this?

MelvinBot avatar May 03 '23 20:05 MelvinBot

This is on Hold

jliexpensify avatar May 04 '23 00:05 jliexpensify

So far pretty much everyone in this thread agreed there's nothing "wrong" here so let's do nothing 👍

Beamanator avatar May 12 '23 11:05 Beamanator