Skip to content

Conversation

@hotchemi
Copy link

@hotchemi hotchemi commented Mar 27, 2019

Fixes wordpress-mobile/gutenberg-mobile#605

Overview

This PR fixes gutenberg editor screen to honor OS font size when fontScale is changed(The issue is reported at gutenberg-mobile/issues/605.

Since GutenbergContainerFragment is initialized with setRetainInstance(true) I just moved WPAndroidGlueCode#onCreateView in Fragment#onCreateView which can be called even if retainInstance is true.

Links

Screenshots

Before After

To test

  • Turn on Use Block Editor from app setting
  • Open editor screen and type something
  • Go to OS setting(Accessibility→Font Size) and change font size
  • Back to editor screen and confirm content font size has been changed

Update release notes

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@hotchemi
Copy link
Author

@hypest @daniloercoli I've updated the PR as discussed yesterday. Could you take a look when you have time?🙇

@daniloercoli
Copy link
Contributor

daniloercoli commented Mar 28, 2019

Did you miss some commits @hotchemi ?
Testing this brach I see the app crashing due to WPAndroidGlueCode.attachToContainer called before GutenbergContainerFragment#onCreateView and then WPAndroidGlueCode.onCreateView is not called yet.

Tested on Android 7.1.1. and 9.0. Would you mind to take a look?

Note: I also have the suspect that recreating the RN app will end up re-introducing problems fixed by #9030.

  • History lost, and restarting of the editor on rotating the device.

@hotchemi
Copy link
Author

Sorry for the late, I think we need more investigation for the change so let me close the PR for the time being 🙇 Thank you for your review! 🙏

@hypest
Copy link
Contributor

hypest commented Mar 29, 2019

👋 @hotchemi , I know this PR is now closed and we also had reservations about the solution that was lifting the setRetainInstance() state of the container fragment.

I noticed that ReactInstanceManager has this recreateReactContextInBackground() method that I have the feeling that it could be used in our case to trigger a RN "repaint" to use new configuration settings (font size in our case). Can you investigate going down that path?

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.

Ensure that OS-level font-size settings are honored in editor

3 participants