[HOLD for payment 2024-05-09] [$500] Hold Request - "Hold" banner is aligned to the top when there is scan request message #37536

lanitochka17 · 2024-02-29T17:44:05Z

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

Version Number: 1.4.45-1
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:

Go to staging.new.expensify.com
Go to 1:1 DM
Create a scan request
Go to request details page
Click 3-dot menu > Hold request
Add a reason and save it

Expected Result:

The "Hold" banner will be aligned in the middle of the given space

Actual Result:

The "Hold" banner is aligned to the top of the given space when there is scan request message

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6397350_1709227408773.20240229_234646.mp4

Bug6397350_1709227408781!Screenshot_2024-03-01_at_01 12 12

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01dd5d1b7c81a77704
Upwork Job ID: 1763306586849918976
Last Price Increase: 2024-03-07
Automatic offers:

neonbhai | Contributor | 0

Issue Owner

Current Issue Owner: @isabelastisser

melvin-bot · 2024-02-29T17:44:18Z

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

lanitochka17 · 2024-02-29T17:45:24Z

We think that this bug might be related to #wave7
CC @RachCHopkins

lanitochka17 · 2024-02-29T17:45:51Z

@isabelastisser 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

gijoe0295 · 2024-02-29T18:08:26Z

Proposal

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

"Hold" banner is aligned to the top when there is scan request message

What is the root cause of that problem?

We only have bottom padding for hold banner as per here. It only works when there's only it and no other banners (money request status bar) above.

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

Add a prop shouldShowPaddingTop to HoldBanner. This check will be true when there's any other status bar above (e.g., MoneyRequestHeaderStatusBar).

<HoldBanner shouldShowPaddingTop={isPending || isScanning} />

In that case, we use styles.pv3 for the HoldBanner. Otherwise, only use pb3.

What alternative solutions did you explore? (Optional)

NA

ghost · 2024-02-29T19:10:11Z

Proposal

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

Hold Request - "Hold" banner is aligned to the top when there is scan request message

What is the root cause of that problem?

Since,the last discussion on this issue i.e. this comment. The solution should be not showing a Hold banner for Scanningrequest. So here :

App/src/components/MoneyRequestHeader.tsx

Line 122 in a04481f

