Skip to content

Fix Additional Information Text Margin #5363

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

Merged
merged 3 commits into from
Sep 21, 2021
Merged

Conversation

luacmartins
Copy link
Contributor

@luacmartins luacmartins commented Sep 20, 2021

Details

This PR fixes the margin on the additional information page.

Fixed Issues

$ #5343

Tests

  1. Login to NewDot using an account with a Workspace and without the Expensify Card.
  2. Navigate to '/bank-account/contract' or follow these steps to get to the additional information step.
  3. Verify that the horizontal margins look like the images below.

QA Steps

Steps above.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web

Mobile Web

mweb

Desktop

desktop

iOS

ios

Android

android

@luacmartins luacmartins requested a review from a team September 20, 2021 19:24
@luacmartins luacmartins self-assigned this Sep 20, 2021
@MelvinBot MelvinBot requested review from Dal-Papa and removed request for a team September 20, 2021 19:24
@@ -224,7 +224,7 @@ class BeneficialOwnersStep extends React.Component {
)}
</View>
)}
<Text style={[styles.textStrong, styles.mb5]}>
<Text style={[styles.textStrong, styles.mv5]}>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a top margin to separate "Agreement" from the top checkboxes and make it look more like @shawnborton's image.

@@ -235,7 +235,7 @@ class BeneficialOwnersStep extends React.Component {
<View style={[styles.flexRow]}>
<Text>{this.props.translate('common.iAcceptThe')}</Text>
<TextLink href="https://use.expensify.com/achterms">
{`${this.props.translate('beneficialOwnersStep.termsAndConditions')}.`}
{`${this.props.translate('beneficialOwnersStep.termsAndConditions')}`}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed "." to make it consistent with the other checkbox items.

@shawnborton
Copy link
Contributor

I think you can also make the "Agreement" use a normal font weight and not bold.

@luacmartins luacmartins requested a review from a team as a code owner September 20, 2021 19:36
@luacmartins luacmartins removed the request for review from a team September 20, 2021 19:36
@luacmartins
Copy link
Contributor Author

Updated!

@luacmartins luacmartins removed the request for review from MonilBhavsar September 20, 2021 19:39
@shawnborton
Copy link
Contributor

Looks good, thanks Carlos!

@Dal-Papa Dal-Papa merged commit ef46b12 into main Sep 21, 2021
@Dal-Papa Dal-Papa deleted the cmartins-additionalInfoMargin branch September 21, 2021 08:36
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @Dal-Papa in version: 1.1.0-3 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @Jag96 in version: 1.1.1-8 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants