[HOLD for payment 2023-09-04] [$1000] Update Dropdown Button UI

[grgia]

Coming from: #20486

before:

after (in mockup):

Notes:

• Button height (40px)
• Divider line (1px, same color as text, 28px height)
• Caret is still 20x20 px
• Same flex styling, here's an example on an expense report view.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01939b2277abfdcd49
• Upwork Job ID: 1678311351793197056
• Last Price Increase: 2023-07-31
• Automatic offers:
• abdulrahuman5196 | Reviewer | 25926814
• dhairyasenjaliya | Contributor | 25926817

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01939b2277abfdcd49

[melvin-bot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @abdulrahuman5196 (Internal)

[ygshbht]

Is this open for contributors to work? I can do it. Do i need to submit a poposal?

[abdulrahuman5196]

@ygshbht Only issues open and have a 'Help Wanted' label is open for contributors to submit proposals

[grgia]

cc @shawnborton does this style change affect only the settlement button, or all dropdown buttons?

[shawnborton]

I guess we'd want to do that to all split buttons that exist for consistency!

[grgia]

I'm going to make this one external @abdulrahuman5196. Happy to help with any styling questions that come up

cc @ygshbht if you're still interested!

[melvin-bot]

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

[melvin-bot]

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

[huzaifa-99]

Can i work on this?

[himanshuragi456]

Proposal

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

UI changes are required to make the component as per the design.

What is the root cause of that problem?

Unwanted UI changes that need to be fixed

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

Will fix the following points:

Button height (40px)
Divider line (1px, same color as text, 28px height)
Caret is still 20x20 px
Same flex styling, will keep as per design

https://github.com/Expensify/App/blob/HEAD/src/components/ButtonWithDropdownMenu.js

add medium to buttons to change the min-height to 40px. also set the height to 40px (componentSizeNormal) using innerStyles prop of Button and add styles.p0

for divider:

// in Button Component
<View style={styles.dividerContainer}>
    <View style={styles.dividerElement} />
</View>
// CSS being:
dividerContainer: {
    width: 1,
    justifyContent: 'center',
    backgroundColor: themeColors.success,
    height: 40
},
dividerElement: {
    backgroundColor: themeColors.text,
    height: 28
},

What alternative solutions did you explore? (Optional)

N/A

[ishpaul777]

Proposal

Please re-state the problem that we are trying to solve in this issue.
UI changes are required to make the component as per the design.

What is the root cause of that problem?

Not a problem

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

Will add following changes to the ButtonWithDropdownMenu component https://github.com/Expensify/App/blob/HEAD/src/components/ButtonWithDropdownMenu.js

Button height (40px)
Divider line (1px, same color as text, 28px height)
Caret - 20x20 px

What alternative solutions did you explore? (Optional)

N/A

[dhairyasenjaliya]

Proposal

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

• Update Dropdown Button UI

What is the root cause of that problem?

• New Feature request 


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

• Here as per the moke-up we need to change below

Button height (40px)

• On the below lines we use <Button> component and we need to pass medium props to achieve min-height 40px height to the buttons by default we have 52 so this is needed to add
  -Also we need to pass actual height as 40px or componentSizeNormal on both below lines as innerStyles props

App/src/components/ButtonWithDropdownMenu.js

Line 78 in 25c8ae5

<Button

App/src/components/ButtonWithDropdownMenu.js

Line 89 in 25c8ae5

<Button

Divider line (1px, same color as text, 28px height)

• For divider, we need to remove marginVertical and add the height of 28 on styles.buttonDivider
• Also add themeColors.text as backgroundColor for consistency
• And then we need to move buttonDivider view inside the cartButton as from the mock design we can Cleary see that we need to remove the divider and add a small white divider with 28 height that does not look like 2 seprate button
• After moving we need to adjust the styling for the entire cart button since now it includes the divider and place it on the left side

Caret is still 20x20 px

• The cart is the same size
• We can also pass small to cart <icon> to change the size optionally


Same flex styling, here's an example on an expense report view.

• It's the same styling so no need to update

What alternative solutions did you explore? (Optional)

• N/A

[neonbhai]

Proposal

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

Update Dropdown Button UI

What is the root cause of that problem?

New Feature Request

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

The ButtonWithDropdownMenu component renders the divider using the line:

App/src/components/ButtonWithDropdownMenu.js

Line 88 in 7770b86

<View style={styles.buttonDivider} />

which references the CSS:

App/src/styles/styles.js

Lines 551 to 556 in 7770b86

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

To change the colour to the divider to the same color as text we would the property of backgroundColor to themeColors.text
To change the height of the divider we would set the property of height to 28.
Since the divider isn't stretching to the whole button we will center the alignment with alignSelf: 'center'

Currently the divider is rendered between the two buttons (1st button contains text and 2nd contains the dropdown icon). To make sure the background is not shown in the height where the divider is not present, the simplest solution is to move the divider as content of 2nd button (which contains the dropdown icon).

To change the height of the whole component we need to pass the medium prop to all the Button component calls in ButtonWithDropDownMenu.js
Since caret image has more padding than the required, we would remove the padding and center the image.

Currently Button component without a size prop renders with CSS:

App/src/styles/styles.js

Lines 443 to 449 in 7770b86

	button: {
	backgroundColor: themeColors.buttonDefaultBG,
	borderRadius: variables.buttonBorderRadius,
	minHeight: variables.componentSizeLarge,
	justifyContent: 'center',
	...spacing.ph3,
	},

The minHeight is currently componentSizeLarge which is 52px

Passing the medium prop to the Button component calls makes the buttonMedium CSS style is applied to the button which renders with minHeight: componentSizeNormal which is 40px as required.

What alternative solutions did you explore? (Optional)

To reduce the button size anytime a general button is rendered we can simply change the button component's style minHeight to componentSizeNormal in the CSS:

App/src/styles/styles.js

Line 446 in 7770b86

minHeight: variables.componentSizeLarge,

With this change anytime a large button has to be rendered, we would have to explicitly pass the large prop. This change should be done only if the Expensify Design Team is moving away from a general button size to be large. I would advocate for this approach since 40px height buttons look more elegant

[Nikhil-Vats]

Proposal

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

Feature: Update the UI of Dropdown button

What is the root cause of that problem?

No root cause, this is not a bug.

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

We have to make the following changes in ButtonWithDropdownMenu -

1. Pass medium to both buttons - the one showing text and the one showing caret icon. This would change the min-height to 40px. We should also set the height to 40px (componentSizeNormal) using innerStyles prop of Button and remove unnecessary padding of 1px by passing styles.p0 in line 85 and 93.

2. Pass medium and set height to 40px, padding to 0px for the button on line 104 which is shown when there is only 1 option so we just show normal button but no caret button. This would ensure consistency.
   

   App/src/components/ButtonWithDropdownMenu.js

   Line 104 in 7770b86

   <Button

   See screenshot

   Before -

   

   After -
   

3. For caret button, the horizontal padding in new mockup seems less than that in the app right now. The top and bottom (vertical) padding can be made 0 since element is anyway vertically centered and the left and right (horizontal) padding should be 12px (approx from design). We can do this via a new object in styles.js or a new function in StyleUtils.

   See screenshot

   Before (black background behind divider line is handled in next point) -

   

   After -

   

4. For divider line, since we are reducing height to 28px so the background color of wrapper (dark green) will be visible, to avoid that we will wrap divider line View with another wrapper View -

   // in `ButtonWithDropdownMenu`
   <View style={styles.buttonDividerWrapper}>
       <View style={styles.buttonDivider} />
   </View>
   // with following css in `styles.js` -
   buttonDividerWrapper: {
       width: 1,
       justifyContent: 'center',
       backgroundColor: themeColors.success,
       height: 40
   },
   buttonDivider: {
       backgroundColor: themeColors.text, // change from 'appBG' to 'text'
       height: 28 // remove marginVertical, add this
   },

   See screenshot

   Before -

   

   After -

   

What alternative solutions did you explore? (Optional)

If we go with the above approach for making the divider line smaller, there will be a 1px area on line where click behaviour won't work (which is also what happens right now). If we want to avoid this, we can move the divider line to either the right button or the left button depending on what the Expensify team thinks should happen if user clicks on the line - pay or open payment options.

If we use this approach then we have to change the flex properties of the Button using innerStyles prop to change flex-direction to row, align-items to center and use gap or padding for adding space between divider link and caret icon.

The downside of this approach is that, when we hover over caret, it will also change the background colour of line which looks weird -

See video

Screen.Recording.2023-07-18.at.8.15.28.PM.mov

Result -

Final screenshots

[ygshbht]

Thanks for letting me know @grgia. Looks like some awesome proposals have already taken off. So I'll let this one go and jump to other open issues

I'm going to make this one external @abdulrahuman5196. Happy to help with any styling questions that come up

cc @ygshbht if you're still interested!

[grgia]

that's correct that Money request header buttons will be at the top in the header.

[dhairyasenjaliya]

@grgia should we wait for that or just add medium size as per design? As currently button is bottom and all bottom button has large size as I have posted above also draft PR Is up

[dhairyasenjaliya]

hey @grgia waiting for 🟢 on above comment

[trjExpensify]

For the first screenshot you shared, I believe that button will be moving to the top header area right? cc @trjExpensify @JmillsExpensify @grgia on that one

Just so I'm sure, which is the first screenshot being referenced? 😅

[dhairyasenjaliya]

@trjExpensify here is the comment I have added a screenshot for the money request page #22540 (comment)

[trjExpensify]

Thanks! That's the confirmation screen ahead of creating the request. I don't believe that confirmation button is moving to the top header area. I've been out for a couple of days though, so might have missed something? @shawnborton @JmillsExpensify

[shawnborton]

Ah, great point. Then I think we would keep that as a bottom-docked button and keep it at 52px tall.

[dhairyasenjaliya]

@abdulrahuman5196 PR ready for review with latest changes to display large button for bottom-docked

[JmillsExpensify]

Great catch, agree with @trjExpensify

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @dhairyasenjaliya got assigned: 2023-08-03 13:21:52 Z
• when the PR got merged: 2023-08-22 15:29:55 UTC
• days elapsed: 13

On to the next one 🚀

[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.57-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [UI]-Dropdown Button #24034

If no regressions arise, payment will be issued on 2023-09-04. 🎊

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

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

• @abdulrahuman5196 requires payment offer (Reviewer)
• @dhairyasenjaliya requires payment offer (Contributor)

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

[garrettmknight]

Summary of payments

urgency bonus: no
Reporter: N/A
Contributor: @dhairyasenjaliya $1000 paid in Upwork
Contributor+: @abdulrahuman5196 $1000 paid in Upwork