[HOLD for payment 2024-04-25] [$125] No spacing between explanatory text and toggle button in "settings" #38370

m-natarajan · 2024-03-15T03:48:16Z

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.52-6
Reproducible in staging?: y
Reproducible in production?: new feature
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: @JmillsExpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1710461880379559

Action Performed:

Open app and switch the language to spanish
Go to any "collect" workspace
Click categories >settings

Expected Result:

There should be spacing between the explanatory text and toggle switch

Actual Result:

No enough spacing

Workaround:

unknown

Platforms:

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

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/~014169e1c9001cba30
Upwork Job ID: 1768677955202637824
Last Price Increase: 2024-03-15

The text was updated successfully, but these errors were encountered:

melvin-bot · 2024-03-15T03:48:26Z

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

tienifr · 2024-03-15T04:00:03Z

Proposal

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

No enough spacing

What is the root cause of that problem?

We don't have margin/ padding for text or switch button

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

Lines 70 to 75 in f1f22c7

    
           <Text>{translate('workspace.categories.enableCategory')}</Text> 
        
           <Switch 
        
               isOn={policyCategory.enabled} 
        
               accessibilityLabel={translate('workspace.categories.enableCategory')} 
        
               onToggle={updateWorkspaceRequiresCategory} 
        
           />

we also don't have any logic to break new line for long text

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

We can add marginRight: 8 and flexShrink: 1 in

<Text style={[styles.textNormal, styles.colorMuted, {marginRight: 8, flexShrink: 1}]}>{translate('workspace.categories.requiresCategory')}</Text>

What alternative solutions did you explore? (Optional)

NA

Result

joekaufmanexpensify · 2024-03-15T16:37:34Z

Reproduced.

melvin-bot · 2024-03-15T16:37:41Z

Job added to Upwork: https://www.upwork.com/jobs/~014169e1c9001cba30

melvin-bot · 2024-03-15T16:37:46Z

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

joekaufmanexpensify · 2024-03-15T16:42:29Z

This is a super minor change/spacing issue. Feels like this might be a $125 issue, but we should fix it.

melvin-bot · 2024-03-15T16:42:43Z

Upwork job price has been updated to $125

brandonhenry · 2024-03-15T18:38:08Z

Proposal

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

The problem is that in multiple places throughout the codebase, the text next to a switch component is not properly formatted or aligned, causing inconsistent styling and layout issues.

What is the root cause of that problem?

The main issue here, is there is no margin. You normally don't see this because we are doing text wrapping. However, if the viewport width lines up just right it will be way to close to the switch. See example screens below.

Case reported in original post root can be found here

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

Line 45 in d17b4df

    
           <Text style={[styles.textNormal, styles.colorMuted]}>{translate('workspace.categories.requiresCategory')}</Text>

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

To address this issue, I propose we audit the codebase and update all instances where we have text next to a switch that is not properly formatted.

Specifically, we should:

Locate all usages of the Switch component that have adjacent text
Ensure there is a View wrapping the text and Switch
Apply the styles.mr2 style to the text, to add consistent right margin between the text and switch (we might need padding but I recognized styles.mr2 as being used on other Text next to a Switch in the codebase)

<Text style={[styles.mr2]}>

Cases I've found

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

Line 45 in d17b4df

    
           <Text style={[styles.textNormal, styles.colorMuted]}>{translate('workspace.categories.requiresCategory')}</Text>

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

Line 70 in d17b4df

<Text>{translate('workspace.categories.enableCategory')}</Text>

App/src/pages/settings/Profile/TimezoneInitialPage.tsx

Line 47 in d17b4df

<Text>{translate('timezonePage.getLocationAutomatically')}</Text>

App/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js

Line 862 in d17b4df

    
           <Text color={!iouIsBillable ? theme.textSupporting : undefined}>{translate('common.billable')}</Text>

App/src/components/MoneyRequestConfirmationList.tsx

Line 851 in d17b4df

    
           <Text color={!iouIsBillable ? theme.textSupporting : undefined}>{translate('common.billable')}</Text>

Example Screens With Viewport Adjusted To Show Issue

WorkspaceCategoriesSettingsPage.tsx

TimezoneInitialPage.tsx

