[$500] Workspace - No padding around workspace upload error

[kbecciv]

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.3.84.1
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: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1697206853776269

Action Performed:

1. Open the app
2. Open settings->workspaces->any workspace->settings
3. Click on profile and click upload
4. Upload file greater then 6mb
5. Observe error has no padding around
6. Try similar file upload in settings->profile->click on profile->upload
7. Observe that it has proper padding around error

Expected Result:

App should display proper padding around workspace upload errors like it does for user profile upload errors

Actual Result:

App does not display proper padding around workspace upload errors

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

Android: Native

Android.native.no.padding.workspace.upload.error.mp4

Android: mWeb Chrome

Android.chrome.no.padding.workspace.upload.error.mp4

iOS: Native

ios.native.no.padding.workspace.upload.error.mov

iOS: mWeb Safari

ios.safari.no.padding.workspace.upload.error.mov

MacOS: Chrome / Safari

windows.chrome.no.padding.around.workspace.upload.errors.mp4

mac.chrome.no.padding.workspace.upload.error.mov

Recording.5005.mp4

MacOS: Desktop

mac.desktop.no.padding.workspace.upload.error.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01226fa069ff423c98
• Upwork Job ID: 1713727632636690432
• Last Price Increase: 2023-10-30
• Automatic offers:
• esh-g | Contributor | 27451133
• dhanashree-sawant | Reporter | 27451134

[c3024]

Proposal

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

No margin around the error when uploading large image for workspace image upload but in profile pic update there is a proper margin

What is the root cause of that problem?

There is a horizontal margin for AvatarWithImagePicker here

App/src/pages/settings/Profile/ProfilePage.js

Line 135 in fe282b4

style={[styles.mh5]}

but not here

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 128 in fe282b4

style={[styles.mb3]}

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

We need to add the horizontal margin mh5 for the AvatarWithImagePicker of WorkspaceSettingsPage also.

What alternative solutions did you explore? (Optional)

[esh-g]

Proposal

Please re-state the problem we are trying to solve:

No padding around workspace upload error

What is the root cause of this problem?

The root cause is that there is no horizonal margin on the AvatarWithImagePicker component in WorkspaceSettingsPage:

Unlike the profile page with has margin on AvatarWithImagePicker:

I also noticed that the name field errorr on this page does not have margin as well. I believe this can be fixed as a part of this issue.

On almost every page with form, the Form has the style styles.ph5 but since this page has the currency picker that needs to span the width of the entire page, that style is not used here.

What changes should be made to fix this?

1. We should add the styles.ph5 style to this Form so that all the errors are padded. We do this on almost every page with a form.
   

   App/src/pages/workspace/WorkspaceSettingsPage.js

   Lines 101 to 105 in 834b3d2

   	<Form
   	formID={ONYXKEYS.FORMS.WORKSPACE_SETTINGS_FORM}
   	submitButtonText={translate('workspace.editor.save')}
   	style={styles.flexGrow1}
   	submitButtonStyles={[styles.mh5]}

2. We should remove the styles.mh5 from the name text input here:
   

   App/src/pages/workspace/WorkspaceSettingsPage.js

   Line 148 in 834b3d2

   containerStyles={[styles.mt4, styles.mh5]}

3. Since we need the currency selector to span the entire width, we should add the styles.mhn5 to the following View which adds negative margin so that it can span the entire length.
   

   App/src/pages/workspace/WorkspaceSettingsPage.js

   Line 153 in 834b3d2

   <View style={[styles.mt4]}>

If you are having doubts about using negative margin, here are some examples of it being used in pages like this where there are both text inputs with padding and value selectors that need to span full width.

In WorkspaceNewRoomPage

App/src/pages/workspace/WorkspaceNewRoomPage.js

Line 215 in 834b3d2

<View style={[styles.mhn5]}>

App/src/pages/workspace/WorkspaceNewRoomPage.js

