-
Notifications
You must be signed in to change notification settings - Fork 379
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
fix: Bottom Navigation Tab Reselected Bug #725
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM& changes work really fine.
Thanks for your contribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
When i click on the already active bottom navigation item then the fragment i think it is still changing..don't know whether it is the problem with my internet or system... @foongminwong , @HaripriyaB , @sakshivij can one of you test this, just to make sure.. |
Sorry for the late reply @faznan3nazer , let me check right now.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes made in this PR were tested locally. Following are the results:
-
Code review - Done
-
All possible responses were tested as below:
Expected Result: The Bottom Navigation Tab Reselected Bug exists.
Actual Result: Same as expected.Expected Result: The Bottom Navigation Tab Reselected is eliminated.
Actual Result: Same as expected. -
Additional testcases covered: N/A
-
Additional Comments:
- @faznan3nazer I am able to see the changes when tested locally. It also took me awhile to refresh the app since my laptop is slow too 😆
- Good job @DhaneshShetty !!
-
Status of PR Changed to: Ready to Merge.
-
Android Version: 9.0, Device: Android Emulator Pixel API 28
@isabelcosta feel free to approve and merge this pr when you have time, it's reviewed and tested (see above ☝) |
There was a problem hiding this 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 for this amazing contribution @DhaneshShetty 🎉
Thank you so much for testing this @foongminwong 👏 |
Description
After the fix, Fragment is not replaced on reselecting the current fragment. This is done by adding onNavigationItemReselectedListener.
Fixes #724
Type of Change:
Delete irrelevant options.
Code/Quality Assurance Only
How Has This Been Tested?
Tested Locally on Redmi 8A
Android 9
Checklist:
Delete irrelevant options.
Code/Quality Assurance Only