[HOLD for payment 2024-04-15] Split button has incorrect hover styles #39020

m-natarajan · 2024-03-26T21:23:03Z

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.56-5
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: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1711469875796639

Action Performed:

Click on fab > send money
enter amount and click next
select a user and go to confirmation page

Expected Result:

The split button should have hover zones that line up with the two button areas that are split by a border

Actual Result:

There is a weird overlap and you can trigger the hover zone of one side of the button without fully being hovered over it.

Workaround:

unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Recording.2908.mp4

View all open jobs on GitHub

melvin-bot · 2024-03-26T21:23:14Z

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

nkdengineer · 2024-03-26T22:00:06Z

Proposal

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

Split button has incorrect hover styles

What is the root cause of that problem?

In here, the divider is in the button view

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

We should move the divider to outside the dropdown button like below:

                    <Button />
                    <View style={[styles.dropDownButtonCartIconView, innerStyleDropButton]}>
                        <View style={[success ? styles.buttonSuccessDivider : styles.buttonDivider]} />
                    </View>
                    <Button />

What alternative solutions did you explore? (Optional)

NA

Krishna2323 · 2024-03-26T22:32:00Z

Proposal

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

Split button has incorrect hover styles

What is the root cause of that problem?

The divider is in the Button which shows the arrow.

App/src/components/ButtonWithDropdownMenu/index.tsx

Line 102 in f7a697a

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

We need to move the divider between both buttons and have update styles.

Steps:

Move the line below from:

App/src/components/ButtonWithDropdownMenu/index.tsx

Line 102 in f7a697a

<View style={[success ? styles.buttonSuccessDivider : styles.buttonDivider]} />

to:

App/src/components/ButtonWithDropdownMenu/index.tsx

Line 88 in f7a697a
Remove ...sizing.h100 from buttonSuccessDivider & buttonDivider, it will look overflowed if we have ...sizing.h100 because the buttons have 1px padding.
Update the height of the divider based on the button size.

<View
    style={[success ? styles.buttonSuccessDivider : styles.buttonDivider, {minHeight: isButtonSizeLarge ? variables.componentSizeLarge : variables.componentSizeNormal}]}
/>

Optional: We can create a util function to get the divider styles based on sizes just like we have for buttons (getButtonStyleWithIcon)

Result

allgandalf · 2024-03-26T23:10:20Z

Proposal

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

Split button has incorrect hover styles

What is the root cause of that problem?

We mixup style for Large button sizes.

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

I have some different approach but it has a downside, so maybe we can ask @shawnborton whether it would be fine

We need to pass a optional propisButtonSizeLarge to Icon component from Settlement Button i.e. :

App/src/components/Button/index.tsx

Line 319 in f7a697a

    
           StyleUtils.getButtonStyleWithIcon(styles, small, medium, large, Boolean(icon), Boolean(text?.length > 0), shouldShowRightIcon),

the default value of isButtonSizeLarge would be false