if (!isOnHold && (isRequestIOU || canModifyStatus)) {

we need to add a condition that if the request is scanning till then don't show hold. Also, we have just added a style for padding bottom - pb here :

App/src/components/HoldBanner.tsx

Line 13 in a1a6a74

    
           <View style={[styles.dFlex, styles.flexRow, styles.alignItemsCenter, styles.pb3, styles.ph5, styles.borderBottom]}>

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

Here :

App/src/components/MoneyRequestHeader.tsx

Line 122 in a04481f

if (!isOnHold && (isRequestIOU || canModifyStatus)) {

we need to add a condition to check for !isScanning and also we need to add padding top i.e. pt with the same value as pb here :

App/src/components/HoldBanner.tsx

Line 13 in a1a6a74

    
           <View style={[styles.dFlex, styles.flexRow, styles.alignItemsCenter, styles.pb3, styles.ph5, styles.borderBottom]}>

As per Comment this we need to use these styles mentioned here :

App/src/components/MoneyRequestHeaderStatusBar.tsx

Lines 21 to 28 in a04481f

    
           <View style={[styles.dFlex, styles.flexRow, styles.alignItemsCenter, styles.flexGrow1, styles.overflowHidden, styles.ph5, styles.pb3, borderBottomStyle]}> 
        
               <View style={[styles.moneyRequestHeaderStatusBarBadge]}> 
        
                   <Text style={[styles.textStrong, styles.textMicroBold]}>{title}</Text> 
        
               </View> 
        
               <View style={[styles.flexShrink1]}> 
        
                   <Text style={[styles.textLabelSupporting]}>{description}</Text> 
        
               </View> 
        
           </View>

and add change it here :

App/src/components/HoldBanner.tsx

Lines 13 to 16 in a04481f

    
           <View style={[styles.dFlex, styles.flexRow, styles.alignItemsCenter, styles.pb3, styles.ph5, styles.borderBottom]}> 
        
               <TextPill>{translate('iou.hold')}</TextPill> 
        
               <Text style={[styles.textLabel, styles.pl3, styles.mw100, styles.flexShrink1]}>{translate('iou.requestOnHold')}</Text> 
        
           </View>

To make the Hold request similar to Scanning... request ofcourse with danger background.

Just like moneyRequestHeaderBadge here :

App/src/styles/index.ts

Lines 4134 to 4143 in a04481f

    
           moneyRequestHeaderStatusBarBadge: { 
        
               width: 68, 
        
               height: variables.inputHeightSmall, 
        
               borderRadius: variables.componentBorderRadiusSmall, 
        
               display: 'flex', 
        
               justifyContent: 'center', 
        
               alignItems: 'center', 
        
               backgroundColor: theme.border, 
        
               marginRight: 12, 
        
           },

We need can a new class like holdRequestHeaderBadge, we can copy the entire style but just the background can be changed to backgroundColor: theme.danger

What alternative solutions did you explore? (Optional)

Alternatively, what we can do is
Here :

App/src/components/MoneyRequestHeader.tsx

Line 122 in a04481f

if (!isOnHold && (isRequestIOU || canModifyStatus)) {

we need to add a condition to check for !isScanning and then here :

App/src/components/HoldBanner.tsx

Line 13 in a1a6a74

    
           <View style={[styles.dFlex, styles.flexRow, styles.alignItemsCenter, styles.pb3, styles.ph5, styles.borderBottom]}>

we can add pv3 or padding vertical - 03 and remove padding-bottom or pb completely, which will make the padding equal from top and bottom to adjust the "Hold" banner in the centre.

Results

Screen.Recording.2024-03-06.at.12.33.13.PM.mp4

isabelastisser · 2024-02-29T20:49:43Z

@lanitochka17 I agree that this can barely be noticed. I raised it for discussion in Slack.

melvin-bot · 2024-02-29T20:53:48Z

Job added to Upwork: https://www.upwork.com/jobs/~01dd5d1b7c81a77704

melvin-bot · 2024-02-29T20:53:52Z

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

isabelastisser · 2024-02-29T20:54:32Z

I confirmed this can be handled externally. @Santhosh-Sellavel to review the proposals above. Thanks!

neonbhai · 2024-02-29T20:56:46Z

Proposal

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

"Hold" banner is aligned to the top when there is scan request message

What is the root cause of that problem?

The style for the banner is incorrect

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

We should add styles.pv3 instead of pb3 here:

App/src/components/HoldBanner.tsx

Line 13 in a1a6a74

    
           <View style={[styles.dFlex, styles.flexRow, styles.alignItemsCenter, styles.pb3, styles.ph5, styles.borderBottom]}>

ghost · 2024-02-29T21:15:36Z

This is my proposal.

ghost · 2024-03-01T04:47:39Z

Updated Proposal

askavyblr · 2024-03-01T05:51:41Z

Proposal

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

Hold Request - "Hold" banner is aligned to the top when there is a scan request message

What is the root cause of that problem?

Missing top padding styles.pt3

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

Need to add top padding styles.pt3

App/src/components/HoldBanner.tsx

Line 13 in 48657b4

    
           <View style={[styles.dFlex, styles.flexRow, styles.alignItemsCenter, styles.pb3, styles.ph5, styles.borderBottom]}>

What alternative solutions did you explore? (Optional)

N/A

isabelastisser · 2024-03-01T15:51:07Z

Hi @Santhosh-Sellavel, please review the proposals above. Thanks!

dragnoir · 2024-03-01T16:10:55Z

Proposal

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

A seperator botween Scanning and HOLD

What is the root cause of that problem?

The issues is that the component MoneyRequestHeaderStatusBar for the Scanning, is showing the border bottom when HOLD is diplayed

App/src/components/MoneyRequestHeader.tsx

Lines 180 to 194 in de3a9e1

    
           {isPending && ( 
        
               <MoneyRequestHeaderStatusBar 
        
                   title={translate('iou.pending')} 
        
                   description={translate('iou.transactionPendingText')} 
        
                   shouldShowBorderBottom={!isScanning} 
        
               /> 
        
           )} 
        
           {isScanning && ( 
        
               <MoneyRequestHeaderStatusBar 
        
                   title={translate('iou.receiptStatusTitle')} 
        
                   description={translate('iou.receiptStatusText')} 
        
                   shouldShowBorderBottom 
        
               /> 
        
           )} 
        
           {isOnHold && <HoldBanner />}

as you can see the 1st component for PENDING is showing only the border at the bottom when there's no scanning shouldShowBorderBottom={!isScanning}

{isPending && (
  <MoneyRequestHeaderStatusBar
    title={translate('iou.pending')}
    description={translate('iou.transactionPendingText')}
    shouldShowBorderBottom={!isScanning}
  />
)}

And this give the result below when SCANNING and PENDING are on the screen. (we don't need to ad margin top to SCANNING)

We should follow the same for SCANNING and ONHOLD. The component at the top, SCANNING, should check if ONHOLD is displayed before displaying the border at the bottom

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

1 - Disable the border on the bottom from the Scanning part when the HOLD is diplayed

Same as pending component checking f scanning component is displayed to display the border on the bottom, we should do the same. The logic is that every component should check if any thing below it will be displayed. IF there's a nother part displayed below it, then there's no need to display a border at the bottom.

{!isScanning && (
  <MoneyRequestHeaderStatusBar
    title={translate('iou.receiptStatusTitle')}
    description={translate('iou.receiptStatusText')}
-     shouldShowBorderBottom
+     shouldShowBorderBottom={!isOnHold}
  />
)}

2 - Use the same style for HOLD as SCANNING

As the last request from the design team, to make the HOLD part similar visually as SCANNING and PENDING, we can use the same component MoneyRequestHeaderStatusBar istead of HoldBanner adding a new prop for style (backgroundColor to danger).

{isPending && (
  <MoneyRequestHeaderStatusBar
    title={translate('iou.pending')}
    description={translate('iou.transactionPendingText')}
    shouldShowBorderBottom={!isScanning}
  />
)}

{isScanning && (
  <MoneyRequestHeaderStatusBar
    title={translate('iou.receiptStatusTitle')}
    description={translate('iou.receiptStatusText')}
    shouldShowBorderBottom={!isOnHold}
  />
)}

{isOnHold && (
  <MoneyRequestHeaderStatusBar
    title={translate('iou.hold')}
    description={translate('iou.requestOnHold')}
    shouldShowBorderBottom={isOnHold}
    backgroundColor={themeColors.danger}
  />
)}

POC:

What alternative solutions did you explore? (Optional)

Santhosh-Sellavel · 2024-03-04T18:08:42Z

I've been OOO, will check this in a day

Santhosh-Sellavel · 2024-03-04T18:29:06Z

@Expensify/design Do we need a separator between Scanning & Hold status banner? Or should the hold status banner should be aligned center vertically?

shawnborton · 2024-03-04T18:36:41Z

Whoa! I would think that we only ever show one of these banners at once. cc @trjExpensify @JmillsExpensify

Also, why are the banner styles so different? The font colors and font sizes are way different across the badge and the message next to it. cc @robertjchen - any context there?

JmillsExpensify · 2024-03-04T18:46:40Z

Yeah you shouldn't be able to hold a scanning request. That feels like the solution in my opinion.

melvin-bot · 2024-05-02T12:55:14Z

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

melvin-bot · 2024-05-02T12:55:19Z

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

Fix: Hold request style and visibility #38471

If no regressions arise, payment will be issued on 2024-05-09. 🎊

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

@dragnoir requires payment (Needs manual offer from BZ)
@neonbhai requires payment automatic offer (Contributor)
@Santhosh-Sellavel requires payment through NewDot Manual Requests

melvin-bot · 2024-05-02T12:55:20Z

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:

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

isabelastisser · 2024-05-08T21:34:16Z

Not overdue.

@Santhosh-Sellavel, please complete the BZ list above before I can process the payments in Upwork and close the issue. Thanks!

dragnoir · 2024-05-13T11:14:59Z

@Santhosh-Sellavel friendly bump :)

isabelastisser · 2024-05-13T17:11:56Z

It looks like @Santhosh-Sellavel is OOO until May 15. I've sent them a DM for visibility.

isabelastisser · 2024-05-16T17:20:43Z

Bump @Santhosh-Sellavel

Santhosh-Sellavel · 2024-05-16T17:45:39Z

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:

[@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR: [WAVE 7] Hold Requests: Individual Money Request Page, Reason Interstitial, Banner #33897 (comment)
[@Santhosh-Sellavel] 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: [WAVE 7] Hold Requests: Individual Money Request Page, Reason Interstitial, Banner #33897 (comment)
[@Santhosh-Sellavel] 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: Alignment issue should have been caught while testing.
[@Santhosh-Sellavel] Determine if we should create a regression test for this bug.
Mainly this bug its a minor UI Polish. Also about the bug that Hold option shown while report in scanning state, now its been fixed, it's not the case user might run into easily. cc: @francoisl your thoughts if you disagree
[@Santhosh-Sellavel] 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.
[@isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: N/A

melvin-bot · 2024-05-16T18:02:39Z

@francoisl, @dragnoir, @isabelastisser, @neonbhai, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

francoisl · 2024-05-16T22:20:17Z

Works for me. @isabelastisser feel free to process the payments and close the issue. Thanks!

neonbhai · 2024-05-16T22:27:20Z

Noting that the PR was reviewed by me here but seems like the automation didn't mention it. Please take a look 🙇

cc: @isabelastisser

dragnoir · 2024-05-17T07:14:24Z

@isabelastisser pls consider this when payment #37536 (comment)

Thank you

isabelastisser · 2024-05-17T20:06:53Z

Based on @shawnborton's comment here, I'm increasing the payment to @dragnoir to $750.

Payments summary:

@dragnoir requires payment $750 (manual offer https://www.upwork.com/nx/wm/offer/102358820)
@neonbhai requires payment automatic offer (Contributor) $750
@Santhosh-Sellavel requires payment through NewDot Manual Requests $500 PENDING

isabelastisser · 2024-05-17T20:06:58Z

All set!

dragnoir · 2024-05-17T20:10:17Z

@isabelastisser offer accepted. Thank you

neonbhai · 2024-05-17T20:13:18Z

@isabelastisser hi, the original job was split with me and @dragnoir. As noted here #37536 (comment)

I also ended up reviewing one of the linked PRs, so the payout would be $250 + $500 for me! Please take a look 🙇

Santhosh-Sellavel · 2024-05-22T18:14:53Z

Requested on ND

JmillsExpensify · 2024-05-31T04:32:44Z

$500 approved for @Santhosh-Sellavel

lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 29, 2024

melvin-bot bot assigned isabelastisser Feb 29, 2024

isabelastisser added External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Feb 29, 2024

melvin-bot bot changed the title ~~Hold Request - "Hold" banner is aligned to the top when there is scan request message~~ [$500] Hold Request - "Hold" banner is aligned to the top when there is scan request message Feb 29, 2024

melvin-bot bot assigned Santhosh-Sellavel Feb 29, 2024

isabelastisser closed this as completed Feb 29, 2024

isabelastisser reopened this Feb 29, 2024

melvin-bot bot added the Overdue label Mar 4, 2024

melvin-bot bot removed the Overdue label Mar 4, 2024

melvin-bot bot changed the title ~~[$500] Hold Request - "Hold" banner is aligned to the top when there is scan request message~~ [HOLD for payment 2024-05-09] [$500] Hold Request - "Hold" banner is aligned to the top when there is scan request message May 2, 2024

melvin-bot bot removed the Reviewing Has a PR in review label May 2, 2024

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels May 8, 2024

melvin-bot bot added Overdue and removed Overdue labels May 8, 2024

melvin-bot bot added Overdue and removed Overdue labels May 13, 2024

Santhosh-Sellavel mentioned this issue May 16, 2024

[WAVE 7] Hold Requests: Individual Money Request Page, Reason Interstitial, Banner #33897

Merged

49 tasks

melvin-bot bot removed the Overdue label May 17, 2024

isabelastisser closed this as completed May 17, 2024

[HOLD for payment 2024-05-09] [$500] Hold Request - "Hold" banner is aligned to the top when there is scan request message #37536

[HOLD for payment 2024-05-09] [$500] Hold Request - "Hold" banner is aligned to the top when there is scan request message #37536

Comments

lanitochka17 commented Feb 29, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Feb 29, 2024

lanitochka17 commented Feb 29, 2024

lanitochka17 commented Feb 29, 2024

gijoe0295 commented Feb 29, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

ghost commented Feb 29, 2024 • edited by ghost Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Results

isabelastisser commented Feb 29, 2024

melvin-bot bot commented Feb 29, 2024

melvin-bot bot commented Feb 29, 2024

isabelastisser commented Feb 29, 2024

neonbhai commented Feb 29, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

ghost commented Feb 29, 2024

ghost commented Mar 1, 2024

askavyblr commented Mar 1, 2024

Proposal

isabelastisser commented Mar 1, 2024

dragnoir commented Mar 1, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

1 - Disable the border on the bottom from the Scanning part when the HOLD is diplayed

2 - Use the same style for HOLD as SCANNING

What alternative solutions did you explore? (Optional)

Santhosh-Sellavel commented Mar 4, 2024

Santhosh-Sellavel commented Mar 4, 2024

shawnborton commented Mar 4, 2024

JmillsExpensify commented Mar 4, 2024

melvin-bot bot commented May 2, 2024

melvin-bot bot commented May 2, 2024

melvin-bot bot commented May 2, 2024

isabelastisser commented May 8, 2024

dragnoir commented May 13, 2024

isabelastisser commented May 13, 2024

isabelastisser commented May 16, 2024

Santhosh-Sellavel commented May 16, 2024 • edited by francoisl Loading

melvin-bot bot commented May 16, 2024

francoisl commented May 16, 2024

neonbhai commented May 16, 2024

dragnoir commented May 17, 2024

isabelastisser commented May 17, 2024 • edited Loading

isabelastisser commented May 17, 2024

dragnoir commented May 17, 2024

neonbhai commented May 17, 2024

Santhosh-Sellavel commented May 22, 2024

JmillsExpensify commented May 31, 2024

lanitochka17 commented Feb 29, 2024 •

edited by melvin-bot bot

Loading

gijoe0295 commented Feb 29, 2024 •

edited

Loading

ghost commented Feb 29, 2024 •

edited by ghost

Loading

neonbhai commented Feb 29, 2024 •

edited

Loading

dragnoir commented Mar 1, 2024 •

edited

Loading

Santhosh-Sellavel commented May 16, 2024 •

edited by francoisl

Loading

isabelastisser commented May 17, 2024 •

edited

Loading