[HOLD for payment 2024-06-06] [$250] mWeb - Taxes - Buttons on the top not filing the space evenly

[m-natarajan]

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.66-0
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/p1714046529643169

Action Performed:

1. Resize the browser to mobile view
2. Go to workspace settings > Taxes
3. Observe the top 2 buttons

Expected Result:

The two buttons at the top should fill the space evenly

Actual Result:

The two buttons are too wide and cause overflow from the wrapping container.

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018c24cec1ff92c41b
• Upwork Job ID: 1783687288261435392
• Last Price Increase: 2024-05-03
• Automatic offers:
• DylanDylann | Reviewer | 0
• Krishna2323 | Contributor | 0

Issue Owner

Current Issue Owner: @jliexpensify

[melvin-bot]

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[shawnborton]

While we're at it, I think we might consider adjusting the space between the buttons here - maybe tighten it up to be 8px gap instead of 12px? Thoughts @Expensify/design?

12px:

8px:

And we'd wanna do that everywhere across all workspace pages.

[dubielzyk-expensify]

I'm down for 8px

[Krishna2323]

Proposal

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

Workspace - Taxes - Buttons on the top not filing the space evenly

What is the root cause of that problem?

The stylings applied to the container and button isn't correct. The buttons takes 50% width each and then the first one has margin right of 12px.

App/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx

Lines 225 to 243 in 42d4ed7

	<View style={[styles.w100, styles.flexRow, isSmallScreenWidth && styles.mb3]}>
	{!PolicyUtils.hasAccountingConnections(policy) && (
	<Button
	medium
	success
	onPress={navigateToCreateCategoryPage}
	icon={Expensicons.Plus}
	text={translate('workspace.categories.addCategory')}
	style={[styles.mr3, isSmallScreenWidth && styles.w50]}
	/>
	)}
	<Button
	medium
	onPress={navigateToCategoriesSettings}
	icon={Expensicons.Gear}
	text={translate('common.settings')}
	style={[isSmallScreenWidth && styles.w50]}
	/>
	</View>

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

Instead of setting 50% width, we should use flex1 on buttons and for spacing between them, we should use gap on the parent container. The expected spacing between button will be decided by the design team. This issue seems to be happening on all policy pages so we need to find and fix all of them. We also need to remove the margin right on the first button.

What alternative solutions did you explore? (Optional)

Result

spacing_between_btns.mp4

[Krishna2323]

Proposal Updated

• Minor edit

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~018c24cec1ff92c41b

[melvin-bot]

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

[jliexpensify]

@shawnborton not sure of the complexity of this issue (it feels easy, but I could be wrong), but if you feel like it should be less than $250, feel free to adjust the price!

[TheFinalPush]

Hi Expensify team,


I want to contribute in this issue.



Please add me in your Slack Channel.

Email- [email protected]


I already sent the email to [email protected] but still not received any invitation.

Looking to learn, collaborate, and contribute.

[dannymcclain]

@shawnborton @dubielzyk-expensify I'm not sure about adjusting the spacing. When I measure the gap in the product, it looks like it's 14px, not 12.

Here's a comparison of actually 12px vs. 8px

Honestly both feel fine to me though. I just want to make sure we're making adjustments based on how things should be, not just based on how they look in the product (because that might be not quite right).

[dragnoir]

Proposal

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

Taxes - Buttons on the top not filing the space evenly

What is the root cause of that problem?

on WorkspaceTaxesPage the buttons use 50% width. When we apply margin to one of buttons, this push the other one out of the borders.

App/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx

Line 217 in d0c2551

style={[styles.mr3, isSmallScreenWidth && styles.w50]}

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

We need to clean the style.

1- replace the styles.w50 with styles.flex1

We need to allow button to stretch to the spaces availabe without passing border

2- replace margin between buttons with flex gap on the container

Flex gap will do the spacing when there's more than one div inside the container.

3- fix the button when row selected

We need also to set styles.flex1 for this button

App/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx

Line 228 in d0c2551

<ButtonWithDropdownMenu<WorkspaceTaxRatesBulkActionType>

4- move styles.mb3 from both parts, to the container

from

App/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx

Line 210 in d0c2551

<View style={[styles.w100, styles.flexRow, isSmallScreenWidth && styles.mb3]}>

and
https://github.com/Expensify/App/blob/d0c2551d0ae354438972e2c97b014e98dbd92664/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx#L236C77-L236C87
to

App/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx

Line 261 in d0c2551

{isSmallScreenWidth && <View style={[styles.pl5, styles.pr5]}>{headerButtons}</View>}

Those updates need to be applied to all top-right buttons like workspace pages.

[dragnoir]

@shawnborton when you select one of the options, the new button "1 selected", I think it should be stretched on all the screen width. What do you think?

[dragnoir]

@shawnborton on the same screen, the table header "Status" is not aligned correctly

