[$1000] Workspace - Margin of error is slightly shifted to the left compared to other items

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Issue found when executing PR #21662

Action Performed:

1. Open Desktop app
2. Create a Workspace X as user A
3. Invite user B to the Workspace
4. Login as same user A on Web
5. Copy URL of the created Workspace X on Web
6. Login as user B in incognito
7. As user B create a public room for Workspace X
8. As user A on Desktop app delete the Workspace X
9. As user A on Web Open link from step 5
10. As user A on Web accept redirection to desktop app

Expected Result:

The margin of error is consistent with the workspace item margin

Actual Result:

The margin of the error is slightly shifted to the left (Red circle is not aligned with other items)

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.36.4

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6116580_Recording__19.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014ed9881792dde5c5
• Upwork Job ID: 1676594232511758336
• Last Price Increase: 2023-07-05

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[jeet-dhandha]

Proposal

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

• Auth DeletePolicy error's Dot indicator is not aligned properly with other icons.

What is the root cause of that problem?

• Margin is not set properly for the Dot indicator.

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

• We can set marginLeft: 10 in errorRowStyles of OfflineWithFeedback in WorkspaceInitialPage.js.
• I tried using default values like ml2, ml3. But 10 value was perfect for this scenario.

Screenshot after changes

What alternative solutions did you explore? (Optional)

• N/A

[Nikhil-Vats]

Proposal

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

Workspace - Dot error icon is not left aligned with other items.

What is the root cause of that problem?

WorkspaceInitialPage uses MenuItem which uses styles.popoverMenuIcon for styling icons of menu items.

App/src/components/MenuItem.js

Line 152 in 11b8908

<View style={[styles.popoverMenuIcon, ...props.iconStyles]}>

styles.popoverMenuIcon has a width of 40px and alignment properties for centering the icon horizontally and vertically.

App/src/styles/styles.js

Lines 1309 to 1313 in 11b8908

	popoverMenuIcon: {
	width: variables.componentSizeNormal,
	justifyContent: 'center',
	alignItems: 'center',
	},

The error message on WorkspaceInitialPage is shown by passing errors={policy.errors} to OfflineWithFeedback which renders DotIndicatorMessage.

App/src/components/DotIndicatorMessage.js

Lines 57 to 62 in 11b8908

	<View style={styles.offlineFeedback.errorDot}>
	<Icon
	src={Expensicons.DotIndicator}
	fill={props.type === 'error' ? colors.red : colors.green}
	/>
	</View>

DotIndicatorMessage uses only styles.errorDot for styling container of icon due to which there is a mismatch in the styles used for menu item icons and error dot icon.

App/src/styles/styles.js

Lines 2846 to 2848 in 11b8908

	errorDot: {
	marginRight: 12,
	},

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

We can't simply add styles.popoverMenuIcon to container of icon in DotIndicatorMessage as it will break styling on other pages like WorkspaceMembersPage.

We should create a new prop named errorIconContainerStyles in OfflineWithFeedback and DotIndicatorMessage.
From WorkspaceInitialPage we can pass a new prop to OfflineWithFeedback -

errorIconContainerStyles={[styles.popoverMenuIcon]}

Then in OfflineWithFeedback we simply pass on the prop to DotIndicatorMessage.

In DotIndicatorMessage we can add this prop to the container of icon. We can change line 57 to -

<View style={[styles.offlineFeedback.errorDot, ...props.errorIconContainerStyles]}>

This will fix the issue while also not causing any regressions on other pages like WorkspaceMembersPage.

What alternative solutions did you explore? (Optional)

NA

Result -

Screen.Recording.2023-07-05.at.12.05.58.PM.mov

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014ed9881792dde5c5

[melvin-bot]

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

[melvin-bot]

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

[thesahindia]

Let's fix this for payments page too. I think we should also align the dismiss button.

@Nikhil-Vats, can you look into other pages as well? Le't fix all the instances here.

[Nikhil-Vats]

Sure, I will come up with a list today and update the proposal.

[Nikhil-Vats]

@thesahindia Here is the list of components that use OfflineWithFeedback to show errors -

1. IOUPreview

2. ReportActionItem

3. ReportActionItemCreated

4. ReportActionItemParentAction

5. ContinueBankAccountSetup

6. EnableStep

7. PaymentMethodList

8. BasePaymentsPage

9. ContactMethodDetailsPage

10. BaseValidateCodeForm

11. ReportSettingsPage

12. WorkspaceInitialPage

13. WorkspaceMembersPage

14. WorkspacesListPage

15. WorkspaceRateAndUnitPage

16. AvatarWithImagePicker - Definitely needs fixing

We were not showing errors for this component before but in this PR, AvatarWithImagePicker was changed to use OfflineWithFeedback and errors prop was passed (though it had nothing to do with PR) so that's why this looks completely messed up because it was not intentionally added -

[Nikhil-Vats]

The following pages have alignment issues in my opinion -
5, 6 - left align the red dot with icons in buttons
8 - both errors need alignment
12 (this issue) - left align red dot with icons
13, 15 - align close icon
16 - remove error prop as we were not showing it before and it was added in this PR unintentionally.

[thesahindia]

Cool, thanks for looking into it.

@Nikhil-Vats's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[puneetlath]

@madmax330 thoughts on the proposal?

[madmax330]

Proposal looks good, but
Cc @shawnborton can you confirm that this looks good before we move forward?

[shawnborton]

I actually don't think this is a bug. We used to try to get cheeky with making sure the red dot lined up with the content above it, but we ran into way too many different cases where sometimes the item above was further right than others. So in the spirit of consistency, we just made it so we have one default RBR error message style, which is what you see. Persnoally I don't think this looks broken, and I think we should just close this out.

[madmax330]

Alright thanks, closing