CategorySettingsPage

What alternative solutions did you explore? (Optional)

An alternative could be to build the margin into Switch itself. Though, this might interfere with the text wrapping we're already doing

nayabatir1 · 2024-03-17T07:58:40Z

Proposal

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

No spacing between explanatory text and toggle button in Categories Settings page

What is the root cause of that problem?

Gap between both element is not mentioned. If explanatory text is long then it can push switch button out of view. If text grows it should automatically wrap to next link without disturbing position of switch. I'm using flex gap so that it can be screen responsive.

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

In View wrapper added gap. So there is some spacing in both elements.
explanatory text should wrap to next line if text grows.
In Utils index added spacing in staticStyleUtils.
use useStyleUtils hook for flex gap

Code Preview

<View style={[styles.mt2, styles.mh4]}>
	<View style={[styles.flexRow, styles.mb5, styles.mr2, styles.alignItemsCenter, styles.justifyContentBetween, stylesUtils.spacing.gap5]}>
		<Text style={[styles.textNormal, styles.colorMuted, styles.flexShrink1]}>{translate('workspace.categories.requiresCategory')}</Text>

		<Switch
        	isOn={policy?.requiresCategory ?? false}
			accessibilityLabel={translate('workspace.categories.requiresCategory')}
			onToggle={updateWorkspaceRequiresCategory}
		/>
</View>

Image preview

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

parasharrajat · 2024-03-18T11:49:37Z

@brandonhenry Are you saying that same problem exists on all of these pages?

brandonhenry · 2024-03-18T18:46:19Z

@parasharrajat yes, can provide screens shortly

Victor-Nyagudi · 2024-03-18T22:39:37Z

Proposal

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

No spacing between text and toggle button in "settings".

What is the root cause of that problem?

The View housing the text and toggle button has a justifyContent: "space-between" style that's supposed to create the space between the text and the toggle button.

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

Line 44 in 065e52f

    
           <View style={[styles.flexRow, styles.mb5, styles.mr2, styles.alignItemsCenter, styles.justifyContentBetween]}>

However, when the text gets too long, the text expands its container until it touches the toggle button's container leaving no space between them.

space-between only works if the total size of both children containers (text and toggle button) is less than 100% of the parent (the View).

I'm noticing this issue also on web when viewing the setting on the RHP.

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

Since the text is the one that grows and shrinks based on the amount of words, I propose giving the Text component a maxWidth e.g. maxWidth: "85%" on smaller devices, and maxWidth: "80%" on larger devices.

// This is just for demo. We can create a class if needed or use an existing one if it exists for 'maxWidth'.
<Text style={[styles.textNormal, styles.colorMuted, {maxWidth: '85%'}]}>{translate('workspace.categories.requiresCategory')}</Text>

This will ensure the text container's width only grows to a certain portion of its parent such that space-between still works properly.

These percentages/numbers can be adjusted according to what's agreed upon by the design team.

iOS demo after changes

Chrome demo after changes

I've noticed there are a couple of other places such as TimezoneInitialPage where the toggle button is preceded by text, so we can apply a similar approach in those areas for consistency.

In addition, some of the Views housing the text and toggle button in these other places have the exact same value for the style prop, so we could optionally create one style for the text and toggle container and reuse it where necessary to keep code DRY.

What alternative solutions did you explore? (Optional)

None so far.

parasharrajat · 2024-03-19T11:10:21Z

@brandonhenry Yes please, present some visuals to confirm.

brandonhenry · 2024-03-19T15:59:48Z

@parasharrajat done. updated proposal.

Also, since this took some deep dive and also affects multiple parts of the app - I think the bounty should be raised (at least to $250). Thoughts? @joekaufmanexpensify

parasharrajat · 2024-03-19T16:30:11Z

I like the idea of auditing the app and fixing this issue everywhere in the App in one go. Thus @brandonhenry's proposal sounds good to me.

🎀 👀 🎀 C+ reviewed

melvin-bot · 2024-03-19T16:30:27Z

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

Victor-Nyagudi · 2024-03-19T18:39:07Z

Any feedback on my proposal or why using maxWidth is bad in this case, @parasharrajat?

parasharrajat · 2024-03-19T18:59:16Z

