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

[HOLD for payment 2024-08-02] [HOLD for payment 2024-07-25] [$500] Web - Expense - Blue border selection on back icon RHP after hitting enter on BNP #43662

Closed
2 of 6 tasks
lanitochka17 opened this issue Jun 13, 2024 · 66 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jun 13, 2024

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.83-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to chat
  3. Go to + > Submit expense > Manual
  4. Enter amount
  5. Hit Enter

Expected Result:

The back icon on RHP will not have blue border selection

Actual Result:

The back icon on RHP has blue border selection

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

Bug6511961_1718284382905.enter.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~018358d1ec7f51e120
  • Upwork Job ID: 1801251133562170195
  • Last Price Increase: 2024-06-27
  • Automatic offers:
    • suneox | Contributor | 102988636
Issue OwnerCurrent Issue Owner: @s77rt
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Jun 13, 2024
Copy link

melvin-bot bot commented Jun 13, 2024

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

Copy link
Contributor

👋 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:

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

@lanitochka17
Copy link
Author

@danieldoglas FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@lanitochka17
Copy link
Author

We think that this bug might be related to #vip-vsp

@danieldoglas danieldoglas removed the DeployBlocker Indicates it should block deploying the API label Jun 13, 2024
@danieldoglas
Copy link
Contributor

Not a deploy blocker related to web, removing the label

@danieldoglas danieldoglas added Daily KSv2 Hourly KSv2 External Added to denote the issue can be worked on by a contributor and removed Hourly KSv2 DeployBlockerCash This issue or pull request should block deployment Daily KSv2 labels Jun 13, 2024
@melvin-bot melvin-bot bot changed the title Web - Expense - Blue border selection on back icon RHP after hitting enter on BNP [$250] Web - Expense - Blue border selection on back icon RHP after hitting enter on BNP Jun 13, 2024
Copy link

melvin-bot bot commented Jun 13, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 13, 2024
Copy link

melvin-bot bot commented Jun 13, 2024

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

@luacmartins
Copy link
Contributor

luacmartins commented Jun 13, 2024

This was likely caused by #39520

@gawandeabhishek
Copy link

To address this issue, we can use the .that-element:focus { outline: none; } CSS class. This rule removes the default focus outline that appears on elements when they are focused. This can enhance the visual appearance and usability of the element, especially in cases where the default browser outline is intrusive or unnecessary.

@tsa321
Copy link
Contributor

tsa321 commented Jun 14, 2024

Proposal

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

Web - Expense - Blue border selection on back icon RHP after hitting enter on BNP

What is the root cause of that problem?

The SCREENS.MONEY_REQUEST.STEP_PARTICIPANTS screen isn't included in

