Repository navigation
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: true
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 |
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 20, 2026 11:26
6fc9c98 to
bbc8c4a
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 20, 2026 11:26
9d2e663 to
72dc3de
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 20, 2026 12:32
bbc8c4a to
808265d
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 20, 2026 12:32
72dc3de to
024b609
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
3 times, most recently
from
October 20, 2026 19:25
19cc028 to
ea94447
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 20, 2026 19:25
024b609 to
906233d
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 21, 2026 09:34
ea94447 to
18c9a16
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 21, 2026 09:34
906233d to
663f19f
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
2 times, most recently
from
October 21, 2026 10:01
a38412a to
2fa7395
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 21, 2026 10:01
663f19f to
426fbe8
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 21, 2026 10:27
2fa7395 to
78158a9
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 21, 2026 10:28
426fbe8 to
d2e6c9e
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 21, 2026 10:36
78158a9 to
f456f40
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 21, 2026 10:36
d2e6c9e to
285f47a
Compare
2 of 5 tasks
…y when invalidated The `TITLE`, `BADGE` and `ACCESSIBILITY` flags only routed a change to the per-item menu update, which then re-applied all three. Now, mirroring the Stack header's `needsRebuild || flag`: - the title is applied only for `TITLE` (no more manual diffing), - the badge only for `BADGE`, except in the whole-bar appearance update, which re-applies every badge because badge colours come from the selected tab's appearance, - accessibility only for `ACCESSIBILITY` in the per-item update; the whole-bar update still re-applies it to every item. `TabsAppearanceApplicator.updateMenuItemAppearance` returns the flags it consumed so the coordinator and the container can gate their parts. A fresh menu item state is fully dirty, so new and rebuilt menu items get everything applied. Closes software-mansion/react-native-screens-labs#1956. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 21, 2026 17:20
f456f40 to
2a44942
Compare
…s pending flags Review follow-up: `TabsAppearanceApplicator.updateMenuItemAppearance` cleared all of a menu item's pending flags, applied only some of them (title, icon, active indicator) and returned them so the coordinator and the container could apply the rest (badge, accessibility). Who owned the flags depended on which caller looked at the return value. Now there is one consumer and the parts are applied by their owners: - `TabsContainer.applyPendingMenuItemChanges` is the only caller of `TabsMenuItemState.consumePendingFlags`. It hands the flags to `TabsAppearanceCoordinator.updateMenuItemAppearance` (title, icon, active indicator, badge) and applies accessibility through `TabsHostA11yCoordinator`. - The applicator only applies (`applyTitle`, `applyIcon`, `applyActiveIndicatorSize`, `updateBadgeAppearance`) and knows nothing about flags. An active indicator that can't be applied yet is re-invalidated through `TabsBottomNavigationView.invalidateMenuItem`, like any other change. - The whole-bar update no longer needs `isWholeBarUpdate`: it invalidates what it re-applies to every menu item (`BADGE`, as badge colours come from the selected tab; `ACCESSIBILITY` on appearance changes, replacing `setA11yPropertiesToAllTabItems`), then runs the same per-item path. `TabsContainer.updateTabBarAppearance` orders it: bar-wide appearance, menu items, fonts. No behavioural change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s missing Review follow-up. `setA11yPropertiesToTabItem` sets the test ID only when the menu item's view exists, but its `ACCESSIBILITY` flag was consumed either way, so a missing view dropped the update until the next accessibility change or bar pass (before the flag gating, any per-item update retried it). It now reports whether it found the view, and the container re-invalidates `ACCESSIBILITY` when it didn't, like the active indicator. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 21, 2026 17:43
2a44942 to
8a2bdcf
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
2 times, most recently
from
October 21, 2026 18:20
439fde1 to
467803c
Compare
…item lookup Review follow-ups: - `TabsAppearanceApplicator` and `TabsAppearanceCoordinator` methods are explicitly `internal`: only core's tabs code calls them. - The "apply, and re-invalidate if the item view is missing" checks for the active indicator and accessibility are nested ifs. - New `Menu.findMenuItemForFragment` / `Menu.requireMenuItemForFragment` helpers in `MenuHelpers.kt`, used by the whole-bar update, `getMenuItemForItem` and `getOrCreateMenuItemForFragment` instead of repeating `findItem(fragment.menuItemId)`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 21, 2026 18:28
ac9dc4e to
8bce222
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 21, 2026 18:28
467803c to
5ee72cb
Compare
Review follow-up: instead of `Menu` extensions taking a fragment, `TabsBottomNavigationView` has `findMenuItem(menuItemId)` and `requireMenuItem(menuItemId)`, so call sites pass `fragment.menuItemId` directly. `MenuHelpers.kt` is back to its state on `main`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 21, 2026 18:37
8bce222 to
0fa6b66
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 21, 2026 18:37
5ee72cb to
4341e80
Compare
Review follow-up (adversarial review): `onItemInvalidated` still said the applicator re-applies the invalidated parts, `consumePendingFlags` promised every flag is applied although `APPEARANCE` is bar-wide, and "retry on the next update" suggested a scheduled retry where the flag is only kept pending. Comments only. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 21, 2026 18:38
0fa6b66 to
9d8597d
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 21, 2026 18:38
4341e80 to
4bf2c65
Compare
Review decision: only core's `TabsContainer` uses it, and this PR changes its signature anyway (no `tabsScreenFragments` parameter, a Boolean from `setA11yPropertiesToTabItem`), so it should not be part of the public surface. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 21, 2026 18:46
9d8597d to
a5d4810
Compare
kkafar
force-pushed
the
@kkafar/rfc-1823-android-stack-public-contracts
branch
from
October 21, 2026 18:46
4bf2c65 to
a5d4810
Compare
…e React transaction Tab configuration changes used to reach the tab bar as they arrived: a title, icon, badge or accessibility change updated that tab's menu item synchronously inside the prop setter, and appearance changes posted a whole-bar update. React delivers props in batches, so one transaction could update the same menu item several times, and the posted bar update could run apart from the transaction that caused it. Mirror `StackContainer.beginUpdateBatch` / `endUpdateBatch`: `TabsHost.willMountItems` / `didMountItems` now bracket the transaction with `TabsContainer.beginUpdateBatch` / `endUpdateBatch`. Inside a batch, tab invalidations only record their flags (per menu item, plus `isMenuItemsInvalidated` / `isNavigationMenuAppearanceInvalidated` on the container) and are applied once by the flush at the end of the batch (`updateMenuItemsIfNeeded` after the whole-bar update). Changes raised outside a batch (asynchronous icon loads, configuration changes) post a flush of their own. A detached container keeps accumulating and flushes when attached, as before. This moves menu item updates to the end of the transaction, so it is a timing change, not a structural one. Closes software-mansion/react-native-screens-labs#1957. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ateScheduler TabsContainer posted `flushPendingUpdates` for every change raised outside a React transaction (`tabBarHidden`, `scheduleFlushOutsideUpdateBatch`), so several changes in one main loop iteration sent several messages, each running its own flush. Inside a transaction it held a boolean and flushed at its end whether or not anything had changed. Add `ContainerUpdateScheduler` to core's containers/common: it takes a single update callback and coalesces requests into one run of it. `requestUpdate` posts one run to the main thread's message queue and does nothing while that run is pending; between `beginBatch` and `endBatch` requests are only recorded and `endBatch` runs the update once if any was requested; `flush` runs it right away (inside a batch too) and drops the posted run; `cancel` drops it without running. TabsContainer now requests an update after each invalidation (tab bar visibility, navigation state request, adding and removing items, tab configuration changes), `begin/endUpdateBatch` delegate to the scheduler, `flushPendingUpdates` flushes it, and `tearDown` cancels a pending run. The `tabBarHidden` change is applied at the end of the transaction that set it instead of in a posted runnable. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xx1CNDu9HyxkbE3nRiXrfb
A tap reaches `TabsContainer.onMenuItemSelected` through Material's `menu.performItemAction` (the menu view's click listener), not through `setSelectedItemId`, so `onAfterSetSelectedItemId(USER)` never runs for it. The appearance invalidated by the selection change was only applied by the next container update, usually the React transaction that echoes the new navigation state from JS. Request an update for user-originated selection changes, so the scheduler applies the new tab's appearance on the next main loop iteration. Programmatic changes keep applying it later in the same `performContainerUpdate` pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xx1CNDu9HyxkbE3nRiXrfb
Review follow-up to #4921: a core consumer must be able to provide its own container item, so the stack's item contracts are public API of the core, as the tabs' are: `StackContainerItem` and the types its members name - `PreventNativeDismissChangeObserver`, `OnHeaderConfigurationAttachListener`, `StackHeaderDelegate`, `StackHeaderConfigurationProviding` with `StackHeaderConfigurationObserver`, `StackHeaderInvalidationFlags`, `StackHeaderType`, `StackHeaderCollapsedTitleGravityMode` and `TextAppearance`. `StackHeaderToolbarMenuController` becomes a public class with an internal constructor; its members were already internal. `StackScreen`, `StackHost` and their view managers are public again, as before #4914, which made them internal only because the item contract was internal. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kkafar
force-pushed
the
@kkafar/rfc-1823-android-tabs-batch-item-updates
branch
from
October 22, 2026 09:23
f4283d0 to
4ff4e0f
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by Kacper
Description
Follow-up to the #4921 review: a core consumer must be able to provide its own container item, so the item contracts are public API of the core. #4921 makes the tabs' contracts public. This PR does the same for Stack v5, whose item contract #4914 introduced as
internal.Before:
StackContainerItemand the types it names wereinternal, so #4914 also had to makeStackScreen,StackHostand their view managersinternal.After: the item contract and the types it names are public, and
StackScreen,StackHostand their view managers are public again, as before #4914. Visibility only, no behavioural change.Follow-up to https://github.com/software-mansion/react-native-screens-labs/issues/1905 (A5).
Stacked on #4976.
Changes
StackContainerItemand the types its members name:PreventNativeDismissChangeObserver,OnHeaderConfigurationAttachListener,StackHeaderDelegate,StackHeaderConfigurationProviding,StackHeaderConfigurationObserver,StackHeaderInvalidationFlags,StackHeaderType,StackHeaderCollapsedTitleGravityModeandTextAppearance(core.common.text).StackHeaderToolbarMenuController(named byStackHeaderConfigurationProviding.toolbarMenuController) is now a public class with aninternalconstructor. All its members were alreadyinternal, so its menu model and update types stayinternal.StackScreen,StackHost,StackScreenViewManagerandStackHostViewManagerare public again.Left
internal, for the public API task:StackContainerandStackContainerDelegate(internal since before #4914),StackHeaderConfigand its view manager. A core consumer can implementStackContainerItem, but it can't create a container yet.Test plan
Run locally:
yarn lint-android(layer check +spotlessCheck) passes.:react-native-screens:compileDebugKotlinsucceeds.On device (Android, FabricExample, release build of this branch, Pixel 3a API 34 emulator):
FabricExample/e2e/single-feature-tests/stack-v5/(14 suites; the-iosfiles are iOS-only): 213 tests pass.test-stack-lift-on-scroll-androidfailed once ("should restore lift for the undefined (default) value") and passed on two re-runs; a visibility-only change can't affect it. Crash buffer empty.Checklist
Generated by Claude Code