Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.53k stars 2.88k forks source link

Style mismatch for Add rate button in Taxes #47573

Closed m-natarajan closed 2 months ago

m-natarajan commented 2 months ago

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: 9.0.21-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: ZhenjaHorbach Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1723754129659149

Action Performed:

  1. Open app
  2. Open account settings and open workspaces
  3. Choose any workspace
  4. In more features turn on Distance rates, Categories, Tags and Taxes

Expected Result:

Add rate button in Taxes has should match with other buttons in the headers (Distance rates, Categories, Tags)

Actual Result:

Notice that Add rate button in Taxes has different styles relative to other buttons in the headers (Distance rates, Categories, Tags)

Workaround:

unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/a737c892-4ff9-4245-8011-74293c4e21c5

https://github.com/user-attachments/assets/cabed25a-3829-4c93-81f4-f485aab0c127

View all open jobs on GitHub

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @kadiealexander (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.

daledah commented 2 months ago

Proposal

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

Style mismatch for Add rate button in Taxes

What is the root cause of that problem?

Currently, we have set the gap on the categories page and tags page to 8px, but the padding-right on taxes page is set to 12px.

https://github.com/Expensify/App/blob/5e5bff3630958264c22a34f083879792ff8ee6ab/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx#L230

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

We should update styles here to styles.mr2 to be consistent with categories page and tags page

style={[styles.mr2, shouldUseNarrowLayout && styles.flex1]}

What alternative solutions did you explore? (Optional)

ZhenjaHorbach commented 2 months ago

@kadiealexander I reported this issue Can I be a reviewer here please?

Nodebrute commented 2 months ago

Edited by proposal-police: This proposal was edited at 2024-08-16 16:16:46 UTC.

Proposal

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

Style mismatch for Add rate button in Taxes

What is the root cause of that problem?

we are adding styles.mr3 here which we are not adding in category, distance, or tags page https://github.com/Expensify/App/blob/279f561c90adef037b27f73ef96c8487e44d0be8/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx#L230 also we are not adding styles.gap2 here which we adding in category, distance, or tags page https://github.com/Expensify/App/blob/279f561c90adef037b27f73ef96c8487e44d0be8/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx#L222

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

We should remove styles.mr3 from here https://github.com/Expensify/App/blob/279f561c90adef037b27f73ef96c8487e44d0be8/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx#L230 We can also add styles.gap2 here https://github.com/Expensify/App/blob/279f561c90adef037b27f73ef96c8487e44d0be8/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx#L222

What alternative solutions did you explore? (Optional)

Nodebrute commented 2 months ago

Proposal Updated

melvin-bot[bot] commented 2 months ago

@kadiealexander Whoops! This issue is 2 days overdue. Let's get this updated quick!

kadiealexander commented 2 months ago

I don't think this should be fixed, it's a non-issue.