[$1000] Dev - "Pay with Expensify" button style is broken

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Login with account that has payWithExpensify, expensifyWallet beta enabled or able to pay via PayPal
2. Tap FAB menu -> Send money
3. Select currency as USD
4. Input any amount
5. Select any contact

Expected Result:

No white border around arrow on "Pay with Expensify" button

Actual Result:

There's white border around arrow on "Pay with Expensify" button

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.2.41-1
Reproducible in staging?: Need reproduction
Reproducible in production?: Need reproduction
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Expensify/Expensify Issue URL:
Issue reported by: @0xmiroslav
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1671442540501379

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e2c7bdef927162e8
• Upwork Job ID: 1604912631198826496
• Last Price Increase: 2022-12-19

[puneetlath]

What I'm seeing is slightly different. I see a white line between the button and the down arrow (on staging web, staging desktop, and iOS).

@Expensify/design can you confirm whether what I'm seeing is correct or a bug?

[puneetlath]

Ah looks like it's on main actually, but not yet on staging. Adding deploy blocker.

[OSBotify]

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

[grgia]

Looks like it was caused by this PR #13377

[grgia]

Screen.Recording.2022-12-19.at.10.06.50.AM.mov

cc @shawnborton @marcaaron
I have a fix to ignore the rounded border styles for this type of button so the other buttons are unaffected. Any thoughts on what this should look like or is this okay for this button?

[melvin-bot]

Triggered auto assignment to @flaviadefaria (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[grgia]

@0xmiroslav Thank you, I do prefer this method! I have posted a PR using your suggestions that I am testing to get merged today. Because I am using your solution, we will make sure you get compensated for it.
cc @flaviadefaria

[0xmiroslav]

@grgia I have another suggestion: (to fix divider involved in press highlight)

• Button component changes are the same as above

• In ButtonWithDropdown component:

        <Button
            success
            onPress={props.onButtonPress}
            text={props.buttonText}
            isDisabled={props.isDisabled}
            isLoading={props.isLoading}
            shouldRemoveRightBorderRadius
-           style={[styles.flex1]}
+           style={[styles.flex1, styles.pr0]}
            pressOnEnter
        />
+       <View style={styles.buttonDivider} />
        <Button
            success
            isDisabled={props.isDisabled}
-           style={[styles.buttonDropdown]}
+           style={[styles.pl0]}
            onPress={props.onDropdownPress}
            shouldRemoveLeftBorderRadius
            ContentComponent={() => (
                <Icon src={Expensicons.DownArrow} fill={themeColors.textLight} />
            )}
        />

Define this in styles.js

    buttonDivider: {
        width: 1,
        alignSelf: 'stretch',
        backgroundColor: themeColors.textLight,
        marginVertical: 1,
    },

[s77rt]

Proposal

diff --git a/src/components/Button.js b/src/components/Button.js
index d14d6db5b3..281ee65860 100644
--- a/src/components/Button.js
+++ b/src/components/Button.js
@@ -257,6 +257,8 @@ class Button extends Component {
                     this.props.isDisabled ? {...styles.cursorDisabled, ...styles.noSelect} : {},
                     ...StyleUtils.parseStyleAsArray(this.props.style),
                     styles.buttonContainer,
+                    this.props.shouldRemoveRightBorderRadius ? styles.noRightBorderRadius : undefined,
+                    this.props.shouldRemoveLeftBorderRadius ? styles.noLeftBorderRadius : undefined,
                 ]}
                 nativeID={this.props.nativeID}
             >
@@ -277,8 +279,6 @@ class Button extends Component {
                                 (this.props.isDisabled && !this.props.danger && !this.props.success) ? styles.buttonDisable : undefined,
                                 (this.props.success && activeAndHovered) ? styles.buttonSuccessHovered : undefined,
                                 (this.props.danger && activeAndHovered) ? styles.buttonDangerHovered : undefined,
-                                this.props.shouldRemoveRightBorderRadius ? styles.noRightBorderRadius : undefined,
-                                this.props.shouldRemoveLeftBorderRadius ? styles.noLeftBorderRadius : undefined,
                                 ...this.props.innerStyles,
                             ]}
                         >
