IllegalArgumentException when changing number of tabbar pages - fix #22589
Conversation
|
Hey there @kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
/azp run |
This comment was marked as outdated.
This comment was marked as outdated.
| { | ||
| menu.RemoveItem(menu.GetItem(menu.Size() - 1).ItemId); | ||
| } | ||
| menu.Clear(); |
There was a problem hiding this comment.
Need to verify this change against
16a3972#diff-1787c6c4f33d04c0dd8bceddf563be45c136658b564e5a6be28161f37b79adc5R75-R80
Where I originally modified this.
Make sure the issue fixed by that PR doesn't resurface here
There was a problem hiding this comment.
I think the idea is that we don't want to completely clear the menu and then re-add because then it causes a shuffling to happen visually
So, the point of walking backwards was to just sync the list, instead of just clear and re-add
I think will still need the old code but a variation of it
There was a problem hiding this comment.
@PureWeen I've added a commit. Let me know what you think
|
Azure Pipelines successfully started running 3 pipeline(s). |
jfversluis
added
platform/android 🤖
area-controls-shell
| @@ -104,6 +104,8 @@ internal static void UpdateEnabled(bool tabEnabled, IMenuItem menuItem) | |||
| if (showMore) | |||
| { | |||
| var moreString = context.Resources.GetText(Resource.String.overflow_tab_title); | |||
| if (menu.Size() == maxBottomItems) | |||
There was a problem hiding this comment.
maybe we can grab menu.Size() only once here
| @@ -104,6 +104,8 @@ internal static void UpdateEnabled(bool tabEnabled, IMenuItem menuItem) | |||
| if (showMore) | |||
| { | |||
| var moreString = context.Resources.GetText(Resource.String.overflow_tab_title); | |||
| if (menu.Size() == maxBottomItems) | |||
| menu.RemoveItem(menu.GetItem(menu.Size() - 1).ItemId); | |||
There was a problem hiding this comment.
I feel like in this else check we need to see if we've hit the "more" item and then remove iit if we do
https://github.com/dotnet/maui/pull/22589/files#diff-1787c6c4f33d04c0dd8bceddf563be45c136658b564e5a6be28161f37b79adc5R94-R99
like, if the user has 6 items it'll be
0,1,2,3,more
if they go down to 5 items then I think the more tab won't swap in the correct 5th item
And then instead of "Removing" this item. Can we retrieve the item using the MoreTabId to see if it was still added?
There was a problem hiding this comment.
@PureWeen you're right
Screen.Recording.2024-05-23.at.16.59.36.mov
src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs
Outdated
Show resolved
Hide resolved
src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs
Outdated
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Another @kubaflo original, thank you so much for this! |
Issues Fixed
Fixes #18251
Screen.Recording.2024-05-23.at.01.08.51.mov
Screen.Recording.2024-05-23.at.01.07.26.mov