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

Fix text styles by removing uses of the non-custom text component #3848

Merged
merged 8 commits into from
Jul 9, 2021

Conversation

roryabraham
Copy link
Contributor

@roryabraham roryabraham commented Jul 1, 2021

Details

Enforces the use of our custom text component throughout the app instead of the default RN text component. This ensures that our default styles are used. Also removes styles.textP, which is redundant when the default custom text component is used.

Fixed Issues

$ #3387

Tests / QA Steps

Click through the various screens and components of the app - verify that everywhere you see the correct use of our fonts. Pro-tip: take a look at the g's – they look noticeably different in GT America with this distinctive tail:

image

Pay particular attention to IOU actions and previews, because we know these were previously using the incorrect font.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

Mobile Web

image

Desktop

image

image

iOS

image

Android

image

@roryabraham roryabraham requested a review from shawnborton July 1, 2021 02:05
@roryabraham roryabraham self-assigned this Jul 1, 2021
@roryabraham roryabraham requested a review from a team as a code owner July 1, 2021 02:05
@MelvinBot MelvinBot requested review from MonilBhavsar and removed request for a team July 1, 2021 02:05
@roryabraham
Copy link
Contributor Author

Because 59 files were changed here, this will be a nightmare to keep up-to-date. Quick reviews appreciated 🙇

MonilBhavsar
MonilBhavsar previously approved these changes Jul 1, 2021
Copy link
Contributor

@MonilBhavsar MonilBhavsar left a comment

Choose a reason for hiding this comment

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

Looks good!
Long but simple changes

# Conflicts:
#	src/components/PopoverMenu/BasePopoverMenu.js
#	src/pages/home/HeaderView.js
#	src/pages/home/report/EmojiPickerMenu/index.native.js
#	src/pages/home/report/ReportActionCompose.js
#	src/pages/settings/Payments/AddPayPalMePage.js
@roryabraham
Copy link
Contributor Author

Fixed merge conflicts, going to need another review @MonilBhavsar.

MonilBhavsar
MonilBhavsar previously approved these changes Jul 9, 2021
Copy link
Contributor

@MonilBhavsar MonilBhavsar left a comment

Choose a reason for hiding this comment

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

Looks good!
@shawnborton all yours

shawnborton
shawnborton previously approved these changes Jul 9, 2021
@shawnborton
Copy link
Contributor

Looks like we have a conflict.

# Conflicts:
#	src/pages/settings/Profile/ProfilePage.js
@roryabraham roryabraham dismissed stale reviews from shawnborton and MonilBhavsar via b3b3d69 July 9, 2021 17:00
@roryabraham
Copy link
Contributor Author

Conflicts resolved!

@MonilBhavsar MonilBhavsar merged commit 2e81800 into main Jul 9, 2021
@MonilBhavsar MonilBhavsar deleted the Rory-FixFonts branch July 9, 2021 17:46
@OSBotify
Copy link
Contributor

OSBotify commented Jul 9, 2021

✋ 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 production in version: 1.0.77-5🚀

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