I think flexbox should be more flexible. There might be still be a case where percentage unit will take more than the necessary space. It usually happens when screen dimensions changes.

joekaufmanexpensify · 2024-03-20T20:57:35Z

Also, since this took some deep dive and also affects multiple parts of the app - I think the bounty should be raised (at least to $250). Thoughts?

@brandonhenry mind sharing the various places we're going to be updating this?

melvin-bot · 2024-04-18T04:25:08Z

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

melvin-bot · 2024-04-18T04:25:11Z

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

Fix For Spacing Around Toggle Buttons #39031

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

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

@brandonhenry requires payment (Needs manual offer from BZ)
@parasharrajat requires payment through NewDot Manual Requests

melvin-bot · 2024-04-18T04:25:13Z

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:

[@parasharrajat] The PR that introduced the bug has been identified. Link to the PR: Feat/workspace categories settings #37133
[@parasharrajat] 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: https://github.com/Expensify/App/pull/37133/files#r1570691293
[@parasharrajat] 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: N/A
[@parasharrajat] Determine if we should create a regression test for this bug.
[@parasharrajat] 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.
[@joekaufmanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: N/A

parasharrajat · 2024-04-18T13:00:56Z

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:

[@parasharrajat] The PR that introduced the bug has been identified. Link to the PR: Feat/workspace categories settings #37133
[@parasharrajat] 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: https://github.com/Expensify/App/pull/37133/files#r1570691293
[@parasharrajat] 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
[@parasharrajat] Determine if we should create a regression test for this bug. Yes
[@parasharrajat] 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

Open the Expensify app and switch the language to Spanish.
Navigate to any "collect" workspace.
Click on the categories option and then select settings.
Check that the text is wrapped properly and well aligned.

Do you agree 👍 or 👎 ?

joekaufmanexpensify · 2024-04-22T13:48:17Z

Great. TY! I'll complete my portion of the checklist.

joekaufmanexpensify · 2024-04-24T21:17:58Z

IMO this is a very niche bug and does not require a regression test.

joekaufmanexpensify · 2024-04-24T21:18:03Z

Checklist is all set.

joekaufmanexpensify · 2024-04-24T21:19:19Z

We need to issue the following payments:

@brandonhenry - $125 for PR via upwork.
@parasharrajat - $125 for C+ via Newdot.

joekaufmanexpensify · 2024-04-24T21:20:37Z

opened new job since other one is closed: https://www.upwork.com/jobs/~011f3cc2ed89c19798

joekaufmanexpensify · 2024-04-24T21:21:04Z

@brandonhenry offer sent for $125!

joekaufmanexpensify · 2024-04-24T21:21:45Z

@parasharrajat could you please request $125 via NewDot and confirm here once complete?

parasharrajat · 2024-04-25T06:31:27Z

@joekaufmanexpensify I will request it later. Feel free to close it.

joekaufmanexpensify · 2024-04-25T13:56:51Z

@brandonhenry $125 sent and contract ended!

joekaufmanexpensify · 2024-04-25T13:57:18Z

Payment summary is here for when @parasharrajat later requests payment. All set!

parasharrajat · 2024-05-20T11:41:42Z

Based on the thread https://expensify.slack.com/archives/C02NK2DQWUX/p1715710167738659?thread_ts=1715190852.677769&cid=C02NK2DQWUX, I think the price set on the issue is incorrect. It should be the base price at that time which was 500 at that time.

Given that we think it is a small issue, 250 sounds better. @joekaufmanexpensify thoughts?

joekaufmanexpensify · 2024-05-21T21:56:46Z

Brought to slack to discuss

joekaufmanexpensify · 2024-05-22T12:45:32Z

Discussed and landed on keeping the price as $125 here since I intentionally adjusted the price here because this was a very minor fix.

joekaufmanexpensify · 2024-05-22T12:45:47Z

Payment summary is here.

parasharrajat · 2024-06-14T05:27:33Z

Payment requested as per #38370 (comment)

JmillsExpensify · 2024-06-17T15:56:07Z

$125 approved for @parasharrajat

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

melvin-bot bot assigned joekaufmanexpensify Mar 15, 2024

joekaufmanexpensify added the External Added to denote the issue can be worked on by a contributor label Mar 15, 2024

melvin-bot bot changed the title ~~No spacing between explanatory text and toggle button in "settings"~~ [$500] No spacing between explanatory text and toggle button in "settings" Mar 15, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 15, 2024

melvin-bot bot assigned parasharrajat Mar 15, 2024

joekaufmanexpensify changed the title ~~[$500] No spacing between explanatory text and toggle button in "settings"~~ [$125] No spacing between explanatory text and toggle button in "settings" Mar 15, 2024

melvin-bot bot added the Overdue label Mar 18, 2024

melvin-bot bot removed the Overdue label Mar 18, 2024

melvin-bot bot assigned tylerkaraszewski Mar 19, 2024

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

melvin-bot bot changed the title ~~[$125] No spacing between explanatory text and toggle button in "settings"~~ [HOLD for payment 2024-04-25] [$125] No spacing between explanatory text and toggle button in "settings" Apr 18, 2024

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

parasharrajat referenced this issue Apr 18, 2024

feat: implement settings page

bf10806

parasharrajat mentioned this issue Apr 18, 2024

Feat/workspace categories settings #37133

Merged

50 tasks

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

joekaufmanexpensify closed this as completed Apr 25, 2024

[HOLD for payment 2024-04-25] [$125] No spacing between explanatory text and toggle button in "settings" #38370

[HOLD for payment 2024-04-25] [$125] No spacing between explanatory text and toggle button in "settings" #38370

Comments

m-natarajan commented Mar 15, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Mar 15, 2024

tienifr commented Mar 15, 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)

