fix: inactive screen visible for one frame #11315
Conversation
✅ Deploy Preview for react-navigation-example ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Hey @autofix-ci[bot]! Thanks for opening your first pull request in this repo. If you haven't already, make sure to read our contribution guidelines. |
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #11315 +/- ##
==========================================
+ Coverage 77.12% 77.16% +0.04%
==========================================
Files 211 211
Lines 6269 6280 +11
Branches 2475 2481 +6
==========================================
+ Hits 4835 4846 +11
Misses 1380 1380
Partials 54 54 ☔ View full report in Codecov by Sentry. |
|
The Expo app for the example from this branch is ready! |
WoLewicki
force-pushed
the
@wolewicki/fix-one-frame-active-screen
branch
from
e78cd13
to
c784087
Compare
|
Hi @WoLewicki thanks for this fix, is it possible to apply this same fix to @react-navigation/native-stack? could you guide me on which files to modify because I have the same behavior but in this package? Thank you very much |
|
@jonathanm-tkf |
|
Correct @WoLewicki, the temporary fix I made was to place a normal |
|
The Expo app for the example from this branch is ready! |
| @@ -609,6 +622,7 @@ export class CardStack extends React.Component<Props, State> { | |||
| }) | |||
| : STATE_TRANSITIONING_OR_BELOW_TOP; | |||
| } | |||
| this.lastActivityStateForIndex[index] = isScreenActive; | |||
There was a problem hiding this comment.
This makes render impure. We need to figure out an alternative way that keeps render pure.
There was a problem hiding this comment.
Do you have an idea what could we do to achieve it?
There was a problem hiding this comment.
Maybe let's have a call to understand the logic and come up with an idea
There was a problem hiding this comment.
I don't remember much more than what is written in the description of the PR, but we can do that for sure 😅
|
The Expo app for the example from this branch is ready! |
satya164
force-pushed
the
@wolewicki/fix-one-frame-active-screen
branch
from
d9a76fb
to
6f56ff2
Compare
satya164
force-pushed
the
@wolewicki/fix-one-frame-active-screen
branch
from
6f56ff2
to
d384587
Compare
In a scenario when we have 3 screens pushed on the stack (let's name them A, B, C in the nesting order) and we navigate from C to B, the A screen was attached for about one frame (and it should not be since it will not be visible after transition), causing problems with libraries that subscribe to native attach/detach process.
It happened because when the third screen disappears, the length of the routes becomes smaller, so in our case length becomes
2,activeScreensLimitis still1, so we do go intoelsein line609for screenA, which was inactive during the transition. In there, theoutputValueis set correctly toSTATE_INACTIVEfor after the transition, but it is interpolated from1to0(STATE_INACTIVE) (lines611-616), so it gets the1value. The transition ends in the next frame, so it quickly becomes0again, but still was attached for this one frame.The solution is to keep the information of what was the last state for each index, and if it was
STATE_INACTIVEand will be such after the transition, there is no sense to make it visible. It would be nice to test it in many scenarios for sure.@graszka22 can confirm that it fixes a problem with native attach/detach in their library.