Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/tall-icons-grow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'react-native-bottom-tabs': minor
'@bottom-tabs/react-navigation': minor
---

Add a per-screen option for customizing tab image icon size.
8 changes: 8 additions & 0 deletions docs/docs/docs/guides/standalone-usage.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ Each route in the `routes` array can have the following properties:
- `title`: Display title for the tab
- `focusedIcon`: Icon to show when tab is active
- `unfocusedIcon`: Icon to show when tab is inactive (optional)
- `iconSize`: Width and height of the image icon in logical pixels. Uses the native default when omitted.
- `iconRenderingMode`: Rendering mode for icons. Use `'original'` to preserve multicolor icons instead of applying the native tab tint.
- `badge`: Badge text to display on the tab
- `activeTintColor`: Custom active tint color for this specific tab
Expand Down Expand Up @@ -290,6 +291,13 @@ Function to get the rendering mode for an image icon.

Use `original` to preserve the original colors of multicolor image icons, such as logos or branded assets, instead of applying the native tab tint.

#### `getIconSize`

Function to get the width and height of an image icon in logical pixels.

- Default: Uses `route.iconSize`
- When omitted: Uses the native platform default

#### `getHidden`

Function to determine if a tab should be hidden.
Expand Down
15 changes: 15 additions & 0 deletions docs/docs/docs/guides/usage-with-react-navigation.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,21 @@ Function that given `{ focused: boolean }` returns `ImageSource` or `AppleIcon`
SF Symbols are only supported on Apple platforms.
:::

#### `tabBarIconSize`

Width and height of the tab image icon in logical pixels. This can be set per screen, and uses the native platform default when omitted.

```tsx
<Tab.Screen
name="Profile"
component={Profile}
options={{
tabBarIcon: () => ({ uri: avatarFileUri }),
tabBarIconSize: 34,
}}
/>
```

#### `tabBarIconRenderingMode`

Rendering mode for image icons.
Expand Down
5 changes: 5 additions & 0 deletions packages/example-shared/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import TabBarHidden from './Examples/TabBarHidden';
import CustomTabBar from './Examples/CustomTabBar';
import NativeBottomTabsTabBarHidden from './Examples/NativeBottomTabsTabBarHidden';
import OriginalIconColors from './Examples/OriginalIconColors';
import IconSizes from './Examples/IconSizes';
import NativeBottomTabsOriginalIcons from './Examples/NativeBottomTabsOriginalIcons';
import { useLogger } from '@react-navigation/devtools';
import LazyTabs from './Examples/LazyTabs';
Expand Down Expand Up @@ -126,6 +127,10 @@ const examples = [
component: OriginalIconColors,
name: 'Original icon colors',
},
{
component: IconSizes,
name: 'Icon sizes',
},
{
component: FourTabsRippleColor,
name: 'Four Tabs with ripple Color',
Expand Down
52 changes: 52 additions & 0 deletions packages/example-shared/src/Examples/IconSizes.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import TabView, { SceneMap } from 'react-native-bottom-tabs';
import { useState } from 'react';
import { Article } from '../Screens/Article';
import { Albums } from '../Screens/Albums';
import { Contacts } from '../Screens/Contacts';
import { Chat } from '../Screens/Chat';

const renderScene = SceneMap({
article: Article,
albums: Albums,
contacts: Contacts,
chat: Chat,
});

export default function IconSizes() {
const [index, setIndex] = useState(0);
const [routes] = useState([
{
key: 'article',
title: 'Default',
focusedIcon: require('../../assets/icons/article_dark.png'),
},
{
key: 'albums',
title: 'Size 20',
focusedIcon: require('../../assets/icons/grid_dark.png'),
iconSize: 20,
},
{
key: 'contacts',
title: 'Size 32',
focusedIcon: require('../../assets/icons/person_dark.png'),
iconSize: 32,
},
{
key: 'chat',
title: 'Size 40',
focusedIcon: require('../../assets/icons/chat_dark.png'),
iconSize: 40,
},
]);

return (
<TabView
sidebarAdaptable
labeled
navigationState={{ index, routes }}
onIndexChange={setIndex}
renderScene={renderScene}
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ import com.facebook.react.common.assets.ReactFontManager
import com.facebook.react.modules.core.ReactChoreographer
import com.facebook.react.views.text.ReactTypefaceUtils
import com.google.android.material.bottomnavigation.BottomNavigationView
import com.google.android.material.navigation.NavigationBarItemView
import com.google.android.material.navigation.NavigationBarView.LABEL_VISIBILITY_AUTO
import com.google.android.material.navigation.NavigationBarView.LABEL_VISIBILITY_LABELED
import com.google.android.material.navigation.NavigationBarView.LABEL_VISIBILITY_UNLABELED
import com.google.android.material.transition.platform.MaterialFadeThrough
import kotlin.math.roundToInt

private data class DrawableCacheKey(val source: ImageSource, val size: Int)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.


class ExtendedBottomNavigationView(context: Context) : BottomNavigationView(context) {
override fun getMaxItemCount(): Int {
Expand All @@ -59,7 +63,8 @@ class ReactBottomNavigationView(context: Context) : LinearLayout(context) {
var disablePageAnimations = false
var items: MutableList<TabInfo> = mutableListOf()
private val iconSources: MutableMap<Int, ImageSource> = mutableMapOf()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor · Android drawable cache is keyed per size and never evicted

drawableCache was Map<ImageSource, Drawable> (one entry per icon); it is now Map<DrawableCacheKey, Drawable> keyed by (source, sizePx) (RCTTabView.kt:67), and nothing ever removes entries — onDropViewInstance() only shuts the ImageLoader down (line 493-495). Every distinct size requested for an icon adds a new full-size BitmapDrawable that is kept for the lifetime of the view, and the bitmap is the decoded size of the request (.precision(Precision.EXACT), .scale(Scale.FILL), lines 400-404).

Suggested change

Bound the cache: drop entries for a source when its size changes (per-source map holding the last size only, or an LRU with a small maximum), and clear it in onDropViewInstance.

Details

Reachable case: a tab whose size is driven by state — tabBarIconSize from a runtime option, or getIconSize({ route }) reading anything that changes (a zoom slider, a focus-based size, a breakpoint). updateItems() re-runs on every size change (line 248-270) and each new pixel size caches another drawable for the same source, while the drawable for the previous size is still referenced. With a handful of icons and, say, 20 distinct sizes that is 20 decoded bitmaps per icon, on a view that lives for the whole app session. Previously the map was bounded by the number of distinct icon sources.

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.

private val drawableCache: MutableMap<ImageSource, Drawable> = mutableMapOf()
private var iconSizes: List<Double> = emptyList()
private val drawableCache: MutableMap<DrawableCacheKey, Drawable> = mutableMapOf()

private var isLayoutEnqueued = false
private var selectedItem: String? = null
Expand Down Expand Up @@ -240,7 +245,10 @@ class ReactBottomNavigationView(context: Context) : LinearLayout(context) {
}
}

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()
// If an item got removed, let's re-add all items
if (items.size < this.items.size) {
bottomNavigation.menu.clear()
Expand All @@ -255,9 +263,9 @@ class ReactBottomNavigationView(context: Context) : LinearLayout(context) {
menuItem.isVisible = !item.hidden
updateIconTintMode(menuItem, item)
if (iconSources.containsKey(index)) {
getDrawable(iconSources[index]!!) {
menuItem.icon = it
updateIconTintMode(menuItem, item)
val iconSizePx = getIconSize(item)
getDrawable(iconSources[index]!!, iconSizePx) {
applyIcon(menuItem, index, iconSizePx, it)
}
}

Expand Down Expand Up @@ -288,6 +296,8 @@ class ReactBottomNavigationView(context: Context) : LinearLayout(context) {

view.findViewById<View>(com.google.android.material.R.id.navigation_bar_item_content_container)
?.setTabTestID(item.testID)

updateIconSize(menuItem, item)
}
}
}
Expand All @@ -298,6 +308,11 @@ class ReactBottomNavigationView(context: Context) : LinearLayout(context) {
}
}

fun setIconSizes(iconSizes: List<Double>) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor · Android: setIconSizes runs a full updateItems pass on every prop update, re-enqueuing every icon load and every badge

setIconSizes calls updateItems(items), i.e. the complete item update: for every index it calls getIconSize + getDrawable (a cache lookup or a new Coil request), bottomNavigation.getOrCreateBadge(index) / removeBadge(index), and posts two blocks that re-attach click/long-click listeners and call updateIconSize. The JS side builds iconSizes with React.useMemo(... , [getIconSize, trimmedRoutes]); getIconSize defaults to a new arrow function on every render (getIconSize = ({route}) => route.iconSize), so iconSizes gets a new array reference whenever the parent re-renders, the codegen array is serialized again, and the whole item update runs on the UI thread each time. getIcon/getIconRenderingMode etc. have the same shape but their setters only ran when the item/icon payload actually changed, whereas iconSizes changes identity every render even when the numbers are identical. Either compare against the previous iconSizes before re-running updateItems, or make the JS memo not depend on a default closure (e.g. React.useCallback for the default getter) so an unchanged array is not resent.

Suggested change

Skip the reload when the incoming iconSizes array equals the stored one, and/or avoid re-creating the default getIconSize closure on every render so the memo stays stable.

Details

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.

this.iconSizes = iconSizes
updateItems(items)
}

private fun getOrCreateItem(index: Int, title: String): MenuItem {
return bottomNavigation.menu.findItem(index) ?: bottomNavigation.menu.add(0, index, 0, title)
}
Expand Down Expand Up @@ -344,11 +359,9 @@ class ReactBottomNavigationView(context: Context) : LinearLayout(context) {

// Update existing item if exists.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor · Android: setIcons resolves a missing item to the default icon size instead of deferring, so a default-size drawable can be applied before the real size is known

Open ledger item, stated at the strength the code supports. getIconSize(item) returns bottomNavigation.itemIconSize whenever the item is null or has no size (line 423). setIcons calls it with items.getOrNull(idx), which is null whenever the icons prop is bridged before the items prop (both arrive in the same JS commit, and the codegen delegate dispatches them in declaration order, so this is the normal first-mount case whenever items is set first but a later update re-sends icons after an item removal). The drawable is then decoded at the default size, cached under DrawableCacheKey(source, defaultPx), and applied through applyIcon; a following updateItems issues a second request at the real size, so the wrong-size bitmap is applied only transiently and is later replaced — hence minor, not major. The related risk that is NOT present: because size is part of DrawableCacheKey, a default-size decode can never be served to a later custom-size request, so nothing permanent is cached wrongly. Worth tightening by having setIcons skip/queue items that are not present in this.items instead of resolving them to the default size.

Suggested change

In setIcons, when items.getOrNull(idx) is null, skip the eager getDrawable/applyIcon (the subsequent updateItems will request the icon at the correct size) instead of resolving the size to bottomNavigation.itemIconSize, so a default-size drawable is never applied to a tab that declares a custom size.

Details

persona: swift-ios

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.

bottomNavigation.menu.findItem(idx)?.let { menuItem ->
getDrawable(imageSource) {
menuItem.icon = it
items.getOrNull(idx)?.let { item ->
updateIconTintMode(menuItem, item)
}
val iconSizePx = getIconSize(items.getOrNull(idx))
getDrawable(imageSource, iconSizePx) {
applyIcon(menuItem, idx, iconSizePx, it)
}
}
}
Expand All @@ -374,12 +387,16 @@ class ReactBottomNavigationView(context: Context) : LinearLayout(context) {
}

@SuppressLint("CheckResult")
private fun getDrawable(imageSource: ImageSource, onDrawableReady: (Drawable?) -> Unit) {
drawableCache[imageSource]?.let {
private fun getDrawable(
imageSource: ImageSource,
iconSizePx: Int,
onDrawableReady: (Drawable?) -> Unit
) {
val cacheKey = DrawableCacheKey(imageSource, iconSizePx)
drawableCache[cacheKey]?.let {
onDrawableReady(it)
return
}
val iconSizePx = bottomNavigation.itemIconSize
val request = ImageRequest.Builder(context)
.data(imageSource.getUri(context))
.size(CoilSize(iconSizePx, iconSizePx))
Expand All @@ -388,7 +405,7 @@ class ReactBottomNavigationView(context: Context) : LinearLayout(context) {
.target { drawable ->
post {
val stateDrawable = drawable.asDrawable(context.resources)
drawableCache[imageSource] = stateDrawable
drawableCache[cacheKey] = stateDrawable
onDrawableReady(stateDrawable)
}
}
Expand All @@ -402,6 +419,34 @@ class ReactBottomNavigationView(context: Context) : LinearLayout(context) {
imageLoader.enqueue(request)
}

private fun getIconSize(item: TabInfo?): Int {
val iconSize = item?.iconSize ?: return bottomNavigation.itemIconSize
return TypedValue.applyDimension(
TypedValue.COMPLEX_UNIT_DIP,
iconSize.toFloat(),
resources.displayMetrics
).roundToInt()
}

private fun applyIcon(menuItem: MenuItem, index: Int, iconSizePx: Int, drawable: Drawable?) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 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.

// Icons load asynchronously, so read the latest item instead of the one captured at request time.
val item = items.getOrNull(index)
// The icon size changed while loading, a request for the new size is already in flight.
if (getIconSize(item) != iconSizePx) return

menuItem.icon = drawable
item?.let {
updateIconTintMode(menuItem, it)
updateIconSize(menuItem, it)
}
}

private fun updateIconSize(menuItem: MenuItem, item: TabInfo) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor · Android: per-tab size relies on Material's restricted NavigationBarItemView class

updateIconSize resolves com.google.android.material.navigation.NavigationBarItemView, a Material class restricted to the Material library group and not part of Material's public API, and calls setIconSize on it. The only lint suppression in android/build.gradle is disable "GradleCompatible" (build.gradle:81-83), so any gradle lint / AndroidX-lint run in a consumer build or in CI can flag this as a restricted-API usage, and Material is free to rename or remove the class or the method in a 1.14.0-alpha* bump — this package pins material:1.14.0-alpha05, an alpha line where that presenter class has moved before. The existing code in the same file avoided the concrete type by using findViewById<View>(...) and Material's public surface. Prefer a public API (e.g. BottomNavigationView#setItemIconSize / the theme attribute where available), or at least keep the lookup as View and set the size through a supported path so the module does not break when Material changes an internal class.

Suggested change

Replace the NavigationBarItemView#setIconSize call with a public Material API for per-item icon size (or a drawable-level resize) and note the dependency requirement in the README/changeset if the internal class has to stay.

Details

Also raised here

Also raised at this location by cpp: Android: per-tab sizing depends on Material's internal NavigationBarItemView.setIconSize (RestrictedApi risk + crash on older Material) — This PR reaches into Material's internals to size a single tab icon: import com.google.android.material.navigation.NavigationBarItemView (line 40) and bottomNavigation.findViewById<NavigationBarItemView>(menuItem.itemId) + itemView.setIconSize(getIconSize(item)) (lines 444-448). Two things a maintainer should confirm before merging: NavigationBarItemView and its setIconSize(int) are Material library internals (@RestrictTo(LIBRARY_GROUP_EXT)-level API in Material 1.x — the public surface is NavigationBarView/BottomNavigationView, which exposes only a bar-wide itemIconSize). Calling them from an app/library module makes the project's lint fail with RestrictedApi (lint's RestrictedApi check names it) and removes the icon size per item whenever Material changes the internal view hierarchy. NavigationBarItemView.setIconSize is only present in the Material version the library declares (com.google.android.material:material:1.14.0-alpha05, build.gradle line 115). …

persona: jvm, 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor · Android: updateIconSize()/icon layout is never reset when a tab has no iconSize (and after a configuration change)

NavigationBarItemView.setIconSize(px) is a per-item override in Material's NavigationBarItemView that survives menu invalidation (it re-applies its own fields in postUpdateMenuEntryViews). updateIconSize always sets it from getIconSize(item) — so when item.iconSize is null it sets the current bottomNavigation.itemIconSize. That is only correct as long as the bar-level itemIconSize equals what the item would get from the style/theme. iconSizes is a new optional native prop that is not forwarded by the out-of-tree Fabric path in RCTTabViewComponentView.mm, and onConfigurationChanged builds a brand-new ExtendedBottomNavigationView and calls updateItems(items) with the sizes stored in this view — the freshly created bar has not yet received any setItemIconSize from the new prop, so items keep a pixel size derived from the previous bar. Reset the item to NO_SIZE/the theme default when item.iconSize == null instead of freezing the value read from the current bar, and re-apply sizes after the bar is recreated.

Details

Also raised here

Also raised at this location by senior-developer: Android never resets a tab's icon size once one has been applied — updateIconSize() (RCTTabView.kt:444-448) only ever calls NavigationBarItemView.setIconSize(px); there is no path that restores Material's default once a size has been set. getIconSize() returns bottomNavigation.itemIconSize (the global default) when item.iconSize == null, so once a tab has been laid out at, say, 40 dp, removing the size (tabBarIconSize removed from options, or route.iconSize deleted) keeps the view at 40 dp — the view-level override stays. iOS does not behave this way: TabViewProvider.updateItems() explicitly sets item.iconSize = nil when the incoming iconSizes entry is 0/missing, so the same change resets to the 27 pt default on iOS. Same JS code, two different results per platform, and no test covers it. …

persona: cpp, 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor · updateIconSize silently no-ops while the tab item view is not laid out yet, and nothing retries

updateIconSize looks the NavigationBarItemView up with bottomNavigation.findViewById(menuItem.itemId) and returns when it is null. The item views are created by NavigationBarMenuView only when the menu is built and laid out. setIconSizes -> updateItems(items) posts updateIconSize(menuItem, item) once, so if the iconSizes prop is applied before the bar has been laid out (a re-mount / activity recreate path, or any ordering where setIconSizes lands before the first layout pass), the per-item size is never applied and the tab keeps Material's default icon size with no retry. The drawable itself is also decoded at the requested px size (Coil request .size(CoilSize(iconSizePx, iconSizePx))), so the icon ends up the right bitmap inside a default-sized slot. applyIcon covers the case only when an icon load happens to complete after the item view exists.

Suggested change

Make the icon-size application not depend on the item view existing at that instant: keep the desired px size per index and (re)apply it from a layout callback / addOnLayoutChangeListener on the bottom navigation view, or apply sizes from onConfigurationChanged/after the first layout pass, so a lost findViewById lookup is retried.

Details

persona: swift-ios

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.

val itemView = bottomNavigation.findViewById<NavigationBarItemView>(menuItem.itemId)
?: return
itemView.setIconSize(getIconSize(item))
}

fun setBarTintColor(color: Int?) {
// Set the color, either using the active background color or a default color.
val backgroundColor =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ data class TabInfo(
val badgeBackgroundColor: Int?,
val badgeTextColor: Int?,
val activeTintColor: Int?,
val iconSize: Double?,
val iconRenderingMode: String?,
val hidden: Boolean,
val testID: String?
Expand Down Expand Up @@ -105,6 +106,7 @@ class RCTTabViewManager(context: ReactApplicationContext) :
badgeBackgroundColor = if (item.hasKey("badgeBackgroundColor")) item.getInt("badgeBackgroundColor") else null,
badgeTextColor = if (item.hasKey("badgeTextColor")) item.getInt("badgeTextColor") else null,
activeTintColor = if (item.hasKey("activeTintColor")) item.getInt("activeTintColor") else null,
iconSize = null,
iconRenderingMode = if (item.hasKey("iconRenderingMode")) item.getString("iconRenderingMode") else null,
hidden = if (item.hasKey("hidden")) item.getBoolean("hidden") else false,
testID = item.getString("testID")
Expand All @@ -126,6 +128,16 @@ class RCTTabViewManager(context: ReactApplicationContext) :
view.setIcons(value)
}

override fun setIconSizes(view: ReactBottomNavigationView?, value: ReadableArray?) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 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.

view?.setIconSizes(
if (value == null) {
emptyList()
} else {
(0 until value.size()).map { value.getDouble(it) }
}
)
}

override fun setFocusedIcons(view: ReactBottomNavigationView?, value: ReadableArray?) {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
@property(nonatomic, strong) NSNumber *fontSize;
@property(nonatomic) BOOL tabBarHidden;
@property(nonatomic, copy) NSArray *itemsData;
@property(nonatomic, copy) NSArray<NSNumber *> *iconSizes;
- (void)setImageLoader:(RCTImageLoader *)imageLoader;
- (void)insertChild:(UIView *)child atIndex:(NSInteger)index;
- (void)removeChildAtIndex:(NSInteger)index;
Expand All @@ -53,6 +54,7 @@ FOUNDATION_EXPORT NSObject *RNCCreateBottomAccessoryProvider(id<RNCBottomAccesso
sfSymbol:(NSString *)sfSymbol

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor · RNCTabInfo createWithKey:...: gains a required iconSize: argument — source-breaking for SwiftPM consumers, undocumented

TabInfo is a Swift @objc class, so the SwiftPM path cannot add the new parameter with a default: the C function in the bridge header changed signature, and the call site in RCTTabViewComponentView.mm passes iconSize:nil. Objective-C initializers/functions have no default arguments, so any app that links the package with SWIFT_PACKAGE and calls RNCCreateTabInfo/+[RNCTabInfo createWithKey:...] itself (or subclasses/bridges TabViewProvider) stops compiling with "no known class method for selector ... iconSize:..." — the exact error this PR fixes inside its own example. The changeset is only Add a per-screen option for customizing tab image icon size. and neither guide mentions that the SwiftPM bridge symbol gained a parameter, so downstream SwiftPM integrators get an unannounced compile break.

Suggested change

Mention in the changeset (and, if the SwiftPM bridge is public API, in the upgrade/migration docs) that +[RNCTabInfo createWithKey:...] gained the iconSize: parameter, or keep an overload (createWithKey:...activeTintColor:iconRenderingMode:...) that forwards iconSize:nil so existing SwiftPM call sites keep compiling.

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.

focusedSfSymbol:(NSString *)focusedSfSymbol
activeTintColor:(UIColor *)activeTintColor
iconSize:(NSNumber *)iconSize
iconRenderingMode:(NSString *)iconRenderingMode
hidden:(BOOL)hidden
testID:(NSString *)testID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ + (NSObject *)createWithKey:(NSString *)key
sfSymbol:(NSString *)sfSymbol
focusedSfSymbol:(NSString *)focusedSfSymbol
activeTintColor:(UIColor *)activeTintColor
iconSize:(NSNumber *)iconSize
iconRenderingMode:(NSString *)iconRenderingMode
hidden:(BOOL)hidden
testID:(NSString *)testID
role:(NSString *)role
preventsDefault:(BOOL)preventsDefault {
return [[TabInfo alloc] initWithKey:key title:title badge:badge sfSymbol:sfSymbol
focusedSfSymbol:focusedSfSymbol activeTintColor:activeTintColor
iconSize:iconSize
iconRenderingMode:iconRenderingMode hidden:hidden testID:testID
role:role preventsDefault:preventsDefault];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ - (void)updateProps:(Props::Shared const &)props oldProps:(Props::Shared const &
const auto &oldViewProps = *std::static_pointer_cast<RNCTabViewProps const>(_props);
const auto &newViewProps = *std::static_pointer_cast<RNCTabViewProps const>(props);

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);
}
Expand Down Expand Up @@ -237,6 +245,7 @@ - (void)updateProps:(Props::Shared const &)props oldProps:(Props::Shared const &
sfSymbol:RCTNSStringFromStringNilIfEmpty(item.sfSymbol)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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.

focusedSfSymbol:RCTNSStringFromStringNilIfEmpty(item.focusedSfSymbol)
activeTintColor:RCTUIColorFromSharedColor(item.activeTintColor)
iconSize:nil
iconRenderingMode:RCTNSStringFromStringNilIfEmpty(item.iconRenderingMode)
hidden:item.hidden
testID:RCTNSStringFromStringNilIfEmpty(item.testID)
Expand Down
Loading
Loading