Skip to content

Commit 7d42106

Browse files
lbaldyfacebook-github-bot
authored andcommitted
prevent from publishing dimensions change event when app changes state (#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
1 parent afa5df1 commit 7d42106

File tree

3 files changed

+35
-11
lines changed

3 files changed

+35
-11
lines changed

React/Base/RCTUtils.h

+2
Original file line numberDiff line numberDiff line change
@@ -186,4 +186,6 @@ RCT_EXTERN BOOL RCTValidateTypeOfViewCommandArgument(
186186
NSString const *commandName,
187187
NSString const *argPos);
188188

189+
RCT_EXTERN BOOL RCTIsAppActive(void);
190+
189191
NS_ASSUME_NONNULL_END

React/Base/RCTUtils.m

+5
Original file line numberDiff line numberDiff line change
@@ -1067,3 +1067,8 @@ RCT_EXTERN BOOL RCTValidateTypeOfViewCommandArgument(
10671067

10681068
return true;
10691069
}
1070+
1071+
BOOL RCTIsAppActive(void)
1072+
{
1073+
return [RCTSharedApplication() applicationState] == UIApplicationStateActive;
1074+
}

React/CoreModules/RCTDeviceInfo.mm

+28-11
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ @interface RCTDeviceInfo () <NativeDeviceInfoSpec, RCTInitializing>
2626
@implementation RCTDeviceInfo {
2727
UIInterfaceOrientation _currentInterfaceOrientation;
2828
NSDictionary *_currentInterfaceDimensions;
29+
BOOL _isFullscreen;
2930
}
3031

3132
@synthesize bridge = _bridge;
@@ -60,7 +61,7 @@ - (void)initialize
6061
_currentInterfaceDimensions = RCTExportedDimensions(_moduleRegistry, _bridge);
6162

6263
[[NSNotificationCenter defaultCenter] addObserver:self
63-
selector:@selector(interfaceFrameDidChange)
64+
selector:@selector(interfaceOrientationDidChange)
6465
name:UIApplicationDidBecomeActiveNotification
6566
object:nil];
6667

@@ -173,22 +174,36 @@ - (void)interfaceOrientationDidChange
173174

174175
- (void)_interfaceOrientationDidChange
175176
{
176-
UIInterfaceOrientation nextOrientation = [RCTSharedApplication() statusBarOrientation];
177+
UIApplication *application = RCTSharedApplication();
178+
UIInterfaceOrientation nextOrientation = [application statusBarOrientation];
179+
180+
BOOL isRunningInFullScreen =
181+
CGRectEqualToRect(application.delegate.window.frame, application.delegate.window.screen.bounds);
182+
// We are catching here two situations for multitasking view:
183+
// a) The app is in Split View and the container gets resized -> !isRunningInFullScreen
184+
// b) The app changes to/from fullscreen example: App runs in slide over mode and goes into fullscreen->
185+
// isRunningInFullScreen != _isFullscreen The above two cases a || b can be shortened to !isRunningInFullScreen ||
186+
// !_isFullscreen;
187+
BOOL isResizingOrChangingToFullscreen = !isRunningInFullScreen || !_isFullscreen;
188+
BOOL isOrientationChanging = (UIInterfaceOrientationIsPortrait(_currentInterfaceOrientation) &&
189+
!UIInterfaceOrientationIsPortrait(nextOrientation)) ||
190+
(UIInterfaceOrientationIsLandscape(_currentInterfaceOrientation) &&
191+
!UIInterfaceOrientationIsLandscape(nextOrientation));
177192

178193
// Update when we go from portrait to landscape, or landscape to portrait
179-
if ((UIInterfaceOrientationIsPortrait(_currentInterfaceOrientation) &&
180-
!UIInterfaceOrientationIsPortrait(nextOrientation)) ||
181-
(UIInterfaceOrientationIsLandscape(_currentInterfaceOrientation) &&
182-
!UIInterfaceOrientationIsLandscape(nextOrientation))) {
194+
// Also update when the fullscreen state changes (multitasking) and only when the app is in active state.
195+
if ((isOrientationChanging || isResizingOrChangingToFullscreen) && RCTIsAppActive()) {
183196
#pragma clang diagnostic push
184197
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
185198
[[_moduleRegistry moduleForName:"EventDispatcher"]
186199
sendDeviceEventWithName:@"didUpdateDimensions"
187200
body:RCTExportedDimensions(_moduleRegistry, _bridge)];
201+
// We only want to track the current _currentInterfaceOrientation and _isFullscreen only
202+
// when it happens and only when it is published.
203+
_currentInterfaceOrientation = nextOrientation;
204+
_isFullscreen = isRunningInFullScreen;
188205
#pragma clang diagnostic pop
189206
}
190-
191-
_currentInterfaceOrientation = nextOrientation;
192207
}
193208

194209
- (void)interfaceFrameDidChange
@@ -203,15 +218,17 @@ - (void)_interfaceFrameDidChange
203218
{
204219
NSDictionary *nextInterfaceDimensions = RCTExportedDimensions(_moduleRegistry, _bridge);
205220

206-
if (!([nextInterfaceDimensions isEqual:_currentInterfaceDimensions])) {
221+
// update and publish the even only when the app is in active state
222+
if (!([nextInterfaceDimensions isEqual:_currentInterfaceDimensions]) && RCTIsAppActive()) {
207223
#pragma clang diagnostic push
208224
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
209225
[[_moduleRegistry moduleForName:"EventDispatcher"] sendDeviceEventWithName:@"didUpdateDimensions"
210226
body:nextInterfaceDimensions];
227+
// We only want to track the current _currentInterfaceOrientation only
228+
// when it happens and only when it is published.
229+
_currentInterfaceDimensions = nextInterfaceDimensions;
211230
#pragma clang diagnostic pop
212231
}
213-
214-
_currentInterfaceDimensions = nextInterfaceDimensions;
215232
}
216233

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

0 commit comments

Comments
 (0)