const SCREENS_WITH_AUTOFOCUS: string[] = [

which will make focusTrap to focus the first element of the screen:

if (SCREENS_WITH_AUTOFOCUS.includes(activeRouteName)) {
return false;
}
return undefined;

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

include SCREENS.MONEY_REQUEST.STEP_PARTICIPANTS in SCREENS_WITH_AUTOFOCUS.
And if we need to disable the auto focus on all money request screen we can use ...Object.values(SCREENS.MONEY_REQUEST)

Alternative solutions

We can disable the initialFocus of FocusTrap. We set false in initialFocus for all screens.

and if there is problem with other screen element is still focused and receive key event we can set onActivate property:
The code that work for me is to move the focus to other element that doesn't receive keydown event:

onActivate: () => document?.querySelector('[tabindex="0"]').focus()

This could fix that issue.

Alternative solutions (2)

We can defer the blue outline appearance so it will be shown only when it is focused after some milli seconds. By using this method we can remove the initialFocus property of FocusTrap in FocusTrapForScreen.

The code could be:
In FocusTrapForScreen we add a ref:
const screenRef = useRef(null);

then add the screenRef in children:
const childElement = React.cloneElement(children, {ref: screenRef});

then onActivate will be:

onActivate: () => {
    const firstTabElement = screenRef?.current?.querySelector('[tabindex="0"]');
    if (firstTabElement) {
        const elementBoxShadow = firstTabElement.style.boxShadow;
        firstTabElement.style.boxShadow = "none";
        setTimeout(() => {
            firstTabElement.style.boxShadow = elementBoxShadow;
        }, 300);
    }
},

then change the returned children with childElement

@s77rt
Copy link
Contributor

s77rt commented Jun 14, 2024

@gawandeabhishek Please checkout the contributing guide https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jul 18, 2024
@melvin-bot melvin-bot bot changed the title [$500] Web - Expense - Blue border selection on back icon RHP after hitting enter on BNP [HOLD for payment 2024-07-25] [$500] Web - Expense - Blue border selection on back icon RHP after hitting enter on BNP Jul 18, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 18, 2024
Copy link

melvin-bot bot commented Jul 18, 2024

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

Copy link

melvin-bot bot commented Jul 18, 2024

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

If no regressions arise, payment will be issued on 2024-07-25. 🎊

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 24, 2024
Copy link

melvin-bot bot commented Jul 25, 2024

Issue is ready for payment but no BZ is assigned. @joekaufmanexpensify you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

@joekaufmanexpensify
Copy link
Contributor

joekaufmanexpensify commented Jul 25, 2024

All set to issue payment. We need to pay:

  • @suneox - $250 for PR via Upwork (50% reduction because of regression)
  • @s77rt - $250 for C+ via NewDot (50% reduction because of regression).

@joekaufmanexpensify
Copy link
Contributor

@suneox $500 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

Upwork job closed.

@joekaufmanexpensify
Copy link
Contributor

All that's left is for @s77rt to request their $500 via NewDot. Going to close this one since it's otherwise all set. Thanks everyone!

@s77rt
Copy link
Contributor

s77rt commented Jul 26, 2024

@joekaufmanexpensify This should be $250 for causing a regression #45571

@JmillsExpensify
Copy link

@joekaufmanexpensify mind updating the payment summary above?

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jul 26, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-07-25] [$500] Web - Expense - Blue border selection on back icon RHP after hitting enter on BNP [HOLD for payment 2024-08-02] [HOLD for payment 2024-07-25] [$500] Web - Expense - Blue border selection on back icon RHP after hitting enter on BNP Jul 26, 2024
Copy link

melvin-bot bot commented Jul 26, 2024

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

If no regressions arise, payment will be issued on 2024-08-02. 🎊

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

@joekaufmanexpensify
Copy link
Contributor

Ah okay got it. I don't see that there was a post to the issue about the deploy blocker/regression previously, which is why it was not previously noted in the payment summary.

Updated payment summary to note that both parties are actually due $250 here because of the regression.

@joekaufmanexpensify
Copy link
Contributor

I need to request a $250 refund from @suneox via Upwork. Getting an insufficient permissions error in upwork requesting a refund. Updating my permissions via this SO to allow processing refunds.

@suneox
Copy link
Contributor

suneox commented Jul 26, 2024

I need to request a $250 refund from @suneox via Upwork. Getting an insufficient permissions error in upwork requesting a refund. Updating my permissions via this SO to allow processing refunds.

Sure, I'm looking for the refund section on Upwork and process ASAP

@joekaufmanexpensify
Copy link
Contributor

All good!I haven't requested it yet. Will let you know once I do!

@suneox
Copy link
Contributor

suneox commented Jul 26, 2024

@joekaufmanexpensify I have refunded 50%

@joekaufmanexpensify
Copy link
Contributor

Ah, perfect. TY! I see it processed on my end.

@joekaufmanexpensify
Copy link
Contributor

Okay, now all that's left is to process $250 payment to @s77rt. Payment summary here.

@JmillsExpensify
Copy link

$250 approved for @s77rt

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

No branches or pull requests

9 participants