[dragnoir]

@shawnborton here'is another non-consistent point.

The header on those workspace pages, the space between top and buttons,, is defined by height value and it's equal to 80px

instead, on the report pages, it's 72px

[dragnoir]

@shawnborton Sorry for all those comments, just wanna share the UI bugs to make the best consistency

For buttons with icons, the spacing on the left is not equal to the spacing on the right, there's a 4-pixel difference

So the (text+icon) inside the button are not well aligned.

[dubielzyk-expensify]

Honestly both feel fine to me though.

Yeah. They both look good. Personally probably prefer 8px visually, but given our footer button has 12px, maybe we go with that? I don't mind either way

[jliexpensify]

Not overdue, still discussing!

[shawnborton]

@dragnoir replying to your comments:

when you select one of the options, the new button "1 selected", I think it should be stretched on all the screen width. What do you think?

Agree, it should be full width. I think we have a separate issue somewhere to update this button to be a dropdown button though, is that right @luacmartins?

The header on those workspace pages, the space between top and buttons,, is defined by height value and it's equal to 80px

Yes, this is a separate inconsistency that we've talked about and will handle in a separate issue. One weird thing I noticed though is that there is padding on the left/right sides of the header, which is strange to me.

on the same screen, the table header "Status" is not aligned correctly

This is correct, the Status header is right-aligned. So the alignment is happening on the right side.

For buttons with icons, the spacing on the left is not equal to the spacing on the right, there's a 4-pixel difference

This is also intentional.

[shawnborton]

I think I still lean towards 8px gap as the standard... I'd love to make that a global rule if possible. Mostly thinking about what we plan to do with the Search page, and I think 8px feels a bit nicer there:

So I would love to just use 8px as the standard gap for horizontal button groupings. Thoughts? I don't feel strongly and could totally be convinced that 12px is the better gap.

[dannymcclain]

I think I still lean towards 8px gap as the standard... I'd love to make that a global rule if possible. Mostly thinking about what we plan to do with the Search page, and I think 8px feels a bit nicer there
So I would love to just use 8px as the standard gap for horizontal button groupings. Thoughts? I don't feel strongly and could totally be convinced that 12px is the better gap.

I am totally down for 8px being the standard gap for horizontal button groupings. I just wanted to make sure we were sure 😂. Let's send it.

[Krishna2323]

@DylanDylann, PR ready for review.

[dubielzyk-expensify]

Do you have a link to the PR so I can test it out?

[Krishna2323]

@dubielzyk-expensify, I forgot to tag design team on the PR, PR here.

[dubielzyk-expensify]

It looks more correct now:

There is a 1px padding applied to these buttons which means that the spacing between the buttons becomes 10px and not 8px. Is this a hack for something I'm not aware of?

It also means that there's 21px from the right edge to the button.

The same on desktop:

[Krishna2323]

The 1px padding was added 2 years ago in this PR. I'm also not sure what's the use case for the padding.

@marcaaron, could you please help clarify the use case for the padding?

[shawnborton]

Whoa, that is a weird change. I feel like we should undo that 1px padding hack personally.

[dannymcclain]

Agree! Let's remove that 1px of weirdo padding if we can. That explains why I was measuring 14px instead of 12px a bit ago.

[melvin-bot]

This issue has not been updated in over 15 days. @tylerkaraszewski, @jliexpensify, @Krishna2323, @DylanDylann eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

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

• fix: Worksapce pages - buttons on the top not filing the space evenly. #41647

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

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

• @Krishna2323 requires payment automatic offer (Contributor)
• @DylanDylann requires payment automatic offer (Reviewer)

[melvin-bot]

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:

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

[jliexpensify]

Payment Summry

• C: @Krishna2323 $250
• C+: @DylanDylann $250

Upwork job

Just waiting on the checklist.

[jliexpensify]

Bump @DylanDylann to complete the checklist please

[jliexpensify]

SIGH I have to create a new job because it's expired.

@DylanDylann and @Krishna2323 pleas accept: https://www.upwork.com/jobs/~013b50ebeebd734f43

[DylanDylann]

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:

[@DylanDylann] The PR that introduced the bug has been identified. Link to the PR: NA
[@DylanDylann] 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: NA
[@DylanDylann] 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: NA
[@DylanDylann] Determine if we should create a regression test for this bug. Yes
[@DylanDylann] 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 Proposal

1. Open workspace settings > Enable taxes, categories, distance rates and tags
2. Go taxes, categories, distance rates and tags page one by one
3. Verify the header buttons have 8px gap between them

• For small width devices

1. Open workspace settings > Enable taxes, categories, distance rates and tags
2. Go taxes, categories, distance rates and tags page one by one
3. Verify the header buttons have 8px gap between them and the buttons container has equal horizontal padding

Do we agree 👍 or 👎

[jliexpensify]

Paid and job closed, thanks everyone!