Skip to content

Commit

Permalink
prevent from publishing dimensions change event when app changes state (
Browse files Browse the repository at this point in the history
#34014)

Summary:
This fix solves a problem very well evaluated [here](Expensify/App#2727) as well as this [one](#29290).

The issue is that when the app goes to background, in landscape mode, the RCTDeviceInfo code triggers an orientation change event that did not physically happen. Due to that, we get swapped values returned when going back to the app.

I debugged the react-native code, and to me it seems that react native publishes the orientation change event one extra time when switching the state of the app to 'inactive'. Here is what is happening:

1. iPad is in landscape.
2. We move the app to inactive state.
3. Native Code queues portrait orientation change (no such change happened physically), and immediately after it triggers landscape change (same as we had in point 1).
4. We restore the app to active state.
5. The app receives two queued orientation change events, one after another.
6. The quick transition between portrait and landscape happens even though it never went back to portrait.

Fresh `react-native init` app repro case can be found here: https://github.com/lbaldy/issue-34014-repro

Video presenting the issue (recorded while working on: Expensify/App#2727 ): https://www.youtube.com/watch?v=nFDOml9M8w4

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[iOS] [Fixed] - Fix the way the orientation events are published, to avoid false publish on orientation change when app changes state to inactive

Pull Request resolved: #34014

Test Plan:
### Test Preparation

1. Make sure you have a working version of E/App.
2. Open App/src/components/withWindowDimensions.js and update the constructor by changing this line:

`this.onDimensionChange = _.debounce(this.onDimensionChange.bind(this), 100);`

to

`this.onDimensionChange = this.onDimensionChange.bind(this);`

3. Open the NewExpensify.xcodeproj in xCode.
4. Open the RCTDeviceInfo.mm file and replace it's contents with the file from this PR.
5. Select your device of choice (I suggest starting with iPad mini) and run the app though xCode.
6. From this point you can move to the test scenarios described below.

### iPad Mini tests:

Reproduction + Fix test video (Test 1): https://youtu.be/jyzoNHLYHPo
Reproduction + Fix test video (Test 2): https://youtu.be/CLimE-Fba-g

**Test 1:**
1. Launch app in portrait, open chat - no sidebar visible.
7. Switch to landscape - sidebar shows.
8. Put app to background.
9. Put app back to foreground - make sure the side menu doesn't flicker.

**Test 2:**
1. Launch app in portrait, open chat - no sidebar visible.
2. Switch to landscape - sidebar shows.
3. Put app to background. Switch orientation back to portrait.
4. Put app back to foreground - make sure the side menu hides again as it should be in portrait.

### iPad Pro tests:

Reproduction + Fix test video (Test 3, Test 4): https://youtu.be/EJkUUQCiLRg

iPad mini test 1 applies.

Scenario 2 does not as the screen is too wide in both orientations and iPad pro shows sidebar always.

**Test 3:**

1.  launch the app.
2. Make sure you're in landscape mode.
3. See split screen with some other app. Make sure the side bar is visible.
4. Play with the size of the view, resize it a bit. When the view shrinks it should hide the sidebar, when it grows it should show it.
10. Move the app to background and back to foreground, please observe there are no flickers.

**Test 4:**

1.  Launch the app.
2. Make sure you're in landscape mode.
3. Make the multitasking view and make Expensify app a slide over app.
4. Move back to fullscreen/split screen. Make sure the menu is shown accordingly
5. Move the app to background and back to foreground, please observe there are no flickers.

### iPhone:

Non reg with and without the fix video: https://youtu.be/kuv9in8vtbk

Please perform standard smoke tests on transformation changes.

Reviewed By: cipolleschi

Differential Revision: D37239891

Pulled By: jacdebug

fbshipit-source-id: e6090153820e921dcfb0d823e0377abd25225bdf
  • Loading branch information
lbaldy authored and Titozzz committed Sep 26, 2022
1 parent 38cfe63 commit a9f2429
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 11 deletions.
2 changes: 2 additions & 0 deletions React/Base/RCTUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,4 +186,6 @@ RCT_EXTERN BOOL RCTValidateTypeOfViewCommandArgument(
NSString const *commandName,
NSString const *argPos);

RCT_EXTERN BOOL RCTIsAppActive(void);

NS_ASSUME_NONNULL_END
5 changes: 5 additions & 0 deletions React/Base/RCTUtils.m
Original file line number Diff line number Diff line change
Expand Up @@ -1067,3 +1067,8 @@ RCT_EXTERN BOOL RCTValidateTypeOfViewCommandArgument(

return true;
}

BOOL RCTIsAppActive(void)
{
return [RCTSharedApplication() applicationState] == UIApplicationStateActive;
}
39 changes: 28 additions & 11 deletions React/CoreModules/RCTDeviceInfo.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ @interface RCTDeviceInfo () <NativeDeviceInfoSpec, RCTInitializing>
@implementation RCTDeviceInfo {
UIInterfaceOrientation _currentInterfaceOrientation;
NSDictionary *_currentInterfaceDimensions;
BOOL _isFullscreen;
}

@synthesize bridge = _bridge;
Expand Down Expand Up @@ -60,7 +61,7 @@ - (void)initialize
_currentInterfaceDimensions = RCTExportedDimensions(_moduleRegistry, _bridge);

[[NSNotificationCenter defaultCenter] addObserver:self
selector:@selector(interfaceFrameDidChange)
selector:@selector(interfaceOrientationDidChange)
name:UIApplicationDidBecomeActiveNotification
object:nil];

Expand Down Expand Up @@ -173,22 +174,36 @@ - (void)interfaceOrientationDidChange

- (void)_interfaceOrientationDidChange
{
UIInterfaceOrientation nextOrientation = [RCTSharedApplication() statusBarOrientation];
UIApplication *application = RCTSharedApplication();
UIInterfaceOrientation nextOrientation = [application statusBarOrientation];

BOOL isRunningInFullScreen =
CGRectEqualToRect(application.delegate.window.frame, application.delegate.window.screen.bounds);
// We are catching here two situations for multitasking view:
// a) The app is in Split View and the container gets resized -> !isRunningInFullScreen
// b) The app changes to/from fullscreen example: App runs in slide over mode and goes into fullscreen->
// isRunningInFullScreen != _isFullscreen The above two cases a || b can be shortened to !isRunningInFullScreen ||
// !_isFullscreen;
BOOL isResizingOrChangingToFullscreen = !isRunningInFullScreen || !_isFullscreen;
BOOL isOrientationChanging = (UIInterfaceOrientationIsPortrait(_currentInterfaceOrientation) &&
!UIInterfaceOrientationIsPortrait(nextOrientation)) ||
(UIInterfaceOrientationIsLandscape(_currentInterfaceOrientation) &&
!UIInterfaceOrientationIsLandscape(nextOrientation));

// Update when we go from portrait to landscape, or landscape to portrait
if ((UIInterfaceOrientationIsPortrait(_currentInterfaceOrientation) &&
!UIInterfaceOrientationIsPortrait(nextOrientation)) ||
(UIInterfaceOrientationIsLandscape(_currentInterfaceOrientation) &&
!UIInterfaceOrientationIsLandscape(nextOrientation))) {
// Also update when the fullscreen state changes (multitasking) and only when the app is in active state.
if ((isOrientationChanging || isResizingOrChangingToFullscreen) && RCTIsAppActive()) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
[[_moduleRegistry moduleForName:"EventDispatcher"]
sendDeviceEventWithName:@"didUpdateDimensions"
body:RCTExportedDimensions(_moduleRegistry, _bridge)];
// We only want to track the current _currentInterfaceOrientation and _isFullscreen only
// when it happens and only when it is published.
_currentInterfaceOrientation = nextOrientation;
_isFullscreen = isRunningInFullScreen;
#pragma clang diagnostic pop
}

_currentInterfaceOrientation = nextOrientation;
}

- (void)interfaceFrameDidChange
Expand All @@ -203,15 +218,17 @@ - (void)_interfaceFrameDidChange
{
NSDictionary *nextInterfaceDimensions = RCTExportedDimensions(_moduleRegistry, _bridge);

if (!([nextInterfaceDimensions isEqual:_currentInterfaceDimensions])) {
// update and publish the even only when the app is in active state
if (!([nextInterfaceDimensions isEqual:_currentInterfaceDimensions]) && RCTIsAppActive()) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
[[_moduleRegistry moduleForName:"EventDispatcher"] sendDeviceEventWithName:@"didUpdateDimensions"
body:nextInterfaceDimensions];
// We only want to track the current _currentInterfaceOrientation only
// when it happens and only when it is published.
_currentInterfaceDimensions = nextInterfaceDimensions;
#pragma clang diagnostic pop
}

_currentInterfaceDimensions = nextInterfaceDimensions;
}

- (std::shared_ptr<TurboModule>)getTurboModule:(const ObjCTurboModule::InitParams &)params
Expand Down

0 comments on commit a9f2429

Please sign in to comment.