Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment Aug 18] Add Phone Number row doesn't follow the correct styling #4528

Closed
isagoico opened this issue Aug 10, 2021 · 11 comments
Assignees
Labels
Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@isagoico
Copy link

isagoico commented Aug 10, 2021

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

Upwork post: https://www.upwork.com/jobs/~015cb8be422e99db23


Action Performed:

  1. Navigate to profile settings
  2. Check the Add Phone number line

Expected Result:

Add phone number line should be the same styling as the rest of the profile settings page.

Actual Result:

Styling is different, notice how the text is not bold and that there is no padding above/below the row.

Workaround:

None needed, visual issue.

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.0.83-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

image

Expensify/Expensify Issue URL:

View all open jobs on Upwork


From @shawnborton https://expensify.slack.com/archives/C01GTK53T8Q/p1628580981143900

The Add Phone Number option row does not follow the same styling as other option rows. Notice how the text is not bold and that there is no padding above/below the row.

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented Aug 10, 2021

Proposal

Need to change in LoginField.js file.
We can replace the following lines,

<Pressable
style={[styles.createMenuItem, styles.ph0]}
onPress={() => Navigation.navigate(ROUTES.getSettingsAddLoginRoute(this.props.type))}
>
<View style={styles.flexRow}>
<View style={styles.createMenuIcon}>
<Icon src={Plus} />
</View>
<View style={styles.justifyContentCenter}>
<Text style={[styles.createMenuText, styles.ml3]}>
{`${this.props.translate('common.add')} ${this.props.label}`}
</Text>
</View>
</View>
</Pressable>

With our existing MenuItem component as below.
Screenshot 2021-08-10 at 11 06 35 PM

Post Fix:
Screenshot 2021-08-10 at 11 13 27 PM

@Santhosh-Sellavel
Copy link
Collaborator

@sketchydroide
Proposed changes are available in drafted PR. Additionally after discussion with @shawnborton here, removed left & right padding. Those changes are also included in the drafted PR.
Once you say go head, I'll wrap my PR work & mark ready for review thanks.

@sketchydroide sketchydroide added Weekly KSv2 and removed Daily KSv2 labels Aug 11, 2021
@sketchydroide
Copy link
Contributor

Yeah I'm happy with that solution, go on and make the PR ready for review 👍🏼

@Santhosh-Sellavel
Copy link
Collaborator

Thanks for the confirmation, @sketchydroide, I'll add screenshots and mark PR ready for review.

Kindly create an Upwork job for this thanks!

@sketchydroide sketchydroide added the External Added to denote the issue can be worked on by a contributor label Aug 11, 2021
@MelvinBot
Copy link

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

@MelvinBot MelvinBot added Daily KSv2 and removed Weekly KSv2 labels Aug 11, 2021
@Jag96 Jag96 added the Exported label Aug 11, 2021
@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 11, 2021
@Jag96 Jag96 removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 11, 2021
@Jag96
Copy link
Contributor

Jag96 commented Aug 11, 2021

@Santhosh-Sellavel please be sure to read our contributing guidelines, next time please do not create a PR for the issue until your proposal has been approved.

I've exported the issue to Upwork, please apply here!

@Jag96 Jag96 added Weekly KSv2 and removed Daily KSv2 labels Aug 11, 2021
@Santhosh-Sellavel
Copy link
Collaborator

@Santhosh-Sellavel please be sure to read our contributing guidelines, next time please do not create a PR for the issue until your proposal has been approved.

I've exported the issue to Upwork, please apply here!

@Jag96
Sorry , I already have a couple more PR in draft state. Will not open any further until proposal is accepted.

@Santhosh-Sellavel
Copy link
Collaborator

@Jag96 Applied for job!

@Jag96
Copy link
Contributor

Jag96 commented Aug 11, 2021

Sorry , I already have a couple more PR in draft state. Will not open any further until proposal is accepted.

Thanks, for those issues can you please close any draft PRs that don't have an accepted proposal? Then, if your proposal ends up being accepted you can just re-open the associated draft.

@Jag96 Jag96 added the Reviewing Has a PR in review label Aug 11, 2021
@Santhosh-Sellavel
Copy link
Collaborator

Sorry , I already have a couple more PR in draft state. Will not open any further until proposal is accepted.

Thanks, for those issues can you please close any draft PRs that don't have an accepted proposal? Then, if your proposal ends up being accepted you can just re-open the associated draft.

Done! @Jag96

@Jag96 Jag96 changed the title Add Phone Number row doesn't follow the correct styling [HOLD for payment Aug 18] Add Phone Number row doesn't follow the correct styling Aug 13, 2021
@Jag96
Copy link
Contributor

Jag96 commented Aug 18, 2021

Paid!

@Jag96 Jag96 closed this as completed Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

5 participants