-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
prevent from publishing dimensions change event when app changes state #34014
Closed
Closed
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
8653851
fix useWindowDimensions, prevent from publishing dimensions change ev…
lbaldy b0c3bc7
update code structure and add comments
lbaldy e21080d
improve comments
lbaldy b4b38d2
simplify comments
lbaldy 818a073
update comments
lbaldy 27e65a5
shorten the condition, rename the variable to give it more meaningful…
lbaldy f690dc1
Merge pull request #11 from lbaldy/bugfix/issue-2727
iwiznia 0bee080
Merge branch 'facebook:main' into master
roryabraham e5856b0
improve code style
lbaldy 320cb11
improve code style
lbaldy 9d2be30
refactor based on PR review comments https://github.com/facebook/reac…
lbaldy cabdc03
code style fixes
lbaldy 394fd10
add new line at the end of file for the linter
lbaldy 23f6564
formatting
lbaldy ff62965
formatting
lbaldy 575f054
remove ununsed variable
lbaldy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ @interface RCTDeviceInfo () <NativeDeviceInfoSpec, RCTInitializing> | |
@implementation RCTDeviceInfo { | ||
UIInterfaceOrientation _currentInterfaceOrientation; | ||
NSDictionary *_currentInterfaceDimensions; | ||
BOOL _isFullscreen; | ||
} | ||
|
||
@synthesize bridge = _bridge; | ||
|
@@ -60,7 +61,7 @@ - (void)initialize | |
_currentInterfaceDimensions = RCTExportedDimensions(_moduleRegistry, _bridge); | ||
|
||
[[NSNotificationCenter defaultCenter] addObserver:self | ||
selector:@selector(interfaceFrameDidChange) | ||
selector:@selector(interfaceOrientationDidChange) | ||
name:UIApplicationDidBecomeActiveNotification | ||
object:nil]; | ||
|
||
|
@@ -173,22 +174,34 @@ - (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); | ||
lbaldy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// 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)]; | ||
[[_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 | ||
|
@@ -201,17 +214,20 @@ - (void)interfaceFrameDidChange | |
|
||
- (void)_interfaceFrameDidChange | ||
{ | ||
UIApplication *application = RCTSharedApplication(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you also remove this variable that is unused? It's make our sanity checks exploding! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
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]; | ||
[[_moduleRegistry moduleForName:"EventDispatcher"] sendDeviceEventWithName:@"didUpdateDimensions" | ||
body:nextInterfaceDimensions]; | ||
lbaldy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// 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 | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to bother, but our linter is asking a new line at the end of the file. Could you please add it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, and pushed