[HOLD for payment 2021-12-15] Wrong margin between pronoun form elements

[anthony-hull]

Action Performed:

Choose self-select in the preferred pronouns section of your settings

Expected Result:

Spacing between preferred pronouns input field and the text input field below to be the same as those between First name and Last name.

Actual Result:

Spacing is less

Platform:

Where is this issue occurring?

• Web - only checked web. Likely not platform specific as it's just css margin
• iOS
• Android
• Desktop App
• Mobile Web

Version Number:
v1.1.1-8
Reproducible in staging?:
Reproducible in production?:
yes

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @lschurr (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[anthony-hull]

Proposal:
use styles.mt2 in:

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

Lines 239 to 243 in b37e87e

	<ExpensiTextInput
	value={this.state.selfSelectedPronouns}
	onChangeText={selfSelectedPronouns => this.setState({selfSelectedPronouns})}
	placeholder={this.props.translate('profilePage.selfSelectYourPronoun')}
	/>

as this is the value used as margins between inputs in FullNameInputRow:

App/src/components/FullNameInputRow.js

Lines 54 to 62 in b37e87e

	<View style={[styles.flex1, styles.ml2]}>
	<ExpensiTextInput
	label={translate('common.lastName')}
	value={lastName}
	onChangeText={onChangeLastName}
	placeholder={translate('profilePage.doe')}
	translateX={-10}
	/>
	</View>

[MelvinBot]

Triggered auto assignment to @marcochavezf (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[anthony-hull]

I can see there is a translateX as well. I would have a look at if this affects the spacing vs just the margin style and make sure the spacing is the same.

[marcochavezf]

👍🏽 Adding external label

[MelvinBot]

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

[PrashantMangukiya]

Proposed Solution:

I tested that directly applying style to <ExpensiTextInput /> is not working as expected. So better solution is to wrap <ExpensiTextInput /> within a <View> and apply style to view. It is works perfect on all platform.

i.e. Current code for ProfilePage.js is as under:

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

Lines 238 to 244 in f9f0b3f

	{this.state.pronouns === this.props.translate('pronouns.selfSelect') && (
	<ExpensiTextInput
	value={this.state.selfSelectedPronouns}
	onChangeText={selfSelectedPronouns => this.setState({selfSelectedPronouns})}
	placeholder={this.props.translate('profilePage.selfSelectYourPronoun')}
	/>
	)}

We have to Wrap <ExpensiTextInput /> (line 239-243) within View as under, Also we need to apply styles.mt1 because some top margin comes from ExpensiTextInput component, so adding styles.mt1 is enough, so space looks similar to between first name and lastname.

{this.state.pronouns === this.props.translate('pronouns.selfSelect') && ( 
  <View style={styles.mt1}>
      <ExpensiTextInput
          value={this.state.selfSelectedPronouns}
          onChangeText={selfSelectedPronouns => this.setState({selfSelectedPronouns})}
          placeholder={this.props.translate('profilePage.selfSelectYourPronoun')}
      />
  </View>
)}

Below are screenshot after updates done as suggested, It is working perfect on all platform.

Web

Mobile Web

Desktop

iOS

Android

If my solution accepted then kindly message me so I can submit pr immediately.

[MelvinBot]

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

[anthony-hull]

This is the sort of minor tweak I would have done when implementing. I consider a proposal more of an approach rather than a full implementation else you end up doing the work before getting hired on each job.

Is the expectation on these small jobs that you implement before proposing?

[MelvinBot]

@kevinksullivan Still overdue 6 days?! Let's take care of this!

[kevinksullivan]

Apologies for the delay. I would think the expectation is to get your proposal approved prior to implementation. Job posted here:

https://www.upwork.com/jobs/~010ec755aca3433065

[nvdnvd00]

My Proposed Solution:

The marginBottom of Preferred Pronouns is unnecessary, I wrap selfSelectedPronouns by a View with marginTop, so this spacing only display when input available

Web

[MelvinBot]

Triggered auto assignment to @parasharrajat (Exported)

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to @roryabraham (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[kevinksullivan]

Apologies @anthony-hull , looks like this was dropped over time. I reapplied labels to get reviews in motion.

[parasharrajat]

Thanks, everyone for the proposals. I would go with @anthony-hull proposal for the reason that It's a minor change and UI can be adjusted to look as expected. he also reported the issue.
cc: @puneetlath

[MelvinBot]

📣 @anthony-hull You have been assigned to this job by @puneetlath!
Please apply to this job in Upwork 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 📖

[puneetlath]

Sounds good to me. Thanks everyone for your proposals. I've gone ahead assigned the issue to you @anthony-hull.

@kevinksullivan can you hire @anthony-hull in Upwork please?

[kevinksullivan]

Needed a repost. @anthony-hull i sent you an invte to the job. Here's the link

https://www.upwork.com/jobs/~014c6783114d6e9ab8

[botify]

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

• change pronoun input margin to value for grouped form elements #6586

If no regressions arise, payment will be issued on 2021-12-15. 🎊

[kevinksullivan]

Sorry @anthony-hull , I just had to send you an offer that you have to accept. Once you accept the offer I'll get this paid!

[anthony-hull]

Thank you! I've accepted

[mallenexpensify]

Happened to be trolling cruising by and wanted to get this paid before the weekend. Paid @anthony-hull $250 for the job and $250 for the reporting bonus. Thanks for the help.