[$1000] Inconsistent margins in Home address form

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go to settings -> Profile
2. Personal details -> Home address

Expected Result:

Margins below each field should be consistent

Actual Result:

Margins below each fields are not consistent.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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.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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-04-16.at.6.27.55.PM.mov

Expensify/Expensify Issue URL:
Issue reported by: @DinalJivani
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681652095740999

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017ab6742c47754266
• Upwork Job ID: 1650976064170250240
• Last Price Increase: 2023-04-25

[MelvinBot]

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

[MelvinBot]

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

[MitchExpensify]

@kavimuru what pixel size spacing do we expect for these margins?

[MelvinBot]

@MitchExpensify Eep! 4 days overdue now. Issues have feelings too...

[MitchExpensify]

This does look inconsistent and I am able to reproduce.

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~017ab6742c47754266

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[hoangzinh]

Proposal

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

Inconsistent margins in Home address form

What is the root cause of that problem?

All of wrapper field in this form has been added same margin-bottom number in those lines. So it's good. But according to recording in GH issue, there are 2 places showing inconsistent margin:

1. The zip code field => Root cause is this field having hint text, it adds an extra margin bottom (code ref)
   

2. Country field => The inconsistent margin visualize is not come from this field itself, but come from SubmitButton, where it set margin-top to 20px
   

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

To fix:

1. I think we can remove margin-bottom style in this line. Because it should not carry margin-bottom itself, otherwise it will lead to adding extra spaces like this case. Even with the main component (Input/Checkbox...), it doesn't have default margin-bottom. The margin should be proactive set where it's needed (Or If we want a safer way, we can adding a additional prop hintStyle into TextInput component, so we can adding custom styles for hint message)
2. I don't think we should do anything in this component. Because extra margin is not come from itself.

[ahmedGaber93]

Proposal

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

Inconsistent margins in Home address form for input zip code

What is the root cause of that problem?

every input in the form has margin-bottom: 16px; but zip code have extra margin by FormHelpMessage which add margin-bottom: 4px;

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

remove margin-bottom from FormHelpMessage component it self, or add prop style HelpMessageStyle for TextInput and pass it down to FormHelpMessage.
we can also decrease FormHelpMessage marign-top to be 4px rather than 8px to show constant, but i think no need to remove it

What alternative solutions did you explore? (Optional)

[dukenv0307]

Proposal

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

Inconsistent margins in forms: Company Step, Personal information, Home Address Form,...

What is the root cause of that problem?

• The help message have margin bottom 4px. That makes us think that the margin of each field that has help message is larger
  

  App/src/components/FormHelpMessage.js

  Line 39 in 76c9338

  <View style={[styles.flexRow, styles.alignItemsCenter, styles.mt2, styles.mb1, ...props.style]}>

• With the last field of a form it doesn't have margin bottom, that margin bottom belongs to the container of submit buttom submit buttom and it have margin top and bottom is 20px
  

  App/src/components/Form.js

  Line 365 in 76c9338

  containerStyles={[styles.mh0, styles.mt5, styles.flex1]}

  
  

  App/src/components/FormAlertWithSubmitButton.js

  Line 60 in 76c9338

  containerStyles={[styles.mh5, styles.mb5, styles.justifyContentEnd, ...props.containerStyles]}

  
  This is base component of form so that many forms in product have same issue.

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

To be consistent, we should remove margin bottom of help message in here

App/src/components/FormHelpMessage.js

Line 39 in 76c9338

<View style={[styles.flexRow, styles.alignItemsCenter, styles.mt2, styles.mb1, ...props.style]}>

And change margin top and bottom of the container of submit buttom in here to 16px

App/src/components/Form.js

Line 365 in 76c9338

containerStyles={[styles.mh0, styles.mt5, styles.flex1]}

App/src/components/FormAlertWithSubmitButton.js

Line 60 in 76c9338

containerStyles={[styles.mh5, styles.mb5, styles.justifyContentEnd, ...props.containerStyles]}

In RequestorStep.js I saw that micro text has margin has margin bottom 20px while each field in form has margin bottom 16px. Is this an inconsistent? If yes, we also need to fix it.

App/src/pages/ReimbursementAccount/RequestorStep.js

Line 132 in f3fae40

<View style={[styles.mb5, styles.mt1, styles.dFlex, styles.flexRow]}>

What alternative solutions did you explore? (Optional)

NA

[thesahindia]

It feels like a minor issue. I think we ask @shawnborton for their thoughts.

@shawnborton, do you think we should work on this?

[shawnborton]

Technically this is a design bug but I feel like we should just fix this internally quickly. cc @Beamanator @cristipaval I think we just want 20px below each input grouping (could be just an input, or input + hint text).

[cristipaval]

yes, this can be quickly fixed in the PR on which @gedu is working atm to align the Home Address screen to the push-to-page design. Could we assign @gedu to this one as well?

[Beamanator]

@cristipaval that makes sense, but you can probably also just include this requirement in their PR and close this out now 🤷 Either way sounds reasonable to me 👍

[cristipaval]

just include this requirement in their PR and close this out now

Great idea!

[cristipaval]

I'll actually reopen it and let the asignees close it when they consider, as I think there is a reporting bonus to pay.

[MitchExpensify]

offer sent to @DinalJivani for reporting bonus payment, will close this out when paid!

[DinalJivani]

@MitchExpensify
Offer accepted.

[MitchExpensify]

Paid! Thanks