Result

joekaufmanexpensify commented Mar 15, 2024

melvin-bot bot commented Mar 15, 2024

melvin-bot bot commented Mar 15, 2024

joekaufmanexpensify commented Mar 15, 2024

melvin-bot bot commented Mar 15, 2024

brandonhenry commented Mar 15, 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?

Cases I've found

Example Screens With Viewport Adjusted To Show Issue

What alternative solutions did you explore? (Optional)

nayabatir1 commented Mar 17, 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)

parasharrajat commented Mar 18, 2024

brandonhenry commented Mar 18, 2024

Victor-Nyagudi commented Mar 18, 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)

parasharrajat commented Mar 19, 2024

brandonhenry commented Mar 19, 2024

parasharrajat commented Mar 19, 2024 • edited Loading

melvin-bot bot commented Mar 19, 2024

Victor-Nyagudi commented Mar 19, 2024

parasharrajat commented Mar 19, 2024

joekaufmanexpensify commented Mar 20, 2024

melvin-bot bot commented Apr 18, 2024

melvin-bot bot commented Apr 18, 2024

melvin-bot bot commented Apr 18, 2024 • edited by joekaufmanexpensify Loading

parasharrajat commented Apr 18, 2024

Regression Test Steps

joekaufmanexpensify commented Apr 22, 2024

joekaufmanexpensify commented Apr 24, 2024

joekaufmanexpensify commented Apr 24, 2024

joekaufmanexpensify commented Apr 24, 2024

joekaufmanexpensify commented Apr 24, 2024

joekaufmanexpensify commented Apr 24, 2024

joekaufmanexpensify commented Apr 24, 2024

parasharrajat commented Apr 25, 2024 • edited Loading

joekaufmanexpensify commented Apr 25, 2024

joekaufmanexpensify commented Apr 25, 2024

parasharrajat commented May 20, 2024

joekaufmanexpensify commented May 21, 2024

joekaufmanexpensify commented May 22, 2024

joekaufmanexpensify commented May 22, 2024

parasharrajat commented Jun 14, 2024

JmillsExpensify commented Jun 17, 2024

m-natarajan commented Mar 15, 2024 •

edited by melvin-bot bot

Loading

tienifr commented Mar 15, 2024 •

edited

Loading

brandonhenry commented Mar 15, 2024 •

edited

Loading

nayabatir1 commented Mar 17, 2024 •

edited

Loading

parasharrajat commented Mar 19, 2024 •

edited

Loading

melvin-bot bot commented Apr 18, 2024 •

edited by joekaufmanexpensify

Loading

parasharrajat commented Apr 25, 2024 •

edited

Loading