Repository navigation
feat: add per-tab icon sizing - #608
teddyxfire1992 wants to merge 4 commits into
Conversation
Adds `iconSize` / `getIconSize` for standalone usage and `tabBarIconSize` for React Navigation. Omitted, zero or invalid sizes keep the platform default. Originally opened as callstack#562, rebased onto main and adapted to the tab bar item image cache. The SwiftPM bridge now forwards `iconSizes` and `iconSize` so the SwiftPM build compiles.
Icon load callbacks applied the tab item captured when the request started. When `iconSizes` arrived while an icon was still loading, the late callback reset the icon to the default size. Read the latest item when the icon is ready and drop results loaded at an outdated size.
Adds an "Icon sizes" example based on Four Tabs that shows the default size next to 20, 32 and 40, and removes `tabBarIconSize` from the original icons example.
26a87e5 to
132e513
Compare
Restores the placeholder test in `react-native-bottom-tabs`, removes the React Navigation test and rolls back the added jest and react-test-renderer dev dependencies.
|
@apex-sentinel-staging review |
|
Sentinel reviews this repository only for the account that connected the installation, and for logins in |
|
/review |
|
@apex-sentinel-staging review |
Apex SentinelVerdict: 20 finding(s): 3 major, 11 minor, 6 nit. Verdict: request_changes. reached max rounds (1). Required fixes
🟠 Major (3)
🟡 Minor (11)
🔵 Nit (6)
Reply under an inline comment with |
There was a problem hiding this comment.
Apex Sentinel: 20 new findings on 398b6bc. The verdict and the full list are in the summary comment.
| icon: assetIcon.map(IconSource.image) ?? .sfSymbol(tabData.sfSymbol), | ||
| focusedIcon: focusedAssetIcon.map(IconSource.image) ?? tabData.focusedSfSymbol.map(IconSource.sfSymbol), | ||
| title: tabData.title, | ||
| iconSize: iconSize(for: tabData), |
There was a problem hiding this comment.
🟠 Major · iOS: per-tab iconSize is dead in the default (non baked-tint) path — SFSymbol tabs and the SwiftUI label icon ignore it
TabViewImpl.iconSize(for:) is only consumed inside TabBarItemImageCache.Inputs, i.e. it only changes the rendering done by makeTabBarItemImage, which is used when inputs.rendersLabelIntoImage is true — that requires useBakedTintColors (experimental_bakedTintColors + iOS 26) and hasCustomTintColors && labeled. In every other configuration the images that actually reach the tab bar are produced by renderTabBarIcon(...), which just returns icon / icon.withTintColor(...) at the icon's own size, and the unselected icon is owned by SwiftUI through the tab label. The only code that applies iconSize for asset icons is TabViewProvider.loadIcons (image.resizeImageTo(size: iconSize(at: index))), so:
Suggested change
Apply iconSize on the path that is always taken (resize the image used for the tab label / selected image, including SF Symbols via a symbol configuration with pointSize = iconSize), or document that iconSize only affects asset icons and only on the rendering path where it is actually applied.
Details
- An SF Symbol tab (
route.focusedIcon: { sfSymbol: '...' }, no asset) never gets resized:loadIconsonly iterates theiconsarray built fromRCTImageSources (asset/uri icons), soprops.icons[itemIndex]is nil andmakeSFSymbolImage(named:)produces a symbol rendered at UIKit's default size. PassingtabBarIconSize: 40on such a tab changes nothing on iOS unlessexperimental_bakedTintColorsis on. - Same for the unselected icon in general: it comes from the SwiftUI label, not from
makeTabBarItemImage, so a per-tab size is not applied to it outside the baked-tint path.
The PR's device verification used PNG assets in the example, which is why the feature appears to work there. Either resize the icon in renderTabBarIcon/the label image (and for SF Symbols via UIImage.SymbolConfiguration(pointSize:)), or state that iconSize applies to asset icons only in the docs added in this PR.
Also raised here
Also raised at this location by jvm: SwiftUI path never receives iconSize: TabItem builds Image without it, so SF Symbols and tab-bar background images ignore the new size — Everywhere else the size never reaches the rendered icon: TabItem.swift (unchanged by this PR) builds Image(uiImage:) without passing the new iconSize parameter, and makeTabBarItemImage is the only place that draws an icon at a requested point size. So on the default iOS path only image icons resized in TabViewProvider.loadIcons honour iconSize; SF Symbol tabs (UIImage(systemName:), sized by SwiftUI's font) and the non-baked label/selected images stay at the system size, while the doc page added in this PR states the option is the "width and height of the image icon" for any tab. The changeset and docs promise the option unconditionally, so either the size has to be applied on the SwiftUI/UITabBarItem path too (e.g. symbol configuration / image resizing for SF Symbols) or the docs and the option comment must say it only affects loaded image assets (and, for the baked-label composite, only in combination with baked tint colors).
Also raised at this location by cpp: iOS: iconSize / tabBarIconSize has no effect on SF Symbol tabs outside the experimental baked-tint path — Correction/精确 scope of my earlier finding (which overstated the case): asset icons are sized on iOS in the default path, because TabViewProvider.loadIcons resizes before storing into props.icons/props.focusedIcons, and NewTabView/LegacyTabView render props.icons[index] while configureTabBarItemImages assigns the focusedIcons-derived image. The gap that remains is SF Symbols: For a tab whose icon is an Apple Symbol (focusedIcon: { sfSymbol: 'house' }, or tabBarIcon: () => ({ sfSymbol: 'house' }) in the React Navigation wrapper), no image ever passes through loadIcons — resolvedIconAssets puts { uri: '' } for those routes — and the icon is produced by makeSFSymbolImage(named:) / Image(systemName:) in TabItem. Neither is configured with the requested point size, and renderTabBarIcon returns the symbol untouched. …
persona: cpp, jvm
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| ).roundToInt() | ||
| } | ||
|
|
||
| private fun applyIcon(menuItem: MenuItem, index: Int, iconSizePx: Int, drawable: Drawable?) { |
There was a problem hiding this comment.
🟠 Major · Android: icon scale is applied to only part of the drawable cache after an iconSize change
updateItems recomputes sizes but never clears drawableCache, and getDrawable returns a cached drawable before its size check (drawableCache[cacheKey]?.let { onDrawableReady(it); return }). After an iconSize change, applyIcon compares getIconSize(item) with the requested iconSizePx and silently returns when they differ, trusting that "a request for the new size is already running". That only holds for entries that were not in the cache yet. The entries that were already cached under the old size are never re-requested for the new key (the loop always requests the new key, which misses and starts a load; the old key's drawable stays in the map), so the tab keeps whatever menuItem.icon had before, and the guard drops the callback of any in-flight load for the old size. Failing sequence: an iconSize/tabBarIconSize update (or a route reorder that shifts iconSizes positionally, since updateItems maps sizes by index, not by key) while an icon for the old size is still loading → the item is left at the old size with no further request. Also note the post { } inside Coil's target (line ~406) already delays the callback to the end of the queue, so an old-size load finishing after the new-size load can win the last write to menuItem.icon and then be discarded by the guard, leaving the old drawable assigned with no retry. Clearing the cache in updateItems/setIconSizes (or applying the item's current size to the drawable instead of dropping the mismatch) removes the ordering dependency.
Suggested change
In updateItems(), drop the cache entries for the icon sources whose resolved size changed (or clear drawableCache wholesale) so every visible item has a request for its current size, instead of relying on the "a request is already in flight" assumption in applyIcon.
Details
persona: jvm
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| view.setIcons(value) | ||
| } | ||
|
|
||
| override fun setIconSizes(view: ReactBottomNavigationView?, value: ReadableArray?) { |
There was a problem hiding this comment.
🟠 Major · Android: setIconSizes(null) wipes every per-tab iconSize, including route.iconSize
RCTTabViewManager.setIconSizes maps a missing/null prop to emptyList(), and ReactBottomNavigationView.setIconSizes stores it and re-runs updateItems(items), whose first statement recomputes every item as item.copy(iconSize = iconSizes.getOrNull(index)?.takeIf { it > 0 }). With the parallel array empty, getOrNull(index) is null for every index, so every item's iconSize becomes null and getIconSize falls back to bottomNavigation.itemIconSize.
Suggested change
Treat a null iconSizes array as "no sizes provided" and skip the update (or keep the previous array), so that clearing the prop does not silently drop sizes; if clearing is intended, note it in the changeset.
Details
iconSizes is an optional prop of the native component, so this is reachable: any consumer that passes getIconSize/tabBarIconSize on one render and then renders without the prop (or an out-of-tree/dynamic caller that omits it) gets all icons reset to the default size, whereas before this PR there was no size at all and the default was the only behavior. Because items is overwritten with the recomputed copies, the original per-route sizes cannot be restored either — the information is lost, and a later updateItems(items) (e.g. from onConfigurationChanged) keeps the defaults. A null/absent iconSizes should leave this.iconSizes and the existing sizes untouched (or be treated as "no change"), not as "clear all sizes".
persona: cpp
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| @@ -28,7 +28,13 @@ export default function NativeBottomTabView({ | |||
| return ( | |||
There was a problem hiding this comment.
🟡 Minor · NativeBottomTabView rebuilds navigationState and every route object on each render, defeating TabView's memos and re-rendering all scenes
navigationState={{ ...state, routes: state.routes.map(route => ({ ...route, iconSize: ... })) }} creates a new state object, a new routes array and new route objects on every render of the navigator. TabView keys trimmedRoutes on navigationState.routes (TabView.tsx:296-307), so icons, focusedIcons, items and iconSizes (deps [..., trimmedRoutes]) are recomputed on every render, resolvedIconAssets/resolvedFocusedIconAssets re-run Image.resolveAssetSource for every tab, a fresh items array is handed to the native component each commit, and the route objects passed to renderScene, getLazy, getFreezeOnBlur, getSceneStyle and the SceneMap scenes are new identities — so renderScene({route, jumpTo}) receives a different route object each render, defeating React.memo/useMemo in screens keyed on the route. Since tabBarIconSize is static for the life of a screen, the copy achieves nothing but churn: any parent re-render of the navigator (state/param change, descriptor change, tabPress emit) now re-diffs the whole tab prop set.
| return ( | |
| export default function NativeBottomTabView({ | |
| state, | |
| layoutDirection, | |
| navigation, | |
| descriptors, | |
| tabBar, | |
| ...rest | |
| }: Props) { | |
| const navigationState = React.useMemo( | |
| () => ({ | |
| ...state, | |
| routes: state.routes.map((route) => ({ | |
| ...route, | |
| iconSize: descriptors[route.key]?.options.tabBarIconSize, | |
| })), | |
| }), | |
| [state, descriptors] | |
| ); | |
| return ( | |
| <TabView | |
| {...rest} | |
| navigationState={navigationState} |
Suggested change
Wrap the derived state in React.useMemo keyed on state and descriptors so the routes array and route objects keep a stable identity between renders, or (better) pass a getIconSize={({route}) => descriptors[route.key]?.options.tabBarIconSize} callback — already supported by the new getIconSize prop — instead of mutating the routes.
Details
Also raised here
Also raised at this location by swift-ios: navigationState is a fresh object with fresh route objects on every render, so trimmedRoutes no longer stays referentially stable — NativeBottomTabView now builds a brand-new routes array (and a brand-new object per route) inline in the JSX, which breaks the one referential invariant TabView has: trimmedRoutes = useMemo(..., [navigationState.routes]) (TabView.tsx:296-306) used to be stable because navigationState={state} forwarded react-navigation's stable state object. Now it recomputes on every render of the tab view, and since descriptors (and the inline getIcon/getIconSize closures) already change identity each render, that cascades into fresh icons, focusedIcons, items and iconSizes arrays and a full Image.resolveAssetSource pass every time a descendant of the tab bar re-renders — even when no option changed. …
persona: javascript, jvm, swift-ios, maintainability
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| import com.google.android.material.transition.platform.MaterialFadeThrough | ||
| import kotlin.math.roundToInt | ||
|
|
||
| private data class DrawableCacheKey(val source: ImageSource, val size: Int) |
There was a problem hiding this comment.
🟡 Minor · Android: duplicate drawables per icon size, and every size variant is kept for the view's lifetime
DrawableCacheKey puts the resolved pixel size into the key (private data class DrawableCacheKey(val source: ImageSource, val size: Int)), and drawableCache is never evicted. A tab bar whose iconSizes change over its lifetime — e.g. a size driven by state (the new getIconSize/tabBarIconSize is re-evaluated on every render, and updateItems re-runs whenever items or sizes change) — accumulates one decoded Drawable per (icon, size) pair for as long as the view lives. With 5 tabs and a handful of sizes that is several full bitmaps held in drawableCache in addition to Coil's own memory cache.
Details
The same key change also defeats the cross-tab reuse the old Map<ImageSource, Drawable> provided: two tabs that use the same asset at the same size each got one Drawable before, and still share via the key, but the same asset at 20/32/40 is now decoded and retained three times. updateItems already posts work on every call, so add a size-keyed eviction (e.g. keep only the size currently requested per source) or clear drawableCache when the sizes change.
Also raised here
Also raised at this location by maintainability: The drawable cache key gained size, so the cache is now bounded by distinct (source, size) pairs instead of distinc… — The drawable cache key gained size, so the cache is now bounded by distinct (source, size) pairs instead of distinct sources, and nothing evicts it: resizing the same tabs through several sizes (a state-driven or animated tabBarIconSize, or route churn where a size moves between indexes) keeps every rendered variant alive for the life of the view, and a size derived from layout can take many values. Before this change the worst case was one drawable per icon source. Consider clearing the entries for a source when its size changes, or bucketing the size in the key.
persona: cpp, maintainability
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| @@ -241,6 +246,9 @@ class ReactBottomNavigationView(context: Context) : LinearLayout(context) { | |||
| } | |||
|
|
|||
| fun updateItems(items: MutableList<TabInfo>) { | |||
There was a problem hiding this comment.
🔵 Nit · val items = items.mapIndexed { ... } shadows the parameter and the items property
Inside fun updateItems(items: MutableList<TabInfo>) the new first statement declares a local val items whose own initializer reads the parameter, while the class also has a var items: MutableList<TabInfo>. All three resolve to different things in the same scope, so this.items, the parameter and the local differ after this line; it only reads correctly because the parameter is shadowed. Rename it (e.g. val resolvedItems) and assign items = resolvedItems explicitly.
| fun updateItems(items: MutableList<TabInfo>) { | |
| fun updateItems(newItems: MutableList<TabInfo>) { | |
| val items = newItems.mapIndexed { index, item -> | |
| item.copy(iconSize = iconSizes.getOrNull(index)?.takeIf { it > 0 }) | |
| }.toMutableList() |
Details
Also raised here
Also raised at this location by jvm: Local val items shadows the updateItems parameter and re-reads the mutable property from setIconSizes — fun updateItems(items: MutableList<TabInfo>) { val items = items.mapIndexed { ... } } shadows the parameter with a same-named local, which only compiles because the initializer resolves to the parameter; setIconSizes then calls updateItems(items) on the already-decorated property, so the mapping is applied to its own output and onConfigurationChanged does the same. Renaming the local (e.g. resolvedItems) makes the data flow explicit and keeps items.size < this.items.size from reading one list while this.items = items writes another.
Also raised at this location by maintainability: Inside updateItems this now reads val items = items.mapIndexed { … }, so within one function items means three di… — Inside updateItems this now reads val items = items.mapIndexed { … }, so within one function items means three different things depending on position: the parameter, the size-augmented local, and the property (this.items, used one line below in items.size < this.items.size). Kotlin only warns about the shadowing, so it compiles, but the next reader has to work out which list each read refers to, and the property passed in by setIconSizes (updateItems(items)) is immediately re-derived. Renaming the local (e.g. sizedItems) and assigning the property from it keeps the intent obvious. The same applies to iconSizePx, which is a pixel value in some paths and the Material default in others — a name like iconSizePxOrDefault would say so.
persona: swift-ios, jvm, maintainability
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| @@ -127,6 +127,14 @@ - (void)updateProps:(Props::Shared const &)props oldProps:(Props::Shared const & | |||
| _tabViewProvider.itemsData = convertItemsToArray(newViewProps.items); | |||
There was a problem hiding this comment.
🔵 Nit · iOS: itemsData is assigned before iconSizes, so updateItems() runs once with the stale sizes array
In updateProps:oldProps: (line 127) itemsData is assigned before iconSizes (line 135). itemsData's didSet runs updateItems() first, which re-applies the previous self.iconSizes array to the freshly created TabInfo objects (the itemsData pass reads iconSizes[safe: index]), so the tab bar renders the new items with the previous icons/sizes for one render cycle; the correct sizes are only applied when the iconSizes block runs moments later, and loadIcons may then be issued twice. Swapping the two blocks (sizes first, items second) makes the single updateItems() pass see both new inputs at once, removes the redundant reload, and removes the stale-size intermediate state. It also avoids the case where the route order changes: updateItems maps iconSizes positionally, so between the two assignments every tab can carry a size that belongs to a different route.
| _tabViewProvider.itemsData = convertItemsToArray(newViewProps.items); | |
| if (oldViewProps.iconSizes != newViewProps.iconSizes) { | |
| NSMutableArray<NSNumber *> *iconSizes = [NSMutableArray arrayWithCapacity:newViewProps.iconSizes.size()]; | |
| for (const auto iconSize : newViewProps.iconSizes) { | |
| [iconSizes addObject:@(iconSize)]; | |
| } | |
| _tabViewProvider.iconSizes = iconSizes; | |
| } | |
| if (oldViewProps.items != newViewProps.items) { | |
| _tabViewProvider.itemsData = convertItemsToArray(newViewProps.items); | |
| } |
Suggested change
Assign _tabViewProvider.iconSizes before _tabViewProvider.itemsData in updateProps (or gate updateItems() so it runs once per props update) so the new items are built with the new sizes in a single pass.
Details
persona: jvm
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| @@ -237,6 +245,7 @@ - (void)updateProps:(Props::Shared const &)props oldProps:(Props::Shared const & | |||
| sfSymbol:RCTNSStringFromStringNilIfEmpty(item.sfSymbol) | |||
There was a problem hiding this comment.
🔵 Nit · iconSize is always passed as a constant from the prop conversion on both platforms
The new plumbing hardcodes the value instead of reading it from the prop it sits next to: convertItemsToArray passes iconSize:nil (RCTTabViewComponentView.mm:248) and Android's setItems passes iconSize = null (RCTTabViewManager.kt:109), because the value is carried in the parallel iconSizes array (iconSizes?: ReadonlyArray<Double>). That works only while two invariants hold outside this code: iconSizes is delivered in the same commit as items with the same index alignment, and its length is at least items.count. TabView.tsx keeps them aligned today (both derived from trimmedRoutes, TabView.tsx:343-395), so nothing is wrong at the moment, but an item created before its sizes arrive (or after a truncation) silently gets the platform default with no warning — and on Android a getDouble on a null/undefined entry throws rather than falling back, so an out-of-range/missing array slot fails loudly while the iOS equivalent fails quietly.
Suggested change
Prefer carrying iconSize inside the items struct (like activeTintColor and iconRenderingMode already are), which removes the index-alignment requirement and the SwiftPM bridge change entirely; if the parallel array stays, document the alignment requirement where iconSizes is declared in TabViewNativeComponent.ts.
Details
persona: senior-developer
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| return CGSize(width: value, height: value) | ||
| } | ||
|
|
||
| private func updateItems() { |
There was a problem hiding this comment.
🔵 Nit · updateItems() reloads icons on a size diff only, so a size change arriving with a new icon source in one commit can resize the old image
TabViewProvider.updateItems() (TabViewProvider.swift:337-351) decides whether to re-run loadIcons by comparing previousIconSizes != itemsData.map(\.iconSize), and the reload is driven entirely off props.items, while the new image sources arrive through the separate icons/focusedIcons didSet observers. Fabric assigns several props in one updateProps pass, so a commit that changes both the icon source and the size for the same index can run loadIcons(icons, ...) from updateItems() with the sources icons still holds — i.e. resize and cache the previous image at the new size — before the icons observer fires with the new sources. In practice both observers fire in the same pass and the icons observer re-resizes with the correct source, so the wrong result is only transient and I could not demonstrate a lasting failure; the fragility is that the correctness rests on the observer order in -[RCTTabViewComponentView updateProps:oldProps:] (items → iconSizes → icons → focusedIcons, RCTTabViewComponentView.mm:125-158) rather than on anything in updateItems().
Suggested change
Make the icon reload driven by the sources rather than by a size diff computed from a different prop (e.g. key the cached resized image on (source, size) like the Android DrawableCacheKey does), so the ordering of prop assignments in the Objective-C updateProps cannot influence which image ends up resized.
Details
persona: javascript
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| } | ||
| } | ||
|
|
||
| #if os(iOS) |
There was a problem hiding this comment.
🔵 Nit · iOS per-tab sizing depends on UIImage alignment rects being honored by UITabBarItem
Outside the experimental_bakedTintColors path, the only thing that makes a resized icon render at a non-default size in the bar is alignedIcon, which calls image.withAlignmentRectInsets with (size - 27)/2 on each edge; the icon bitmap itself is already iconSize points and nothing else (no UITabBarItem image-size API, no appearance iconSize) is set in the non-baked path. UITabBarItem/UITabBar layout is not documented as using a bar-button item image's alignment rect, so if UIKit ignores it there, a 40 pt icon would be drawn at its bitmap size only incidentally (or cropped/scaled by the bar's own icon metrics) rather than because the inset did anything — while with experimental_bakedTintColors the size is applied explicitly via makeTabBarItemImage(iconSize:). Worth a comment or an explicit mechanism so the mechanism does not silently depend on undocumented UIKit behaviour; the PR's own "label about 4 pt lower" limitation in the baked path hints the two paths are aligned by different means.
Suggested change
Confirm on the oldest supported iOS version that UITabBarItem honours image alignment rects for icon sizing, or drive the size through a mechanism that is documented (bake the icon into the target-size canvas as the baked-tint path already does) and note the dependency in a comment.
Details
persona: javascript
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
PR Description
Closes #561. Supersedes #562.
The feature was written by @thiagobrez in #562. This PR rebases it onto
mainand fixes two issues found while testing it on devices.It adds a per-tab size for image icons:
route.iconSizeorgetIconSize({ route })tabBarIconSizeIf the size is omitted, zero or invalid, the platform default is used (27 pt on iOS, Material's default on Android). Sizes go to native as a parallel
iconSizescodegen array.Changes on top of #562:
TabViewImplnow uses the tab bar item image cache frommain, soiconSizeis added to the cache inputs and passed tomakeTabBarItemImage.ios/Bridge/BottomTabsBridge.h/.mdid not forward the new prop, so the SwiftPM example did not compile (property 'iconSizes' not found on object of type 'TabViewProvider *'andno known class method for selector 'createWithKey:...iconSize:...'). The bridge now declaresiconSizesand passesiconSizetoTabInfo. The CocoaPods build was not affected.TabInfocaptured when the request started. If the items were updated while the icon was loading, the callback applied the old size, so a 34 dp icon could end up at 24 dp. The callback now reads the current item, and it skips a result whose size is out of date, because a request for the new size is already running.artifacts/issue-561/folder (screenshots, videos and hierarchy dumps, about 3.6 MB) from feat: add per-tab icon sizing #562 is not included.How to test?
apps/example-swiftpm). It must compile.Verified with agent-device:
yarn typecheckandyarn lint(3 existing warnings, no errors) pass.Known limitations, not changed here:
experimental_bakedTintColorson iOS, the label of a larger icon is about 4 pt lower than the other labels.Screenshots
The Icon sizes example.
Recordings
Switching between all tabs of the Icon sizes example.
iOS
icon-sizes-ios.mp4
Android
icon-sizes-android.mp4