Skip to content

Onboarding Improvements: Fix font scaling for the Quick Start prompt - #17662

Merged
momo-ozawa merged 2 commits into
developfrom
fix/qs-prompt-font-scaling
Dec 13, 2021
Merged

Onboarding Improvements: Fix font scaling for the Quick Start prompt#17662
momo-ozawa merged 2 commits into
developfrom
fix/qs-prompt-font-scaling

Conversation

@momo-ozawa

@momo-ozawa momo-ozawa commented Dec 10, 2021

Copy link
Copy Markdown
Contributor

Fixes #17652 (comment)

Description

This PR fixes

Thanks @dvdchr for pointing out these issues.

Notes

⚠️ This PR should be merged into develop. It is temporarily pointing to merge/18.8-beta-4-into-develop.

While this PR is ready to review, I'll wait until merge/18.8-beta-4-into-develop gets merged to open the PR, so it doesn't accidentally get merged in to merge/18.8-beta-4-into-develop.

  • My fixes are dependent on some stuff in merge/18.8-beta-4-into-develop, which is why this PR is based on that branch
  • Since these changes aren't needed in 18.8, merge/18.8-beta-4-into-develop can be merged in to develop, without this PR being merged into merge/18.8-beta-4-into-develop (Slack ref: p1639144141039900-slack-C027K4MNPGQ)
  • Once merge/18.8-beta-4-into-develop gets merged in to develop, the base branch for this PR should change to develop automatically, and my fixes can be part of 18.9

How to test

  1. Do a fresh install of wpios
  2. Login
  3. On the Login Epilogue, choose a site
  4. Change the system font size to the largest size
  5. ✅ The Quick Start prompt should be showing and the content should be scaled according to the selected font size
  6. ✅ You should be able to scroll down to see any text that's larger than the frame of the scrollview
iPhone iPad

Regression Notes

  1. Potential unintended areas of impact
    Quick Start iPad readable content width

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Check manually, see screenshot above

  3. What automated tests I added (or what prevented me from doing so)
    n/a

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Momo Ozawa added 2 commits December 10, 2021 11:09
This fixes an issue where the serif text wasn't scaling properly with the system font size
This fixes an issue where the text was being obscured by the buttons when the system font size was set to a large size
@momo-ozawa momo-ozawa added this to the 18.9 milestone Dec 10, 2021
@momo-ozawa momo-ozawa self-assigned this Dec 10, 2021
@peril-wordpress-mobile

Copy link
Copy Markdown

You can trigger an installable build for these changes by visiting CircleCI here.

@peril-wordpress-mobile

Copy link
Copy Markdown

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@mokagio
mokagio changed the base branch from merge/18.8-beta-4-into-develop to develop December 10, 2021 13:30
@mokagio
mokagio changed the base branch from develop to merge/18.8-beta-4-into-develop December 10, 2021 13:30
@mokagio

mokagio commented Dec 10, 2021

Copy link
Copy Markdown
Contributor

Hey @momo-ozawa 👋 Thank you for looking into this so quickly.

I noticed the PR base branch is merge/18.8-beta-4-into-develop. I was hoping to merge that branch into develop and move on with the release process.

I tried to change the base for this one to develop, but the diff explodes in side.

Do you think you could open a new PR based on develop for it?

I'm saying all this under the assumption it's fine to ship 18.8 with the issue. I think it is because it's an edge case (users with lager than usual font size) on a one-off kind of screen (onboarding).

On the other hand, onboarding is a very important part of the user acquisition funnel, so I'd understand if someone were to argue we should hold the release for this. 🤔

If we decide to add this fix to 18.8, we should reopen the PR against release/18.8.

cc @startuptester

@momo-ozawa

Copy link
Copy Markdown
Contributor Author

Hi @mokagio 👋

Do you think you could open a new PR based on develop for it?

I was thinking that once merge/18.8-beta-4-into-develop gets merged into develop, the base branch for this PR would automatically change to develop.

I'm saying all this under the assumption it's fine to ship 18.8 with the issue. I think it is because it's an edge case (users with lager than usual font size) on a one-off kind of screen (onboarding).

I checked w @leandroalonso and we think it's ok to target 18.9 for these fixes instead of 18.8.

(Slack ref: p1639144141039900-slack-C027K4MNPGQ)

@mokagio

mokagio commented Dec 10, 2021

Copy link
Copy Markdown
Contributor

@momo-ozawa

I was thinking that once merge/18.8-beta-4-into-develop gets merged into develop, the base branch for this PR would automatically change to develop.

That would be correct, but I'm being a bit self-serving here, though, because I'd like to merge that PR ASAP and move on with the release cycle, finalizing the 18.8 build for App Store submission, which is something scheduled for today. 😅

Scratch that. Yeah. I think it should work. Let's see how it goes, hey 🤷‍♂️

Apologies for all the confusion. I should have slowed down and think things through more carefully

@momo-ozawa

Copy link
Copy Markdown
Contributor Author

@mokagio No worries! I should have been more clear about my intentions for this PR so sorry about that!

  • My fixes are dependent on some stuff in merge/18.8-beta-4-into-develop, which is why this PR is based on that branch
  • Since these changes aren't needed in 18.8, I figured merge/18.8-beta-4-into-develop could be merged in to develop, without this PR being merged into merge/18.8-beta-4-into-develop
  • Once merge/18.8-beta-4-into-develop gets merged in to develop, the base branch for this PR should change to develop automatically, and my fixes can be part of 18.9

Base automatically changed from merge/18.8-beta-4-into-develop to develop December 10, 2021 17:15
@momo-ozawa
momo-ozawa marked this pull request as ready for review December 10, 2021 17:36
@momo-ozawa
momo-ozawa merged commit 7028b77 into develop Dec 13, 2021
@momo-ozawa
momo-ozawa deleted the fix/qs-prompt-font-scaling branch December 13, 2021 11:36
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.

3 participants