StyleUtils.getButtonStyleWithIcon(styles, small, medium, isButtonSizeLarge ? false : large,

We do get the line aligned but here we shrink the text a little, i have compared both of them below

Updated Fix	Production

What alternative solutions did you explore? (Optional)

N/A

neonbhai · 2024-03-27T00:22:02Z

Proposal

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

Split button has incorrect hover styles

What is the root cause of that problem?

We added support for medium size dropdown button

Used here

But the new styles added caused alignment issues with the large size, as we didn't update the margin styles here:

App/src/components/ButtonWithDropdownMenu/index.tsx

Line 101 in 7257a0f

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

Update the styles to use bigger margins when isButtonSizeLarge is true

Recommending to use 18px on left and right margins to center the icon:

dropDownButtonArrowContainLarge: {
    marginLeft: 18,
    marginRight: 18,
},

Update this line to use larger margins when isButtonSizeLarge is true:

<View style={[isButtonSizeLarge ? dropDownButtonArrowContainLarge : styles.dropDownButtonArrowContain]}>

Alternatively

We can use the style names as to dropDownButtonArrowContainMedium and dropDownButtonArrowContainLarge

Result:

Screen.Recording.2024-03-27.at.5.51.15.AM.mov

bernhardoj · 2024-03-27T04:17:52Z

Proposal

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

The hover doesn't take the full width of the button.

What is the root cause of that problem?

This only happens when the ButtonWithDropdownMenu has a buttonSize of large. When we set the size to large, the button will have a minimum size of 52.

App/src/styles/index.ts

Lines 598 to 601 in 1bbb594

    
           buttonLarge: { 
        
               borderRadius: variables.buttonBorderRadius, 
        
               minHeight: variables.componentSizeLarge, 
        
               minWidth: variables.componentSizeLarge,

App/src/styles/variables.ts

Line 22 in 1bbb594

componentSizeLarge: 52,

However, the total width of the arrow and divider is only 43 (16 arrow icon + 26 margin horizontal + 1 border width).

Then, the button has an align items center style which puts the children to the center.

Screen.Recording.2024-03-27.at.12.01.55.mov

You can also notice that this doesn't happen in a smaller size.

Screen.Recording.2024-03-27.at.12.02.35.mov

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

First, move the divider element to the direct child of the Button.

App/src/components/ButtonWithDropdownMenu/index.tsx

Line 102 in 1bbb594

Next, add flex row style to the Button innerStyles.

Last, add styles.flex1, styles.justifyContentCenter styles to the dropdown view wrapper. This will align the arrow to the center and take all available space.

App/src/components/ButtonWithDropdownMenu/index.tsx

Line 101 in 1bbb594

<Button ...
    innerStyles={[..., styles.flexRow, ...]} ...
>
    <View style={[success ? styles.buttonSuccessDivider : styles.buttonDivider]} />
    <View style={[styles.dropDownButtonCartIconView, styles.flex1, styles.justifyContentCenter, innerStyleDropButton]}>
...

What alternative solutions did you explore? (Optional)

Adjust the arrow icon margin left and right based on the button size so the whole width is equal or bigger than the button min width.

shawnborton · 2024-03-27T12:16:49Z

Actually @narefyev91 is saying that this is fixed on production already?

Screen.Recording.2024-03-27.at.13.56.53.mov

narefyev91 · 2024-03-27T12:53:44Z

will take this one as it was coming from global button refactoring

CortneyOfstad · 2024-03-27T14:28:08Z

Assigned you to the issue @narefyev91 👍

CortneyOfstad · 2024-03-27T14:29:02Z

Also assigned @akinwale as they are the reviewer on the PR here 👍

melvin-bot · 2024-03-29T12:52:25Z

Triggered auto assignment to @francoisl, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

CortneyOfstad · 2024-04-05T14:31:15Z

PR was deployed to staging 3 days ago — will keep an eye out when it moves to production 👍

melvin-bot · 2024-04-08T10:47:32Z

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

melvin-bot · 2024-04-08T10:47:35Z

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

Fix hover issue with ButtonWithDropdownMenu #39054

If no regressions arise, payment will be issued on 2024-04-15. 🎊

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

@akinwale requires payment (Needs manual offer from BZ)
@narefyev91 does not require payment (Contractor)

melvin-bot · 2024-04-08T10:47:36Z

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:

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

CortneyOfstad · 2024-04-15T13:46:02Z

For some reason job link did not post — https://www.upwork.com/ab/applicants/1779868341125738496/job-details

CortneyOfstad · 2024-04-15T13:46:42Z

@akinwale I sent you an offer for the job in Upwork. Please let me know once you accept.

Also, can you complete the checklist here by EOD? Thank you!

akinwale · 2024-04-15T13:48:39Z

@CortneyOfstad Accepted. Thanks!

akinwale · 2024-04-15T13:53:11Z

[@akinwale] The PR that introduced the bug has been identified. Link to the PR:

[@akinwale] 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:

[@akinwale] 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:

Not a regression. This is a design change following a global button refactor.

[@akinwale] Determine if we should create a regression test for this bug.

[@akinwale] 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.

Regression Test Steps

Launch Expensify
Click on the global create menu and select Send Money
Enter an amount and click Next
Select a recipient and continue to the confirmation page
Hover the mouse on each side of the button separated by the divider (text on the lhs, icon on the rhs).
Verify that the hover effect is separated by the divider.

Do we agree 👍 or 👎?

CortneyOfstad · 2024-04-15T16:47:13Z

Payment Summary

@akinwale — paid $500 via Upwork

CortneyOfstad · 2024-04-15T16:49:06Z

Regression test created here — https://github.com/Expensify/Expensify/issues/387839. Closing!

m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 26, 2024

melvin-bot bot assigned CortneyOfstad Mar 26, 2024

narefyev91 mentioned this issue Mar 27, 2024

Fix hover issue with ButtonWithDropdownMenu #39054

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Mar 27, 2024

CortneyOfstad assigned narefyev91 Mar 27, 2024

CortneyOfstad assigned akinwale Mar 27, 2024

melvin-bot bot assigned francoisl Mar 29, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Apr 8, 2024

melvin-bot bot changed the title ~~Split button has incorrect hover styles~~ [HOLD for payment 2024-04-15] Split button has incorrect hover styles Apr 8, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Apr 8, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 14, 2024

CortneyOfstad closed this as completed Apr 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-04-15] Split button has incorrect hover styles #39020

[HOLD for payment 2024-04-15] Split button has incorrect hover styles #39020

m-natarajan commented Mar 26, 2024

melvin-bot bot commented Mar 26, 2024

nkdengineer commented Mar 26, 2024 •

edited

Loading

Krishna2323 commented Mar 26, 2024 •

edited

Loading

allgandalf commented Mar 26, 2024 •

edited

Loading

neonbhai commented Mar 27, 2024

bernhardoj commented Mar 27, 2024

shawnborton commented Mar 27, 2024

narefyev91 commented Mar 27, 2024

CortneyOfstad commented Mar 27, 2024

CortneyOfstad commented Mar 27, 2024

melvin-bot bot commented Mar 29, 2024

CortneyOfstad commented Apr 5, 2024

melvin-bot bot commented Apr 8, 2024

melvin-bot bot commented Apr 8, 2024

melvin-bot bot commented Apr 8, 2024

CortneyOfstad commented Apr 15, 2024

CortneyOfstad commented Apr 15, 2024

akinwale commented Apr 15, 2024

akinwale commented Apr 15, 2024

CortneyOfstad commented Apr 15, 2024

CortneyOfstad commented Apr 15, 2024

[HOLD for payment 2024-04-15] Split button has incorrect hover styles #39020

[HOLD for payment 2024-04-15] Split button has incorrect hover styles #39020

Comments

m-natarajan commented Mar 26, 2024

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Mar 26, 2024

nkdengineer commented Mar 26, 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)

Krishna2323 commented Mar 26, 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?

Result

allgandalf commented Mar 26, 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)

neonbhai commented Mar 27, 2024

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?

Alternatively

Result:

bernhardoj commented Mar 27, 2024

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)

shawnborton commented Mar 27, 2024

narefyev91 commented Mar 27, 2024

CortneyOfstad commented Mar 27, 2024

CortneyOfstad commented Mar 27, 2024

melvin-bot bot commented Mar 29, 2024

CortneyOfstad commented Apr 5, 2024

melvin-bot bot commented Apr 8, 2024

melvin-bot bot commented Apr 8, 2024

melvin-bot bot commented Apr 8, 2024

CortneyOfstad commented Apr 15, 2024

CortneyOfstad commented Apr 15, 2024

akinwale commented Apr 15, 2024

akinwale commented Apr 15, 2024

CortneyOfstad commented Apr 15, 2024

Payment Summary

CortneyOfstad commented Apr 15, 2024

nkdengineer commented Mar 26, 2024 •

edited

Loading

Krishna2323 commented Mar 26, 2024 •

edited

Loading

allgandalf commented Mar 26, 2024 •

edited

Loading