feat(iOS, Tabs): Use UISearchTab with automaticallyActivatesSearch prop - #4679
kmichalikk wants to merge 3 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueNo actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (16)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds iOS ChangesSearch tab support
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant TabsScreen
participant RNSTabsScreenComponentView
participant RNSTabBarController
participant RNSTabsScreenViewController
participant UISearchTab
TabsScreen->>RNSTabsScreenComponentView: pass automaticallyActivatesSearch
RNSTabBarController->>UISearchTab: create search tab
RNSTabBarController->>RNSTabsScreenViewController: mirror nested search controller
RNSTabsScreenViewController->>UISearchTab: provide synchronized search configuration
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The implementation and documentation use the correct platform ranges, with no concrete merge-blocking issue identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 10 files. (6 skipped: 6 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
| * On iOS 18+, a tab screen with `systemItem: 'search'` is backed natively by `UISearchTab` | ||
| * (other values use a plain `UITab`), and the native tab class cannot change during the | ||
| * screen's lifetime. Changing `systemItem` to or from `'search'` on a mounted tab screen is | ||
| * therefore not supported - it triggers an assertion in development builds. To change the | ||
| * search role at runtime, remount the tab screen (e.g. by changing its React `key`). | ||
| * Changes between the other system item values are applied dynamically. | ||
| * | ||
| * @see {@link https://developer.apple.com/documentation/uikit/uitabbaritem/systemitem|UITabBarItem.SystemItem} | ||
| * | ||
| * @platform ios | ||
| */ | ||
| systemItem?: TabsScreenSystemItem | undefined; |
There was a problem hiding this comment.
correct me if i'm wrong, but aren't we overloading systemItem meaning? as i understand, UITabBarItem.SystemItem is for choosing the built-in localized icon and label for a tab based on predefined sets. so changing the class to something else doesn't fit with native UIKit semantics.
imo we should have a separate prop to explicitly choose to use UISearchTab, or role: "search" to match SwiftUI behavior with an explicit prop.
it'd also be nice to expose the new prominentTabIdentifier which is exposed in SwiftUI as role: "prominent"
There was a problem hiding this comment.
In 26, this specific role made the item go to the side, so I thought this would make sense to keep + the linked issue leans towards it, but I'm open to sth else once I get the core to work
prominent is blocked by the UITab API rework, but should be done shortly after that, the API for that will probably be different than role=prominent, but that for another discussion
There was a problem hiding this comment.
In 26, this specific role made the item go to the side
yes, but this is iOS behavior, so UIKit apps would also need changes. previously there have been arguments towards exposing native behavior directly in screens and let the integration layer handle platform and version differences, so using a UIKit property name to expose custom behavior in screens itself would be non-characteristic imo.
we expose this prop as tabBarSystemItem in React Navigation, so I'd prefer to avoid confusion of using a UIKit API for the name while having different behavior.
a separate prop like role: 'search' | 'prominent' to mirror the similar SwiftUI API, or some other explicit prop would be better.
cc @kkafar
3da6e47 to
1a8803e
Compare
afc8c27 to
0404bce
Compare
66f0a23 to
a7eac47
Compare
0404bce to
2b33cec
Compare
ec026ec to
aaeea05
Compare
| @@ -0,0 +1,80 @@ | |||
| # Test Scenario: Search Tab Activation (iOS) | |||
There was a problem hiding this comment.
Should this be an integration test instead?
b2008ac to
91328d6
Compare
This branch has not been deployed
2 participants
Closes #4671
Description
This PR adds
searchRoleprop that marks the tab to be backed by UISearchTab andautomaticallyActivatesSearchprop, enabling the separated search tab behavior on iOS 27 (on iOS 26 the similar feel could be achieved withsystemItemset to "search", and this system keeps supporting both).UIKit activates the
UISearchControllerattached to the tab view controller'snavigationItem, so the tab screen mirrors the search configuration (searchController, hidesSearchBarWhenScrolling, preferredSearchBarPlacement) from the stack nested inside the tab. A newRNSSearchControllerProvidingprotocol exposes the source UINavigationItem and should be implemented by both v4 and v5 stacks (the latter lacks the search for now, requires a followup in the future). The mirroring is done on every mounting transaction (updateSearchTabsIfNeeded) and just before UIKit may auto-activate search (shouldSelectTab:), so it follows the top screen of the nested stack across push/pop.Important
A live tab is never rebuilt (UIKit asserts when a view controller is resolved by a second UITab instance and crashes the application), so the tab
searchRoleis fixed at creation and changing it on a live screen is unsupported.It is technically possible to have more than 1 UISearchTab but this results in weird behavior observed on native, wrong icons used, separation for only one of the tabs, and glitches on automatic activation
Changes
searchRoleand mirrors the search controller on the stack inside the tabautomaticallyActivatesSearchprop that makes the search tab separated and automatically activateing the search input.test-tabs-search-tab-activation-iosBefore & after - visual documentation
Test plan
Use
test-tabs-search-tab-activation-iosandtest-tabs-system-item-ios- to verify no regression on iOS 26.Checklist