| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
View your CI Pipeline Execution ↗ for commit 3b803c9
☁️ Nx Cloud last updated this comment at 2026-05-21 22:15:40 UTC |
Sorry, something went wrong.
|
✅ Actions performed
Review triggered.
|
Sorry, something went wrong.
📝 Walkthrough
WalkthroughiOS tab view delegate handling is refactored to expose the More navigation controller delegate as a public property wired through TabView initialization and disposal. After iOS clears this delegate on tab selection, the tab bar callback re-asserts it. Navigation callbacks now force layout updates to ensure late-attached pages render after push animations. ChangesiOS tab view delegate and layout handling
✏️ 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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
There was a problem hiding this comment.
packages/core/ui/tab-view/index.ios.ts (2)137-144: 💤 Low value
Consider adding a defensive check for moreNavigationControllerDelegate.
The code re-asserts the delegate to ensure callbacks reach the TabView, but doesn't verify that owner.moreNavigationControllerDelegate is non-null before assignment. While unlikely during an active tab selection callback, adding a check would align with the stated intent of "re-asserting" the delegate rather than potentially clearing it.
🛡️ Proposed defensive check// iOS lazily initializes moreNavigationController (e.g. on first access of // topViewController, or on first tap of the More tab) and can clear the // delegate we set in setViewControllers. Re-assert it whenever any tab is // selected so moreNav push/pop callbacks for tabs >= 5 still reach us. const moreNav = tabBarController.moreNavigationController; -if (moreNav && moreNav.delegate !== owner.moreNavigationControllerDelegate) { +if (moreNav && owner.moreNavigationControllerDelegate && moreNav.delegate !== owner.moreNavigationControllerDelegate) { moreNav.delegate = owner.moreNavigationControllerDelegate; }
606-606: ⚡ Quick win
Add null check on moreNavigationController for consistency.
Line 142 guards access to moreNavigationController with a null check, but this line doesn't. While accessing properties on nil in Objective-C is safe (no-op), adding a consistent check improves clarity and defensive programming, especially since moreNavigationController is nil when there are fewer than 5 tabs.
♻️ Proposed consistency fix// When we set this._ios.viewControllers, someone is clearing the moreNavigationController.delegate, so we have to reassign it each time here. -this._ios.moreNavigationController.delegate = this.moreNavigationControllerDelegate; +if (this._ios.moreNavigationController) { + this._ios.moreNavigationController.delegate = this.moreNavigationControllerDelegate; +}
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 43fd6597-98a1-456b-98af-7fb25bcc3f08
📥 CommitsReviewing files that changed from the base of the PR and between 4070d91 and 3b803c9.
📒 Files selected for processing (1)
Sorry, something went wrong.
Co-authored-by: Nathan Walker <walkerrunpdx@gmail.com>
| Back | FazBrowse Home | New Git URL |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.