-
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
Prevented error when paused Activity isn’t the current Activity #26144
Conversation
Hm, looking at the Activity lifecycle diagram from this article (Back Stack — Scenario 2: Activities in the back stack with configuration changes), it shouldn't be the case that onPause of the Main activity is called after we rotate the screen and press back on the Details activity: I created a sample project and these are the logs I get when I start the Details activity, rotate the screen and press the back button:
As you can see, onPause is not called on the MainActivity, which confirms what the above mentioned diagram shows. Are you sure you have different behaviour? |
I'm sure. It only happens if there’s a shared element transition running between the two Activities. Please try it out in Android’s shared element sample application. Start in portrait, click an image, rotate to landscape and click the Android back button. Then the Detail Activity will pause while the Main Activity is current. |
I've changed my Navigation router to use multiple Fragments instead of multiple Activities. So, although this is still a React Native bug, it means I don't face the problem anymore |
Summary: This sync includes the following changes: - **[86c8c8db7](facebook/react@86c8c8db7 )**: test: Don't retry flushActWork if flushUntilNextPaint threw ([#26121](facebook/react#26121)) //<Sebastian Silbermann>// - **[64acd3918](facebook/react@64acd3918 )**: remove unguarded getRootNode call ([#26152](facebook/react#26152)) //<Josh Story>// - **[71cace4d3](facebook/react@71cace4d3 )**: Migrate testRunner from jasmine2 to jest-circus ([#26144](facebook/react#26144)) //<Ming Ye>// - **[c8510227c](facebook/react@c8510227c )**: Treat displayName as undefined ([#26148](facebook/react#26148)) //<Sebastian Markbåge>// - **[55542bc73](facebook/react@55542bc73 )**: Update jest printBasicPrototype config ([#26142](facebook/react#26142)) //<Ming Ye>// - **[6396b6641](facebook/react@6396b6641 )**: Model Float on Hoistables semantics ([#26106](facebook/react#26106)) //<Josh Story>// - **[ef9f6e77b](facebook/react@ef9f6e77b )**: Enable passing Server References from Server to Client ([#26124](facebook/react#26124)) //<Sebastian Markbåge>// - **[35698311d](facebook/react@35698311d )**: Update jest escapeString config ([#26140](facebook/react#26140)) //<Ming Ye>// - **[6ddcbd4f9](facebook/react@6ddcbd4f9 )**: [flow] enable LTI inference mode ([#26104](facebook/react#26104)) //<Jan Kassens>// - **[53b1f69ba](facebook/react@53b1f69ba )**: Implement unstable_getBoundingClientRect in RN Fabric refs ([#26137](facebook/react#26137)) //<Rubén Norte>// - **[594093496](facebook/react@594093496 )**: Update to Jest 29 ([#26088](facebook/react#26088)) //<Ming Ye>// - **[28fcae062](facebook/react@28fcae062 )**: Add support for SVG `transformOrigin` prop ([#26130](facebook/react#26130)) //<Aravind D>// - **[3ff1540e9](facebook/react@3ff1540e9 )**: Prefer JSX in ReactNoop assertions (to combat out-of-memory test runs) ([#26127](facebook/react#26127)) //<Sebastian Silbermann>// - **[01a0c4e12](facebook/react@01a0c4e12 )**: Add Edge Server Builds for workerd / edge-light ([#26116](facebook/react#26116)) //<Sebastian Markbåge>// - **[f0cf832e1](facebook/react@f0cf832e1 )**: Update Flight Fixture to "use client" instead of .client.js ([#26118](facebook/react#26118)) //<Sebastian Markbåge>// - **[03a216070](facebook/react@03a216070 )**: Rename "dom" fork to "dom-node" and "bun" fork to "dom-bun" ([#26117](facebook/react#26117)) //<Sebastian Markbåge>// - **[4bf2113a1](facebook/react@4bf2113a1 )**: Revert "Move the Webpack manifest config to one level deeper ([#26083](facebook/react#26083))" ([#26111](facebook/react#26111)) //<Sebastian Markbåge>// - **[2ef24145e](facebook/react@2ef24145e )**: [flow] upgrade to 0.199.0 ([#26096](facebook/react#26096)) //<Jan Kassens>// - **[922dd7ba5](facebook/react@922dd7ba5 )**: Revert the outer module object to an object ([#26093](facebook/react#26093)) //<Sebastian Markbåge>// - **[9d111ffdf](facebook/react@9d111ffdf )**: Serialize Promises through Flight ([#26086](facebook/react#26086)) //<Sebastian Markbåge>// - **[0ba4698c7](facebook/react@0ba4698c7 )**: Fix async test in React reconciler ([#26087](facebook/react#26087)) //<Ming Ye>// - **[8c234c0de](facebook/react@8c234c0de )**: Move the Webpack manifest config to one level deeper ([#26083](facebook/react#26083)) //<Sebastian Markbåge>// - **[977bccd24](facebook/react@977bccd24 )**: Refactor Flight Encoding ([#26082](facebook/react#26082)) //<Sebastian Markbåge>// - **[d7bb524ad](facebook/react@d7bb524ad )**: [cleanup] Remove unused package jest-mock-scheduler ([#26084](facebook/react#26084)) //<Ming Ye>// - **[6b3083266](facebook/react@6b3083266 )**: Upgrade prettier ([#26081](facebook/react#26081)) //<Jan Kassens>// - **[1f5ce59dd](facebook/react@1f5ce59dd )**: [cleanup] fully roll out warnAboutSpreadingKeyToJSX ([#26080](facebook/react#26080)) //<Jan Kassens>// Changelog: [General][Changed] - React Native sync for revisions 48b687f...fccf3a9 jest_e2e[run_all_tests] Reviewed By: rubennorte Differential Revision: D43305607 fbshipit-source-id: 8da7567ca2a182f4be27788935c2da30a731f83b
Summary: This sync includes the following changes: - **[86c8c8db7](facebook/react@86c8c8db7 )**: test: Don't retry flushActWork if flushUntilNextPaint threw ([facebook#26121](facebook/react#26121)) //<Sebastian Silbermann>// - **[64acd3918](facebook/react@64acd3918 )**: remove unguarded getRootNode call ([facebook#26152](facebook/react#26152)) //<Josh Story>// - **[71cace4d3](facebook/react@71cace4d3 )**: Migrate testRunner from jasmine2 to jest-circus ([facebook#26144](facebook/react#26144)) //<Ming Ye>// - **[c8510227c](facebook/react@c8510227c )**: Treat displayName as undefined ([facebook#26148](facebook/react#26148)) //<Sebastian Markbåge>// - **[55542bc73](facebook/react@55542bc73 )**: Update jest printBasicPrototype config ([facebook#26142](facebook/react#26142)) //<Ming Ye>// - **[6396b6641](facebook/react@6396b6641 )**: Model Float on Hoistables semantics ([facebook#26106](facebook/react#26106)) //<Josh Story>// - **[ef9f6e77b](facebook/react@ef9f6e77b )**: Enable passing Server References from Server to Client ([facebook#26124](facebook/react#26124)) //<Sebastian Markbåge>// - **[35698311d](facebook/react@35698311d )**: Update jest escapeString config ([facebook#26140](facebook/react#26140)) //<Ming Ye>// - **[6ddcbd4f9](facebook/react@6ddcbd4f9 )**: [flow] enable LTI inference mode ([facebook#26104](facebook/react#26104)) //<Jan Kassens>// - **[53b1f69ba](facebook/react@53b1f69ba )**: Implement unstable_getBoundingClientRect in RN Fabric refs ([facebook#26137](facebook/react#26137)) //<Rubén Norte>// - **[594093496](facebook/react@594093496 )**: Update to Jest 29 ([facebook#26088](facebook/react#26088)) //<Ming Ye>// - **[28fcae062](facebook/react@28fcae062 )**: Add support for SVG `transformOrigin` prop ([facebook#26130](facebook/react#26130)) //<Aravind D>// - **[3ff1540e9](facebook/react@3ff1540e9 )**: Prefer JSX in ReactNoop assertions (to combat out-of-memory test runs) ([facebook#26127](facebook/react#26127)) //<Sebastian Silbermann>// - **[01a0c4e12](facebook/react@01a0c4e12 )**: Add Edge Server Builds for workerd / edge-light ([facebook#26116](facebook/react#26116)) //<Sebastian Markbåge>// - **[f0cf832e1](facebook/react@f0cf832e1 )**: Update Flight Fixture to "use client" instead of .client.js ([facebook#26118](facebook/react#26118)) //<Sebastian Markbåge>// - **[03a216070](facebook/react@03a216070 )**: Rename "dom" fork to "dom-node" and "bun" fork to "dom-bun" ([facebook#26117](facebook/react#26117)) //<Sebastian Markbåge>// - **[4bf2113a1](facebook/react@4bf2113a1 )**: Revert "Move the Webpack manifest config to one level deeper ([facebook#26083](facebook/react#26083))" ([facebook#26111](facebook/react#26111)) //<Sebastian Markbåge>// - **[2ef24145e](facebook/react@2ef24145e )**: [flow] upgrade to 0.199.0 ([facebook#26096](facebook/react#26096)) //<Jan Kassens>// - **[922dd7ba5](facebook/react@922dd7ba5 )**: Revert the outer module object to an object ([facebook#26093](facebook/react#26093)) //<Sebastian Markbåge>// - **[9d111ffdf](facebook/react@9d111ffdf )**: Serialize Promises through Flight ([facebook#26086](facebook/react#26086)) //<Sebastian Markbåge>// - **[0ba4698c7](facebook/react@0ba4698c7 )**: Fix async test in React reconciler ([facebook#26087](facebook/react#26087)) //<Ming Ye>// - **[8c234c0de](facebook/react@8c234c0de )**: Move the Webpack manifest config to one level deeper ([facebook#26083](facebook/react#26083)) //<Sebastian Markbåge>// - **[977bccd24](facebook/react@977bccd24 )**: Refactor Flight Encoding ([facebook#26082](facebook/react#26082)) //<Sebastian Markbåge>// - **[d7bb524ad](facebook/react@d7bb524ad )**: [cleanup] Remove unused package jest-mock-scheduler ([facebook#26084](facebook/react#26084)) //<Ming Ye>// - **[6b3083266](facebook/react@6b3083266 )**: Upgrade prettier ([facebook#26081](facebook/react#26081)) //<Jan Kassens>// - **[1f5ce59dd](facebook/react@1f5ce59dd )**: [cleanup] fully roll out warnAboutSpreadingKeyToJSX ([facebook#26080](facebook/react#26080)) //<Jan Kassens>// Changelog: [General][Changed] - React Native sync for revisions 48b687f...fccf3a9 jest_e2e[run_all_tests] Reviewed By: rubennorte Differential Revision: D43305607 fbshipit-source-id: 8da7567ca2a182f4be27788935c2da30a731f83b
Summary
I’m the author of the Navigation router. It’s a navigation library that is 100% native on Android and iOS. On Android it creates an
Activity
per screen. It uses the nativemakeSceneTransitionAnimation
API to bring shared element animation to Android when navigating between screens/Activities.The Problem
Create two screens A and B. Add a shared element to each screen that animates when navigating from A to B. When the screen orientation isn’t changed then everything works fine. The problem happens when screen B has a different orientation to A. Start the app in portrait and navigate to B. Change the app to landscape and navigate back to A. React Native throws an error saying “Pausing an activity that is not the current activity, this is incorrect!”.
The Reason
React Native wrongly assumes that only current Activity can be paused. To see why this assumption is wrong, download Android’s shared element sample. Add debug logs to the
onPause
andonResume
of the Main and Detail Activities.Navigating back from Detail to Main, without changing the orientation, generates the logs:
Navigating back from Detail to Main, after changing the orientation, generates the logs:
Notice that the Detail Activity is paused while the Main Activity is current!
Changelog
[Android] [Fixed] - Allow change of screen orientation when using the native shared element API
Test Plan
I created a new react native app based on my zoon sample that demonstrates shared element animation using the Navigation router. I followed the “Building from source” instructions to point the app at my fork with the fix in. I started the App in portrait and navigated from the grid to the details. Then I changed to landscape and navigated back to grid. I checked that no error was thrown.