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 dynamic text on settings headers and footers #800

Merged
merged 1 commit into from
Dec 18, 2020

Conversation

THISISDINOSAUR
Copy link
Contributor

@THISISDINOSAUR THISISDINOSAUR commented Dec 17, 2020

Task/Issue URL: https://app.asana.com/0/414709148257752/1199241778510997/f
Tech Design URL:
CC:

Description:
Fixes the text sizes of the headers on footers on the GPC settings page, and the Unprotected sites pages

Steps to test this PR:

  1. Test both pages with a variety of text sizes

Copy Testing:

  • Use of correct apostrophes in new copy, ie rather than '

Orientation Testing:

  • Portrait
  • Landscape

Device Testing:

  • iPhone SE (1st Gen)
  • iPhone 8
  • iPhone X
  • iPad

OS Testing:

  • iOS 11
  • iOS 12
  • iOS 13
  • iOS 14

Theme Testing:

  • Light theme
  • Dark theme

Internal references:

Software Engineering Expectations
Technical Design Template

Copy link
Contributor

@samsymons samsymons left a comment

Choose a reason for hiding this comment

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

Looks solid to me! I only ran into a couple tiny issues, neither of which are blockers:

  1. The text size doesn't change in the GPC settings if you're already on that screen when Dynamic Type changes. If you back out of it to the main settings screen and then back in again, it's fine, so this is a bit of an edge case – not a blocker. 🙂
  2. I changed the Dynamic Type setting in an iOS 12 simulator while on the main settings screen and it caused a few labels to vanish. I don't think this is the fault of this PR, and it doesn't happen on later iOS versions. Dismissing the settings screen then showing it again fixes it.

Simulator Screen Shot - iPhone 5s - 2020-12-17 at 21 08 24

@THISISDINOSAUR
Copy link
Contributor Author

Looks solid to me! I only ran into a couple tiny issues, neither of which are blockers:

  1. The text size doesn't change in the GPC settings if you're already on that screen when Dynamic Type changes. If you back out of it to the main settings screen and then back in again, it's fine, so this is a bit of an edge case – not a blocker. 🙂
  2. I changed the Dynamic Type setting in an iOS 12 simulator while on the main settings screen and it caused a few labels to vanish. I don't think this is the fault of this PR, and it doesn't happen on later iOS versions. Dismissing the settings screen then showing it again fixes it.

Thanks a lot for the review!

Re #2 we definitely have a lot of issues around dynamic type in general, I think we have an asana task for it somewhere

Re #1 That's a good point, although in general I'm not sure it's worth worrying about changing dynamic type with the app open, at least not for cases like this where we have to do some manual management of text sizes

@THISISDINOSAUR THISISDINOSAUR merged commit db5a1c5 into develop Dec 18, 2020
@THISISDINOSAUR THISISDINOSAUR deleted the feature/elle/settings-header-text-sizes branch December 18, 2020 10:09
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.

2 participants