diff --git a/src/styles/styles.js b/src/styles/styles.js
index 27195c89ac..6438357733 100644
--- a/src/styles/styles.js
+++ b/src/styles/styles.js
@@ -343,16 +343,15 @@ const styles = {
     },
 
     button: {
-        backgroundColor: themeColors.buttonDefaultBG,
-        borderRadius: variables.buttonBorderRadius,
         height: variables.componentSizeLarge,
         justifyContent: 'center',
         ...spacing.ph3,
     },
 
     buttonContainer: {
-        padding: 1,
+        backgroundColor: themeColors.buttonDefaultBG,
         borderRadius: variables.buttonBorderRadius,
+        overflow: 'hidden',
     },
 
     buttonText: {
@@ -368,32 +367,26 @@ const styles = {
     },
 
     buttonSmall: {
-        borderRadius: variables.buttonBorderRadius,
         height: variables.componentSizeSmall,
         paddingTop: 4,
         paddingHorizontal: 14,
         paddingBottom: 4,
-        backgroundColor: themeColors.buttonDefaultBG,
     },
 
     buttonMedium: {
-        borderRadius: variables.buttonBorderRadius,
         height: variables.componentSizeNormal,
         paddingTop: 12,
         paddingRight: 16,
         paddingBottom: 12,
         paddingLeft: 16,
-        backgroundColor: themeColors.buttonDefaultBG,
     },
 
     buttonLarge: {
-        borderRadius: variables.buttonBorderRadius,
         height: variables.componentSizeLarge,
         paddingTop: 8,
         paddingRight: 10,
         paddingBottom: 8,
         paddingLeft: 18,
-        backgroundColor: themeColors.buttonDefaultBG,
     },
 
     buttonSmallText: {
diff --git a/web/index.html b/web/index.html
index d7074a03d3..a208a7c9f9 100644
--- a/web/index.html
+++ b/web/index.html
@@ -35,11 +35,11 @@
         }
         :focus-visible {
             outline: 0;
-            box-shadow: inset 0px 0px 0px 1px #5AB0FF;
+            box-shadow: 0px 0px 0px 1px #5AB0FF;
         }
         :focus[data-focusvisible-polyfill] {
             outline: 0;
-            box-shadow: inset 0px 0px 0px 1px #5AB0FF;
+            box-shadow: 0px 0px 0px 1px #5AB0FF;
         }
         input:focus-visible, input:focus[data-focusvisible-polyfill],
         select:focus-visible, select:focus[data-focusvisible-polyfill]  {

Sorry I'm a little late, i started working on this before dropping the Help Wanted label

Details

• There is no need to apply the same style twice
• I have moved the border logic to the button container
• I have moved some duplicated styles to the button container
• I have changed the box-shadow to be outset not inset because box-shadow is replacing the outline prop
• I have removed the weird hack of setting padding to 1

[s77rt]

Heads up!

If you are going to apply my proposal above, first apply this css fix:

diff --git a/src/styles/styles.js b/src/styles/styles.js
index 27195c89ac..a21ac19f4d 100644
--- a/src/styles/styles.js
+++ b/src/styles/styles.js
@@ -466,7 +466,7 @@ const styles = {
     },
 
     buttonCTA: {
-        paddingVertical: 6,
+        marginVertical: 6,
         ...spacing.mh4,
     },

[grgia]

@s77rt thank you for your proposal, but I already accepted @0xmiroslav's suggestions.

[s77rt]

@grgia I think proposals gets reviewed by 2 reviewers. @0xmiroslav has not been assigned yet, besides I think my proposal is cleaner and global.

cc @mollfpr

[grgia]

@s77rt this is a deploy blocker, so I will be finishing this internally. Because @0xmiroslav was assisting in the original thread and posted a solution first, we decided to compensate for their suggestions.

[0xmiroslav]

@s77rt your proposal causes serious regressions in various parts.
Here's an example:

bug.mov

And as @grgia commented already, this is intended to be fixed internally since this is a deploy blocker. @grgia marked this as External just to unlock my comment posted.
And it's not good to tag accidently assigned C+ ignoring internal engineer's comment.

[s77rt]

@grgia If an issue is internal it should be internal for everybody. It does not make sense to make an issue internal in GH but not on slack.

@0xmiroslav This is due to the use of overflow hidden (and the use of outset box-shadow)

[puneetlath]

Hey @s77rt, you'll notice that the Help Wanted label was removed before you started posting on this issue. We tend to move more quickly with Deploy Blockers and I understand that can be confusing, but please try not to get frustrated.

We appreciate your eagerness to contribute, so please feel free make proposals on any of the issues that currently have the Help Wanted label on them. We add new issues every day so there is plenty of work to go around.

[shawnborton]

I wonder if we should make the left border use the same color as the appBG as well? No worries if that's not really part of the issue though.

[flaviadefaria]

As a note, I'm going to be OOO from Dec 23rd to January 9th. In case this needs to be paid before the week of January 9th then please reassign.

[0xmiroslav]

The solution was deployed to production on Dec 21 (#13697 (comment)) but issue title was not updated.
The payment is due today (Dec 28).

[melvin-bot]

@mollfpr, @grgia, @flaviadefaria Eep! 4 days overdue now. Issues have feelings too...

[mallenexpensify]

@mollfpr who is due payment here and how much?
I'm helping out Flavia who's out til next week.

[mollfpr]

@mallenexpensify I did not review any proposals or PR so I’m not eligible for payment here.

[0xmiroslav]

@mallenexpensify I think I am eligible for 1250 - reporting compensation and bonus for my solution being used.

[mallenexpensify]

Thanks @0xmiroslav and @mollfpr
PR - #13697
Compensation for @0xmiroslav is $1000, our base level for jobs, plus $250 for reporting (as they said above, ha) = $1250

@0xmiroslav can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01e2c7bdef927162e8

[mallenexpensify]

Checking on regression test steps https://expensify.slack.com/archives/C01SKUP7QR0/p1672785670352739

[mallenexpensify]

^ TestRail steps added, once @0xmiroslav accepts offer I'll pay then close

[mallenexpensify]

@0xmiroslav paid, thanks! Closing