From efa95f75f14c710b493ed541a4e2ae7147279612 Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Thu, 23 May 2024 01:55:52 +0200 Subject: [PATCH 1/8] Fix #18251 --- .../src/Core/Platform/Android/BottomNavigationViewUtils.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs b/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs index ad2b610edc2e..5d9304ab3855 100644 --- a/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs +++ b/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs @@ -73,10 +73,7 @@ internal static async void SetupMenu( { Context context = mauiContext.Context; - while (items.Count < menu.Size()) - { - menu.RemoveItem(menu.GetItem(menu.Size() - 1).ItemId); - } + menu.Clear(); int numberOfMenuItems = items.Count; bool showMore = numberOfMenuItems > maxBottomItems; From 3047f66059ef6457668544b25e21369d1c865661 Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Thu, 23 May 2024 01:56:19 +0200 Subject: [PATCH 2/8] Added a UI Test (#18251) --- .../Tests/Issues/Issue18251.cs | 26 ++++++ .../tests/TestCases/Issues/Issue18251.xaml | 30 +++++++ .../tests/TestCases/Issues/Issue18251.xaml.cs | 87 +++++++++++++++++++ 3 files changed, 143 insertions(+) create mode 100644 src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs create mode 100644 src/Controls/tests/TestCases/Issues/Issue18251.xaml create mode 100644 src/Controls/tests/TestCases/Issues/Issue18251.xaml.cs diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs new file mode 100644 index 000000000000..ffb0a4687577 --- /dev/null +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs @@ -0,0 +1,26 @@ +using NUnit.Framework; +using UITest.Appium; +using UITest.Core; + +namespace Microsoft.Maui.TestCases.Tests.Issues +{ + public class Issue18251 : _IssuesUITest + { + public Issue18251(TestDevice device) : base(device) { } + + public override string Issue => "IllegalArgumentException when changing number of tabbar pages"; + + [Test] + public void NoExceptionShouldBeThrown() + { + _ = App.WaitForElement("GoToTest"); + App.Click("GoToTest"); + + _ = App.WaitForElement("button"); + App.Click("button"); + App.Click("button"); + + // The test passes if no crash is observed + } + } +} \ No newline at end of file diff --git a/src/Controls/tests/TestCases/Issues/Issue18251.xaml b/src/Controls/tests/TestCases/Issues/Issue18251.xaml new file mode 100644 index 000000000000..5df084613b1f --- /dev/null +++ b/src/Controls/tests/TestCases/Issues/Issue18251.xaml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/Controls/tests/TestCases/Issues/Issue18251.xaml.cs b/src/Controls/tests/TestCases/Issues/Issue18251.xaml.cs new file mode 100644 index 000000000000..a256183371e0 --- /dev/null +++ b/src/Controls/tests/TestCases/Issues/Issue18251.xaml.cs @@ -0,0 +1,87 @@ +using Microsoft.Maui.Controls; + +namespace Maui.Controls.Sample.Issues +{ + [Issue(IssueTracker.Github, 18251, "IllegalArgumentException when changing number of tabbar pages", PlatformAffected.Android)] + public class Issue18251Test : ContentPage + { + public Issue18251Test() + { + Content = new VerticalStackLayout() + { + Children = + { + new Button() + { + Text = "Go To Test", + AutomationId = "GoToTest", + Command = new Command(() => Application.Current.MainPage = new Issue18251()) + } + } + }; + } + } + + public partial class Issue18251 : Shell + { + internal static Issue18251ViewModel ViewModel; + + public Issue18251() + { + InitializeComponent(); + BindingContext = ViewModel = new(); + } + } + + public class Issue18251Page : ContentPage + { + public Issue18251Page() + { + Content = + new VerticalStackLayout() { + new Button() + { + AutomationId = "button", + Command = Issue18251.ViewModel.LoginCommand, + Text = "Click to change tabs" + } + }; + } + } + + public class Issue18251ViewModel : BindableObject + { + private bool _isLoggedIn; + public bool IsLoggedIn + { + get => _isLoggedIn; + set + { + _isLoggedIn = value; + OnPropertyChanged(); + } + } + + private bool _isLoggedOut = true; + public bool IsLoggedOut + { + get => _isLoggedOut; + set + { + _isLoggedOut = value; + OnPropertyChanged(); + } + } + + public Command LoginCommand { get; set; } + + public Issue18251ViewModel() + { + LoginCommand = new Command(() => + { + IsLoggedIn = !IsLoggedIn; + IsLoggedOut = !IsLoggedIn; + }); + } + } +} \ No newline at end of file From 8aeb5f20c5d03a52d4bc0dee6a348bab35c69c09 Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Thu, 23 May 2024 11:16:46 +0200 Subject: [PATCH 3/8] Update BottomNavigationViewUtils.cs --- .../src/Core/Platform/Android/BottomNavigationViewUtils.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs b/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs index 5d9304ab3855..e73b6adc300d 100644 --- a/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs +++ b/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs @@ -73,7 +73,10 @@ internal static async void SetupMenu( { Context context = mauiContext.Context; - menu.Clear(); + while (items.Count < menu.Size()) + { + menu.RemoveItem(menu.GetItem(menu.Size() - 1).ItemId); + } int numberOfMenuItems = items.Count; bool showMore = numberOfMenuItems > maxBottomItems; @@ -101,6 +104,8 @@ internal static async void SetupMenu( if (showMore) { var moreString = context.Resources.GetText(Resource.String.overflow_tab_title); + if (menu.Size() == maxBottomItems) + menu.RemoveItem(menu.GetItem(menu.Size() - 1).ItemId); var menuItem = menu.Add(0, MoreTabId, 0, moreString); menuItems.Add(menuItem); From 139e708d9eb6c9046c22a3b8a2f93b2845f31658 Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Thu, 23 May 2024 17:37:35 +0200 Subject: [PATCH 4/8] Update BottomNavigationViewUtils.cs --- .../Android/BottomNavigationViewUtils.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs b/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs index e73b6adc300d..64a2a21f6104 100644 --- a/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs +++ b/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs @@ -94,8 +94,16 @@ internal static async void SetupMenu( else { menuItem = menu.GetItem(i); - SetMenuItemTitle(menuItem, item.title); - loadTasks.Add(SetMenuItemIcon(menuItem, item.icon, mauiContext)); + if (menuItem.ItemId == MoreTabId) + { + menu.RemoveItem(MoreTabId); + loadTasks.Add(SetupMenuItem(item, menu, i, currentIndex, bottomView, mauiContext, out menuItem)); + } + else + { + SetMenuItemTitle(menuItem, item.title); + loadTasks.Add(SetMenuItemIcon(menuItem, item.icon, mauiContext)); + } } menuItems.Add(menuItem); @@ -103,9 +111,10 @@ internal static async void SetupMenu( if (showMore) { + var menuSize = menu.Size(); var moreString = context.Resources.GetText(Resource.String.overflow_tab_title); - if (menu.Size() == maxBottomItems) - menu.RemoveItem(menu.GetItem(menu.Size() - 1).ItemId); + if (menuSize == maxBottomItems) + menu.RemoveItem(menu.GetItem(menuSize - 1).ItemId); var menuItem = menu.Add(0, MoreTabId, 0, moreString); menuItems.Add(menuItem); From 76c480b9dc467d8cac0244136926099a0c3eec0f Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Thu, 23 May 2024 20:28:25 +0200 Subject: [PATCH 5/8] Update BottomNavigationViewUtils.cs --- .../src/Core/Platform/Android/BottomNavigationViewUtils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs b/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs index 64a2a21f6104..fcee80c0f14e 100644 --- a/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs +++ b/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs @@ -94,7 +94,7 @@ internal static async void SetupMenu( else { menuItem = menu.GetItem(i); - if (menuItem.ItemId == MoreTabId) + if (menuItem.ItemId != i) { menu.RemoveItem(MoreTabId); loadTasks.Add(SetupMenuItem(item, menu, i, currentIndex, bottomView, mauiContext, out menuItem)); From cdadefc3b996ab4ca0b83a078d460ae7916bda3a Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Fri, 24 May 2024 00:38:23 +0200 Subject: [PATCH 6/8] Update BottomNavigationViewUtils.cs --- .../src/Core/Platform/Android/BottomNavigationViewUtils.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs b/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs index fcee80c0f14e..60de64e5f181 100644 --- a/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs +++ b/src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs @@ -96,7 +96,7 @@ internal static async void SetupMenu( menuItem = menu.GetItem(i); if (menuItem.ItemId != i) { - menu.RemoveItem(MoreTabId); + menu.RemoveItem(menuItem.ItemId); loadTasks.Add(SetupMenuItem(item, menu, i, currentIndex, bottomView, mauiContext, out menuItem)); } else @@ -109,9 +109,9 @@ internal static async void SetupMenu( menuItems.Add(menuItem); } - if (showMore) + var menuSize = menu.Size(); + if (showMore && menu.GetItem(menuSize - 1).ItemId != MoreTabId) { - var menuSize = menu.Size(); var moreString = context.Resources.GetText(Resource.String.overflow_tab_title); if (menuSize == maxBottomItems) menu.RemoveItem(menu.GetItem(menuSize - 1).ItemId); From deb4e77950b78b133d3fadc0e01f76826f6101a8 Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Sat, 25 May 2024 18:14:07 +0200 Subject: [PATCH 7/8] Ui Test improvement --- .../Tests/Issues/Issue18251.cs | 3 --- .../tests/TestCases/Issues/Issue18251.xaml.cs | 19 ------------------- 2 files changed, 22 deletions(-) diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs index ffb0a4687577..68988bd27e54 100644 --- a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs @@ -13,9 +13,6 @@ public Issue18251(TestDevice device) : base(device) { } [Test] public void NoExceptionShouldBeThrown() { - _ = App.WaitForElement("GoToTest"); - App.Click("GoToTest"); - _ = App.WaitForElement("button"); App.Click("button"); App.Click("button"); diff --git a/src/Controls/tests/TestCases/Issues/Issue18251.xaml.cs b/src/Controls/tests/TestCases/Issues/Issue18251.xaml.cs index a256183371e0..3f4b5e61cc9e 100644 --- a/src/Controls/tests/TestCases/Issues/Issue18251.xaml.cs +++ b/src/Controls/tests/TestCases/Issues/Issue18251.xaml.cs @@ -3,25 +3,6 @@ namespace Maui.Controls.Sample.Issues { [Issue(IssueTracker.Github, 18251, "IllegalArgumentException when changing number of tabbar pages", PlatformAffected.Android)] - public class Issue18251Test : ContentPage - { - public Issue18251Test() - { - Content = new VerticalStackLayout() - { - Children = - { - new Button() - { - Text = "Go To Test", - AutomationId = "GoToTest", - Command = new Command(() => Application.Current.MainPage = new Issue18251()) - } - } - }; - } - } - public partial class Issue18251 : Shell { internal static Issue18251ViewModel ViewModel; From 1dafc183e043d646cdfc593dc6b36504b235d860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Tue, 28 May 2024 13:46:32 +0200 Subject: [PATCH 8/8] Updated test --- .../TestCases.Shared.Tests/Tests/Issues/Issue18251.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs index 68988bd27e54..63f61d50d8a9 100644 --- a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue18251.cs @@ -11,11 +11,14 @@ public Issue18251(TestDevice device) : base(device) { } public override string Issue => "IllegalArgumentException when changing number of tabbar pages"; [Test] - public void NoExceptionShouldBeThrown() + public void NoExceptionShouldBeThrownAddingShellTabs() { - _ = App.WaitForElement("button"); - App.Click("button"); - App.Click("button"); + App.WaitForElement("button"); + + for (int i = 0; i < 2; i++) + { + App.Click("button"); + } // The test passes if no crash is observed }