Lines 224 to 225 in 834b3d2

	{isPolicyAdmin && (
	<View style={styles.mhn5}>

App/src/pages/workspace/WorkspaceNewRoomPage.js

Line 235 in 834b3d2

<View style={[styles.mb1, styles.mhn5]}>

In AddressPage

App/src/pages/settings/Profile/PersonalDetails/AddressPage.js

Line 212 in 834b3d2

<View style={styles.mhn5}>

[namhihi237]

Proposal

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

The app should display proper padding around workspace upload errors as user profile upload errors

What is the root cause of that problem?

Currently, we do not have padding paddingHorizontal for AvatarWithImagePicker in WorkspaceSettingsPage, which mean when the long error show we can easily see

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 128 in dc1ff84

style={[styles.mb3]}

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

We should add ph5 into AvatarWithImagePicker the same as with the profile page here

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 128 in dc1ff84

style={[styles.mb3]}

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01226fa069ff423c98

[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

[saranshbalyan-1234]

Proposal

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

No padding around workspace upload error

What is the root cause of that problem?

We have not passed styles for horizontal margin here.

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 128 in 749f17f

style={[styles.mb3]}

unlike the profile page where we are sending the horizontal style here.

App/src/pages/settings/Profile/ProfilePage.js

Line 135 in ba992bb

style={[styles.mh5]}

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

We need to add horizontal margin in the style prop of AvatarWithImagePicker when calling in WorkspaceSettingsPage like this.

style={[styles.mb3,styles.mh5]}

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

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

[dukenv0307]

Proposal

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

No padding around workspace upload error

What is the root cause of that problem?

We have no horizontal padding or margin style for AvatarWithImagePicker in WorkspaceSettingPage

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 128 in 0e64161

style={[styles.mb3]}

That is added here in ProfilePage

App/src/pages/settings/Profile/ProfilePage.js

Line 135 in 0e64161

style={[styles.mh5]}

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

To consistent the AvatarWithImagePicker component between two page, we can add style styles.mh5 in WorkspaceSettingPage

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 128 in 0e64161

style={[styles.mb3]}

And add style styles.mb3 in ProfilePage.

App/src/pages/settings/Profile/ProfilePage.js

Line 135 in 0e64161

style={[styles.mh5]}

That means both page with add style prop with value as {[styles.mb3,styles.mh5]} into AvatarWithImagePicker

What alternative solutions did you explore? (Optional)

[PiyushChandra17]

Proposal

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

Workspace - No padding around workspace upload error

What is the root cause of that problem?

We are applying mh5 here in AvatarWithImagePicker component in profile page,

App/src/pages/settings/Profile/ProfilePage.js

Line 135 in 0e64161

style={[styles.mh5]}

But we are not applying this style mh5 in workspace

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

We need to apply mh5 in AvatarWithImagePicker component in workspace page aswell like we did in profile page,

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 128 in 0e64161

style={[styles.mb3]}

What alternative solutions did you explore? (Optional)

NA

Result:

[abdel-h66]

Proposal

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

Workspace settings avatar container has no horizontal paddings

What is the root cause of that problem?

For both profile settings and workspace settings, we are using the same component AvatarWithImagePicker to display the handle the display and upload of avatar. In the profile settings page we are passing horizontal paddings to the component, we should do the same in the Workspace settings

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

<AvatarWithImagePicker
    style={[styles.mb3, styles.mh5]}

What alternative solutions did you explore? (Optional)

I noticed some differences in margins of the profile page vs the workspace settings page.

Profile settings page:
The AvatarWithImagePicker has styles.mh5
The section right under it has styles.mt4

Workspace settings page
If we apply suggestion above AvatarWithImagePicker will have style={[styles.mb3, styles.mh5]}

I suggest removing the styles.mt4 from the View in the profile settings page.
And moving it to the AvatarWithImagePicker to have [styles.mb3, styles.mh5]

[peterdbarkerUK]

One note for future reference: expensify.com user accounts will get the MISSING TRANSLATION error here instead.

[dhairyasenjaliya]

Proposal

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

•  Workspace name Red dot error does not align horizontally with the text field

What is the root cause of that problem?

• The root cause is we have not added marginHorizontal directly to <Form> instead we have added it on the separate component which is placed inside <Form> making the error style to broke

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

• Firstly we need to remove submitButtonStyles style from <Form> which has mh5 only to the submitButton and not to the entire <Form>

  App/src/pages/workspace/WorkspaceSettingsPage.js

  Line 105 in 91e2120

  submitButtonStyles={[styles.mh5]}

• After that we need to add mh5 directly to <Form> style to apply on entire <Form>
  

  App/src/pages/workspace/WorkspaceSettingsPage.js

  Line 104 in 91e2120

  style={styles.flexGrow1}

• After that, we need to clean up the extra horizontal style applied to multiple components which is redundant as we already added directly to <Form> which the above proposal is missing which can cause regression (at the time of adding this non of the proposal added this)

  App/src/pages/workspace/WorkspaceSettingsPage.js

  Line 148 in 91e2120

  containerStyles={[styles.mt4, styles.mh5]}

  

  

  App/src/pages/workspace/WorkspaceSettingsPage.js

  Line 161 in 91e2120

  <Text style={[styles.textLabel, styles.colorMuted, styles.mt2, styles.mh5]}>

• Once we added mh5 to <Form> we need to adjust styling of MenuItemWithTopDescription by adding wrapperStyle and we need to add both the styles as styles.mhn5, styles.wAuto seems like the component already had mh5 by default so we need to remove and the same condition can be found on other pages like WorkspaceReimburseView.js

  

  App/src/pages/workspace/reimburse/WorkspaceReimburseView.js

  Line 149 in 91e2120

  wrapperStyle={[styles.mhn5, styles.wAuto]}

What alternative solutions did you explore? (Optional)

• N/A

Result

[melvin-bot]

@eVoloshchak, @peterdbarkerUK Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[peterdbarkerUK]

@eVoloshchak could you take a look at these proposals? Thanks!

[eVoloshchak]

I think we should proceed with @c3024's proposal, as this was the first proposal to fund the root cause for this and propose a valid fix
The fix is simple and straightforward with zero potential regressions
There are a couple of proposals that extend @c3024's proposal finding further inconsistencies on this page, but those should be reported and handled in a separate issue.

🎀👀🎀 C+ reviewed!

[melvin-bot]

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

[esh-g]

@eVoloshchak if we create a separate issue for those inconsistencies, we will end up undoing what we are going to do here... because to fix the whole page, we need to add padding to the whole form and if we do that, the change that @c3024 proposed (of adding margin to avatar) will need to reversed...

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[Beamanator]

Hmm I'm actually leaning toward @esh-g 's proposal here - because it is an attempt to make the entire page layout similar to other pages - Plus it fixes that additional name error message issue. What do you think, @eVoloshchak ?

[melvin-bot]

@Beamanator, @eVoloshchak, @peterdbarkerUK Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[Beamanator]

Bump @eVoloshchak

[eVoloshchak]

I'm actually leaning toward @esh-g 's proposal #29652 (comment) - because it is an attempt to make the entire page layout similar to other pages - Plus it fixes that additional name error message issue

Yeah, that is a good point. While this isn't the first proposal, it will allow us to make the whole page consistent with the rest of the app, and the UX is more important
Let's proceed with @esh-g's proposal

[melvin-bot]

@Beamanator @eVoloshchak @peterdbarkerUK this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

📣 @eVoloshchak Please request via NewDot manual requests for the Reviewer role ($500)

[melvin-bot]

📣 @esh-g 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[esh-g]

PR is here: #30638

cc @eVoloshchak

[peterdbarkerUK]

PR deployed to production without issue.

UpWork job

Payment summary:

• $50 bug report to @dhanashree-sawant
• $500 contributor to @esh-g
• $500 to reviewer @eVoloshchak (paid via NewDot, CC @JmillsExpensify )

@eVoloshchak please request payment in NewDot (assigned to @JmillsExpensify for payout)

[JmillsExpensify]

Haven't received a money request yet, though feel free to close this out and I'll comment with approval once I receive it via NewDot.

[eVoloshchak]

Requested the payment

[JmillsExpensify]

Alright, missed this earlier. @peterdbarkerUK Can you please update that payment summary to clarify the correct payment amount for @eVoloshchak? Thanks!

[peterdbarkerUK]

Updated! It should be $500.

[JmillsExpensify]

$500 payment approved for @eVoloshchak based on comment above.

[peterdbarkerUK]

Thanks Jason!