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

Mobile - RCTAztecView - Fixes font customization not getting updated #53391

Merged
merged 6 commits into from
Aug 16, 2023

Conversation

geriux
Copy link
Member

@geriux geriux commented Aug 7, 2023

Related PRs:

What?

This PR fixes an issue that started to happen after the recent React Native upgrade where the post title and some other functionality related to custom font size and font styles like bold formatting wouldn't work as expected.

Why?

It appears after setting the props fontSize, fontWeight wouldn't reflect the changes when rendering or typing new content. This is an iOS-only issue and it was affecting the setup we currently have with Aztec.

How?

By forcing to update the current font, defaultFont, and the placeholder's font as well as the typingAttributes to keep them updated after the font props are set.

I couldn't find another way to address this issue than by doing this proposal. We can explore other options if needed.

Testing Instructions

Scenario 1 - Post title placeholder shows the correct font styling

  • Open the app using the build linked in this PR
  • Start a new post
  • Expect to see the post title's placeholder with the bold font styling and expected font family
  • Type a title
  • Expect the title to have the bold font styling and the expected font family

Scenario 2 - Test the text formatting options

  • Open the app using the build linked in this PR
  • Start a new post
  • Write some text content
  • Test using the bold, italic, link, and text color formatting options
  • Expect these options to work as expected
  • Save the post
  • Open it again and expect to see the formats previously set

Scenario 3 - Font size and line height

  • Open the app using the build linked in this PR
  • Start a new post
  • Write some text content
  • Change the font size and line height for the Paragraph blocks (a block-based theme is needed)
  • Expect to see the selected values
  • Save the post
  • Open the post and expect to see the previously selected values

Testing Instructions for Keyboard

N/A

Screenshots or screencast

Expected behaviour
FontStylingBugiOS.MP4

@geriux geriux added [Type] Bug An existing feature does not function as intended Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) Mobile App - Automation Label used to initiate Mobile App PR Automation labels Aug 7, 2023
@geriux geriux removed the Mobile App - Automation Label used to initiate Mobile App PR Automation label Aug 8, 2023
@@ -689,7 +689,13 @@ class RCTAztecView: Aztec.TextView {
///
private func refreshFont() {
let newFont = applyFontConstraints(to: defaultFont)
font = newFont
Copy link
Contributor

Choose a reason for hiding this comment

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

It's really interesting that this didn't take place before. I also see remnants of a past history where refreshFont probably called refreshTypingAttributesAndPlaceholderFont, due to the comment on line 702. 🤷

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.

Thanks for the fix @geriux!

In addition to the tests you provided I also tested VoiceOver and didn't spot any regressions. 🚀

# Conflicts:
#	packages/react-native-editor/CHANGELOG.md
@geriux geriux merged commit e1f8a9e into trunk Aug 16, 2023
50 checks passed
@geriux geriux deleted the rnmobile/fix/ios-font-customization branch August 16, 2023 13:08
@github-actions github-actions bot added this to the Gutenberg 16.5 milestone Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

2 participants