[go_router] [shell_route] Fix BottomNavigationBar might lost current index in long URIs. #2636
Conversation
… index in long URIs.
I used shell route example in my own app and liked it, thanks to flutter engineers and contributors. I noticed that when I navigate to a sub route of a specific BottomNavigationBarItem route e.g "/c/details/, the _calculateSelectedIndex returns the default index -> 0. That's because in if statement we use location == "BottomNavigationBarItem Location", which returns false in all cases if the location = /c/details.
I did change location == "location" to location.startsWith("location"), so the _calculateSelectedIndex determine the selected index based on the first route of the location not the exact one.
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@omar-hanafy it looks like you need to update the pubspec and CHANGELOG, even though this is only changing a sample |
|
@johnpryan I updated the pubspec and CHANGELOG if there is anything left I did not do just let me know. |
chunhtai
left a comment
There was a problem hiding this comment.
I didn't see the changelog and version bump in the diff.
…ple test (shell_route_test.dart) added in example/test to make sure every route including c/details can display the right content.
I added a change log and diff before, but when i tried to merge there was a conflict and i did not accept my changes. |
|
yes, please do so |
I used the shell route example in my app. I noticed that when I navigate to a sub route of a specific BottomNavigationBarItem route e.g "/c/details/, the _calculateSelectedIndex returns the default index -> 0. That's because in if statement we use location == "BottomNavigationBarItem Location", which returns false in all cases if the location = /c/details. I did change location == "location" to location.startsWith("location"), so the _calculateSelectedIndex determine the selected index based on the first route of the location not the exact one.
before:
after:
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].CHANGELOG.mdto add a description of the change, [following repository CHANGELOG style].///).