[ANDROID] Fix DeviceInfoModule LifecycleEventListener #17227
Conversation
facebook-github-bot
added
the
CLA Signed
|
Also fwiw I have no idea why circleci is failing, especially on the js and objc tests, since this PR makes effectively a 1-line change to Java code used only on Android... |
|
@hramos Any chance you could have a look at this? Alternatively, if you're not the right person to tag, do you know who would be? |
|
@joshyhargreaves Any chance you could have a look at this? |
|
@janicduplessis any chance you can review this PR? |
| @@ -9,11 +9,6 @@ | |||
|
|
|||
| package com.facebook.react.modules.deviceinfo; | |||
|
|
|||
| import javax.annotation.Nullable; | |||
There was a problem hiding this comment.
Unless there's any styling rule requirement, moving of the imports is unnecessary.
There was a problem hiding this comment.
Sure, that was Android Studio moving them around automatically but i can set them back.
steveccable
force-pushed
the
android-fontscale-eventlistener
branch
from
fa5766c to
733a313
Compare
|
@joshyhargreaves Cleaned up the imports, now it should be just the one line of actual fix. |
|
lgtm |
|
@facebook-github-bot shipit |
facebook-github-bot
added
the
Import Started
There was a problem hiding this comment.
@grabbou is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Fixes one of the two parts of [issue posted here](facebook#17209). Motivation is that we wanted our app to be able to handle user changing font size while the app is running. Yes, we know that changing font size doesn't inherently trigger a re-render, but at the minimum it should cause `PixelRatio.getFontScale()` to return an updated value. 1. Render an interface and invoke `PixelRatio.getFontScale()` 2. Change the font scale in Settings > Accessibility > Font Size 3. Return to the React Native app (without reloading Javascript) and invoke `PixelRatio.getFontScale()` again. 4. Verify that the result of `PixelRatio.getFontScale()` has changed to reflect the new font size. For a video of the problem, see the linked issue under the Motivation heading. In this fixed version, the number actually does update as expected. None. This should require no documentation change, as it is behavior that the docs seem to indicate should be happening already. In fact, [the documentation at the bottom of this page](https://facebook.github.io/react-native/docs/native-modules-android.html) appears to already indicate that this should be done for anything implementing LifecycleEventListener. [ANDROID] [BUGFIX] [DeviceInfo] - Fix the DeviceInfoModule to properly respond to LifecycleEvents Closes facebook#17227 Differential Revision: D6692358 Pulled By: hramos fbshipit-source-id: 3db212fe8103c7aa29a29ead6c772abb7ae4cd85
Motivation
Fixes one of the two parts of issue posted here. Motivation is that we wanted our app to be able to handle user changing font size while the app is running. Yes, we know that changing font size doesn't inherently trigger a re-render, but at the minimum it should cause
PixelRatio.getFontScale()to return an updated value.Test Plan
PixelRatio.getFontScale()PixelRatio.getFontScale()again.PixelRatio.getFontScale()has changed to reflect the new font size.For a video of the problem, see the linked issue under the Motivation heading. In this fixed version, the number actually does update as expected.
Related PRs
None. This should require no documentation change, as it is behavior that the docs seem to indicate should be happening already.
In fact, the documentation at the bottom of this page appears to already indicate that this should be done for anything implementing LifecycleEventListener.
Release Notes
[ANDROID] [BUGFIX] [DeviceInfo] - Fix the DeviceInfoModule to properly respond to LifecycleEvents