fix: BottomNavigation warn when shifting with the wrong number of tabs #3408
Conversation
|
Hey @lukewalczak is been a while since my last contribution 😄. I wasn't sure if I should point this PR to the branch main or 4.0 |
| @@ -352,6 +352,14 @@ const BottomNavigation = ({ | |||
| }: Props) => { | |||
There was a problem hiding this comment.
I'm wondering if we can change shifting = navigationState.routes.length > 3 to shifting = navigationState.routes.length >= 2
Would that be considered a breaking change?
There was a problem hiding this comment.
Would that be considered a breaking change?
Partially it's because it affects users with two tabs, who didn't want shiftingstyle to be applied. Why would you like to change?
There was a problem hiding this comment.
Just thought it would make more sense since the interpolation animation expects at least 2 items. So having 2 tabs would be perfectly fine to enable the animation.
I'm totally ok keeping it as it is 😃
brunohkbx
force-pushed
the
fix/throw-warning-in-bottom-navigation
branch
from
300d110
to
4d28ea4
Compare
|
The mobile version of example app from this branch is ready! You can see it here |
1 similar comment
|
The mobile version of example app from this branch is ready! You can see it here |
|
Hey @brunohkbx, thanks for your contribution!
Generally, please target the |
|
Hey @brunohkbx, you have a lot of conflicts after changing the target branch. I think the easiest way will be to push your changes again on that branch however having Please let me know if you need some assist on that ✌🏽 |
brunohkbx
force-pushed
the
fix/throw-warning-in-bottom-navigation
branch
from
4d28ea4
to
b4eca7b
Compare
|
Hey @brunohkbx, thank you for your pull request 🤗. The documentation from this branch can be viewed here. |
|
The mobile version of example app from this branch is ready! You can see it here |
|
@lukewalczak Fixed it, sorry I couldn't get back to it earlier. While rebasing this PR I found another issue and opened #3415 to address it. |
|
@brunohkbx no problem, I will check another PR as well. Please correct the tests since CI is failing on that check. |
brunohkbx
force-pushed
the
fix/throw-warning-in-bottom-navigation
branch
from
b4eca7b
to
09a597a
Compare
|
The mobile version of example app from this branch is ready! You can see it here |
Summary
Recently when working on a new feature I realized that the component BottomNavigation crashes when the prop
shiftingis true but the number of tabs is < 2 as in the following screenshotTest plan
shiftingto the BottomNavigation component