Skip to content

feat(Android, Tabs): support badge in bottom navigation #3017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jul 18, 2025

Conversation

maciekstosio
Copy link
Contributor

@maciekstosio maciekstosio commented Jul 17, 2025

Description

Closes https://github.com/software-mansion/react-native-screens-labs/issues/256

This PRs adds basic support for badge for android. It allows to set color and label. To reuse the iOS logic (which allows string as badge value) I need to parse it to int and I log error otherwise. This isn't ideal and I created low-priority task to change it at some point https://github.com/software-mansion/react-native-screens-labs/issues/249.

I merged rest of the PRs related to badge to this PR:

  • text color
  • label visibility
  • handling text

It currently works in a way that if it can parse it as int, it will be a number and displayed as string otherwise. Developer can use just label visibility to show "badge dot". At the same time I got rid of the number warning. It could be potentially changed to have separate methods for string and number, but for now I conformed to iOS API.

Screenshots / GIFs

image

Test code and steps to reproduce

See TestBottomTabs/index.tsx

Checklist

  • Included code example that can be used to test this change
  • Ensured that CI passes

Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

This looks good overall. I have one request regarding value sanitisation.

Comment on lines 384 to 387
val badgeValue = tabScreen.badgeValue?.toIntOrNull()
if (tabScreen.badgeValue != null && badgeValue == null) {
Log.e(TAG, "[RNScreens] Android supports only numbers as badge value")
}
Copy link
Member

Choose a reason for hiding this comment

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

I think that sanitization code should be on setter level, or even on view manager level. View should hold only the valid state (of Int type)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -34,7 +34,7 @@ const TAB_CONFIGS: TabConfiguration[] = [
{
tabScreenProps: {
tabKey: 'Tab2',
badgeValue: '2',
badgeValue: 'SWM',
Copy link
Member

Choose a reason for hiding this comment

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

Won't this crash?

Copy link
Contributor Author

@maciekstosio maciekstosio Jul 18, 2025

Choose a reason for hiding this comment

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

It won't if it fail to parse as a string we disable the badge, but this behaviour will be changed in:
#3025
and
#3024

Base automatically changed from @maciekstosio/Support-activity-indicator-color- to main July 18, 2025 08:11
@maciekstosio maciekstosio requested a review from kkafar July 18, 2025 08:21
… bottom navigation (#3023)

This PR allows to set badge text color. I suppressed the lint, as I
believe we want to use original color variables from material design in
cases like this.

## Screenshots / GIFs

<img width="439" height="907" alt="image"
src="https://github.com/user-attachments/assets/83e62206-b792-4fbf-962d-d72d935ee553"
/>

## Test code and steps to reproduce

See `TestBottomTabs/index.tsx`

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Ensured that CI passes
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Good job overall. I've found a bug, see below. Also I have concerns about necessity of tabbaritembadgevisible prop.

view.tabBarItemBadgeVisible = value
}

@ReactProp(name = "tabBarItemBadgeTextColor")
Copy link
Member

Choose a reason for hiding this comment

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

All the color prop need the `customType = "Color" annotation argument, otherwise it wong work on old architecture.

Suggested change
@ReactProp(name = "tabBarItemBadgeTextColor")
@ReactProp(name = "tabBarItemBadgeTextColor", customType = "Color")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed in 69df8cf

Comment on lines 393 to 406
val badgeValue = tabScreen.badgeValue
val badgeValueNumber = badgeValue?.toIntOrNull()

val badge = bottomNavigationView.getOrCreateBadge(menuItemIndex)
badge.isVisible = true

if (badgeValueNumber != null) {
badge.number = badgeValueNumber
} else if (badgeValue != null) {
badge.text = badgeValue
} else {
badge.clearNumber()
badge.clearText()
}
Copy link
Member

Choose a reason for hiding this comment

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

Okay, so badgeValue is either a string or a number. If it is a number I guess we get this nice shortening mechanism (picture below 👇🏻), is that right?

We need to document this behaviour. Please let's start creating short documentation around exposed types (similar to what we have in v4 implementation). We need to have such interactions described. It can be done in separate PR, I don't mind here.

image

Comment on lines 393 to 406
val badgeValue = tabScreen.badgeValue
val badgeValueNumber = badgeValue?.toIntOrNull()

val badge = bottomNavigationView.getOrCreateBadge(menuItemIndex)
badge.isVisible = true

if (badgeValueNumber != null) {
badge.number = badgeValueNumber
} else if (badgeValue != null) {
badge.text = badgeValue
} else {
badge.clearNumber()
badge.clearText()
}
Copy link
Member

Choose a reason for hiding this comment

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

There is a bug here. When I set a number as a badge value first, then change it to text, I can not go back to a number. See the recording.

Screen.Recording.2025-07-18.at.14.09.17.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 69df8cf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fix is basically that we clear first - there are separate properties for text and number, text takes precedence over number, thus it didn't change back - it wasn't cleared when setting number.

@@ -75,6 +75,8 @@ export interface BottomTabsScreenProps {

// Android specific
iconResourceName?: string;
tabBarItemBadgeVisible?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

Let's document what is the default value here. Also I wonder whether we need this prop? Shouldn't it be enough to determine badge visibility by the value passed as badgevalue? e.g. empty string or undefined means its not visible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can do:

  • "" -> dot icon
  • undefined -> disabled
  • string -> string
  • string parsable to string -> number
    But isn't it more tricky?

Copy link
Member

Choose a reason for hiding this comment

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

Now to show a dot you pass badgeValue: undefined & badgeVisible: true OR badgeValue: "" & badgeVisible: true?

Therefore to get a dot, user still must consult the documentation. Moreover "" as a value to get a dot only seems natural to me, since it's a badge w/o text 😅

I think it should work as follows:

  1. We should have a single prop, no badgeVisible
  2. badge is not visible when badgeValue == null
  3. badgeValue === undefined by default
  4. badgeValue === "" results in dot only
  5. any other badge value gets current treatment.

Additional logic, visible / invisible etc. can be defined by downstream libraries. Let's keep our API minimal here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed in 69df8cf

Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Answered your question

@@ -75,6 +75,8 @@ export interface BottomTabsScreenProps {

// Android specific
iconResourceName?: string;
tabBarItemBadgeVisible?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

Now to show a dot you pass badgeValue: undefined & badgeVisible: true OR badgeValue: "" & badgeVisible: true?

Therefore to get a dot, user still must consult the documentation. Moreover "" as a value to get a dot only seems natural to me, since it's a badge w/o text 😅

I think it should work as follows:

  1. We should have a single prop, no badgeVisible
  2. badge is not visible when badgeValue == null
  3. badgeValue === undefined by default
  4. badgeValue === "" results in dot only
  5. any other badge value gets current treatment.

Additional logic, visible / invisible etc. can be defined by downstream libraries. Let's keep our API minimal here.

@@ -142,24 +144,18 @@ class TabScreenViewManager :
}

override fun setBadgeValue(
Copy link
Member

Choose a reason for hiding this comment

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

I don't like that every other prop has tabBarItem... prefix and this one does not. But this is outside of the scope of this PR. I'll create ticket for this.

@maciekstosio maciekstosio requested a review from kkafar July 18, 2025 14:21
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@maciekstosio maciekstosio merged commit 767d007 into main Jul 18, 2025
5 checks passed
@maciekstosio maciekstosio deleted the @maciekstosio/support-badge-in-bottom-navigation branch July 18, 2025 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants