fix(android): Added missing nested frame restoration step#11226
Conversation
|
View your CI Pipeline Execution ↗ for commit 04b3e45
☁️ Nx Cloud last updated this comment at |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAndroid frame lifecycle and transition handling is refactored to decouple ChangesAndroid Frame Lifecycle Refactor
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e31b07ac-6d47-41c5-a4a9-7685730d7780
📒 Files selected for processing (6)
packages/core/ui/frame/fragment.transitions.android.tspackages/core/ui/frame/frame-common.tspackages/core/ui/frame/frame-helper-for-android.tspackages/core/ui/frame/index.android.tspackages/core/ui/frame/index.d.tspackages/core/ui/frame/index.ios.ts
PR Checklist
What is the current behavior?
Right now, the nested frame first simulated navigation does not work if already done once.
That usually occurs because the cached transition state is not unset after the simulated navigation.
What is the new behavior?
This PR is a continuation of #9725 & #10713 and contains the following:
onResumelifecycleonDestroylifecycle of the old fragment if entry doesn't own it anymoreloadedlifecycle (this used to help solve the problem before fix(android): Better handling for nested frames #10713 which broke it)resetRootViewin case flavors reuse the same root viewFixes issues mentioned in PR #11214.
Just a few notes about the transition restore change.
In the future, we could keep a reference of the last executed context at all times and set it as executing to simulate that navigation. This way, the simulated navigations will always reach
setCurrentcall.Right now, this isn't possible as it's breaking current
TabViewfunctionality and automated tests fail.Also, there are checks that proceed to actions based on whether a context is executing or not so it will need extended testing.
The first step to leaning to that direction is to take care of the fragment manager mess on
TabViewelements. Then, we can think about it.Luckily, this PR keeps the simulated navigation workaround as is more or less and solves the apparent problem.