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

[RNMobile] Ensure font is scaled on iOS when accessibility settings are changed #57339

Merged
merged 5 commits into from
Dec 22, 2023

Conversation

SiobhyB
Copy link
Contributor

@SiobhyB SiobhyB commented Dec 22, 2023

Note There are times where the font appears overly large when used with custom fonts and block-based themes, this will be addressed in a separate Gutenberg PR for both platforms. The scope of this specific PR is to enable support for Dynamic Type on iOS.

Related PRs

What?

With this PR, we ensure that font in the editor is changed when users change their accessibility settings on iOS.

Why?

At the moment, Dynamic Type is not supported in the editor's RichText component for iOS. This is because the refreshFont() function here is 'undoing' the scaling that takes place elsewhere in Aztec's codebase.

How?

We ensure scaling is applied to the new font when it's set here within the applyFontConstraints() function.

Testing Instructions

An installable build to test these changes can be found here.

  • Navigate to the Accessibility section in the Settings app.
  • Select Display & Text Size then Larger Accessibility Sizes to on.
  • Select a larger accessibility size.
  • Next, open the WordPress app and select a site with a block-based theme.
  • Navigate to the editor in the app.
  • Either add a heading block or begin typing into a paragraph block.
  • Select the block's cog/gear icon to open its settings and select a larger font size.
  • Experiment with typing more text and adding more heading or paragraph blocks.
  • Verify that the font has been scaled in accordance with your device and block settings.

Screenshots or screencast

Before and after ⤵️
Before After
Android comparison ⤵️
Android iOS

@SiobhyB SiobhyB added Mobile App - Automation Label used to initiate Mobile App PR Automation Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Bug An existing feature does not function as intended labels Dec 22, 2023
@SiobhyB SiobhyB changed the title [RNMobile] Ensure font is scaled as expected [RNMobile] Ensure font is scaled on iOS when accessibility settings are changed Dec 22, 2023
Copy link

github-actions bot commented Dec 22, 2023

Flaky tests detected in d98f0b7.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7303326923
📝 Reported issues:

@SiobhyB SiobhyB requested review from geriux and twstokes December 22, 2023 16:02
@SiobhyB
Copy link
Contributor Author

SiobhyB commented Dec 22, 2023

@geriux, @twstokes, I know you have some upcoming AFK, and I do too, so don't expect that this will be prioritised. Just wanted to get it on your radar for when you're back!

@SiobhyB SiobhyB marked this pull request as ready for review December 22, 2023 16:03
Copy link
Contributor

@twstokes twstokes left a comment

Choose a reason for hiding this comment

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

Worked great for me @SiobhyB! 🚀

As a future enhancement it would be nice if the editor responded to Dynamic Type changes without needing to be closed and reopened. That's not nearly as important, though, as supporting it in the first place.

@SiobhyB
Copy link
Contributor Author

SiobhyB commented Dec 22, 2023

@twstokes, thank you so much for the review! 🙌

As a future enhancement it would be nice if the editor responded to Dynamic Type changes without needing to be closed and reopened. That's not nearly as important, though, as supporting it in the first place.

This makes sense, I've created a new issue at #57354 listing some of the remaining tasks left to make Dynamic Type support better overall. I've made a note to shape a pitch around accessibility fixes in the New Year which would include this. 🙇‍♀️

@SiobhyB SiobhyB enabled auto-merge (squash) December 22, 2023 20:22
@SiobhyB SiobhyB merged commit f572440 into trunk Dec 22, 2023
55 checks passed
@SiobhyB SiobhyB deleted the rnmobile/enable-dynamic-type-on-ios branch December 22, 2023 20:35
@github-actions github-actions bot added this to the Gutenberg 17.4 milestone Dec 22, 2023
} else {
newFontSize = baseFont.pointSize
}
let newFontSize = fontMetrics.scaledValue(for: fontSize ?? baseFont.pointSize)
Copy link
Member

Choose a reason for hiding this comment

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

TIL about scaledValue 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - Automation Label used to initiate Mobile App PR Automation Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants