Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 70 additions & 14 deletions src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,13 @@ internal static Task SetupMenuItem(
int currentIndex,
BottomNavigationView bottomView,
IMauiContext mauiContext,
out IMenuItem menuItem)
out IMenuItem menuItem,
Action<IMenuItem> onIconLoaded = null)
{
Task returnValue;
using var title = new Java.Lang.String(item.title);
menuItem = menu.Add(0, index, 0, title);
returnValue = SetMenuItemIcon(menuItem, item.icon, mauiContext);
returnValue = SetMenuItemIcon(menuItem, item.icon, mauiContext, onIconLoaded);
UpdateEnabled(item.tabEnabled, menuItem);
if (index == currentIndex)
{
Expand All @@ -90,7 +91,8 @@ internal static async void SetupMenu(
List<(string title, ImageSource icon, bool tabEnabled)> items,
int currentIndex,
BottomNavigationView bottomView,
IMauiContext mauiContext)
IMauiContext mauiContext,
Action<IMenuItem> onIconLoaded = null)
{
maxBottomItems = Math.Min(maxBottomItems, MaxBottomNavigationItems);
Context context = mauiContext.Context;
Expand All @@ -112,43 +114,75 @@ internal static async void SetupMenu(

IMenuItem menuItem;
if (i >= menu.Size())
loadTasks.Add(SetupMenuItem(item, menu, i, currentIndex, bottomView, mauiContext, out menuItem));
loadTasks.Add(SetupMenuItem(item, menu, i, currentIndex, bottomView, mauiContext, out menuItem, onIconLoaded));
else
{
menuItem = menu.GetItem(i);
if (menuItem.ItemId != i)
{
menu.RemoveItem(menuItem.ItemId);
loadTasks.Add(SetupMenuItem(item, menu, i, currentIndex, bottomView, mauiContext, out menuItem));
loadTasks.Add(SetupMenuItem(item, menu, i, currentIndex, bottomView, mauiContext, out menuItem, onIconLoaded));
}
else
{
SetMenuItemTitle(menuItem, item.title);
loadTasks.Add(SetMenuItemIcon(menuItem, item.icon, mauiContext));
loadTasks.Add(SetMenuItemIcon(menuItem, item.icon, mauiContext, onIconLoaded));
Comment thread
Vignesh-SF3580 marked this conversation as resolved.
// Reapply enabled/selected state since this IMenuItem is being reused, not recreated.
UpdateEnabled(item.tabEnabled, menuItem);
if (i == currentIndex)
Comment thread
Vignesh-SF3580 marked this conversation as resolved.
{
menuItem.SetChecked(true);
bottomView.SelectedItemId = i;
Comment thread
Vignesh-SF3580 marked this conversation as resolved.
}
Comment thread
Vignesh-SF3580 marked this conversation as resolved.
}
}

menuItems.Add(menuItem);
}

var menuSize = menu.Size();
IMenuItem moreMenuItem = null;
if (showMore && menu.GetItem(menuSize - 1).ItemId != MoreTabId)
{
var moreString = context.Resources.GetText(Resource.String.overflow_tab_title);
if (menuSize == maxBottomItems)
menu.RemoveItem(menu.GetItem(menuSize - 1).ItemId);
var menuItem = menu.Add(0, MoreTabId, 0, moreString);
menuItems.Add(menuItem);
moreMenuItem = menu.Add(0, MoreTabId, 0, moreString);
menuItems.Add(moreMenuItem);

menuItem.SetIcon(Resource.Drawable.abc_ic_menu_overflow_material);
if (currentIndex >= maxBottomItems - 1)
menuItem.SetChecked(true);
moreMenuItem.SetIcon(Resource.Drawable.abc_ic_menu_overflow_material);
}
else if (showMore)
{
// The More item already exists (reused, not recreated) — still need to
// reapply its selected state below in case currentIndex has changed.
moreMenuItem = menu.GetItem(menuSize - 1);
}

if (moreMenuItem is not null && currentIndex >= maxBottomItems - 1)
{
// Use SetChecked only — setting SelectedItemId would trigger ShowMoreBottomSheet().
moreMenuItem.SetChecked(true);
}

bottomView.SetShiftMode(false, false);

if (loadTasks.Count > 0)
await Task.WhenAll(loadTasks);
{
try
{
await Task.WhenAll(loadTasks);
}
catch (Exception ex)
{
// SetupMenu is async void — an unhandled exception here would crash
// the app. SetMenuItemIcon itself no longer swallows exceptions so
// its other caller (ShellItemRenderer.UpdateShellSectionIcon, via
// FireAndForget) can still observe/log a faulted Task; this catch
// only protects SetupMenu's own async-void boundary.
System.Diagnostics.Debug.WriteLine($"SetupMenu: one or more icon loads failed: {ex}");
}
}
}

internal static void SetMenuItemTitle(IMenuItem menuItem, string title)
Expand All @@ -157,14 +191,31 @@ internal static void SetMenuItemTitle(IMenuItem menuItem, string title)
menuItem.SetTitle(jTitle);
}

internal static async Task SetMenuItemIcon(IMenuItem menuItem, ImageSource source, IMauiContext context)
// Records which ImageSource each reused IMenuItem is currently supposed to show, so a
// slower, stale load (from before the item was repurposed for a different tab) can
// detect it's been superseded and skip applying its now-outdated result.
static readonly ConditionalWeakTable<IMenuItem, ImageSource> s_pendingIconSource = new();

internal static async Task SetMenuItemIcon(IMenuItem menuItem, ImageSource source, IMauiContext context, Action<IMenuItem> onIconLoaded = null)
{
if (!menuItem.IsAlive())
return;

s_pendingIconSource.AddOrUpdate(menuItem, source);
Comment thread
Vignesh-SF3580 marked this conversation as resolved.

if (source is null)
{
// Clear any stale icon left on this (possibly reused) menu item.
menuItem.SetIcon(null);
onIconLoaded?.Invoke(menuItem);
return;
}

// Exceptions are intentionally allowed to propagate here (not swallowed) so
// callers can observe/log failures via the returned Task — e.g. the legacy
// ShellItemRenderer.UpdateShellSectionIcon relies on FireAndForget's error
// handler seeing a faulted Task. SetupMenu (the other caller) guards its own
// async-void boundary separately when awaiting these tasks.
var services = context.Services;
var provider = services.GetRequiredService<IImageSourceServiceProvider>();
var imageSourceService = provider.GetRequiredImageSourceService(source);
Expand All @@ -173,9 +224,14 @@ internal static async Task SetMenuItemIcon(IMenuItem menuItem, ImageSource sourc
source,
context.Context);

if (menuItem.IsAlive())
// Skip applying if this menu item has since been repurposed for a different
// source (i.e. a newer SetMenuItemIcon call updated the pending source above).
if (menuItem.IsAlive() && s_pendingIconSource.TryGetValue(menuItem, out var pending) && ReferenceEquals(pending, source))
{
menuItem.SetIcon(result?.Value);
// Let the caller reapply per-item icon tint (e.g. to preserve a
// FontImageSource's own Color) now that the drawable is installed.
onIconLoaded?.Invoke(menuItem);
}
}

Expand Down
79 changes: 29 additions & 50 deletions src/Controls/src/Core/Platform/Android/TabbedViewManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,9 @@ internal void UpdateSwipePaging()
var tab = Element.Tabs[i];
// ITab.Icon is IImageSource; convert to ImageSource if possible for BottomNavigationViewUtils
var icon = tab.Icon as ImageSource;
items.Add((tab.Title, icon, tab.IsEnabled));
// Fallback for null/whitespace Title to avoid a blank bottom tab label.
var title = !string.IsNullOrWhiteSpace(tab.Title) ? tab.Title : $"Tab {i + 1}";
items.Add((title, icon, tab.IsEnabled));
}

return items;
Expand All @@ -758,11 +760,13 @@ internal virtual void SetupBottomNavigationView()
}

var menu = _bottomNavigationView.Menu;
menu.Clear();

var tabs = Element.Tabs;
if (tabs is null || tabs.Count == 0)
{
menu.Clear();
_bottomNavigationView.SetOnItemSelectedListener(null);
_bottomNavigationView.SetOnItemReselectedListener(null);
return;
}

Expand All @@ -776,43 +780,21 @@ internal virtual void SetupBottomNavigationView()

_bottomNavigationView.Visibility = ViewStates.Visible;

int maxItems = Math.Min(_bottomNavigationView.MaxItemCount, BottomNavigationViewUtils.MaxBottomNavigationItems);
bool showMore = tabs.Count > maxItems;
int end = showMore ? maxItems - 1 : tabs.Count;

for (int i = 0; i < end; i++)
{
var tab = tabs[i];
var title = !string.IsNullOrWhiteSpace(tab.Title) ? tab.Title : $"Tab {i + 1}";
var menuItem = menu.Add(0, i, i, title);

if (menuItem is null)
{
continue;
}

if (!tab.IsEnabled)
{
menuItem.SetEnabled(false);
}

LoadBottomNavIconAsync(menuItem, tab);
}

// Add "More" overflow item if needed
if (showMore)
{
var moreItem = menu.Add(0, BottomNavigationViewUtils.MoreTabId, maxItems - 1, "More");
moreItem?.SetIcon(Resource.Drawable.abc_ic_menu_overflow_material);
}
// Use BottomNavigationViewUtils.SetupMenu which does incremental in-place updates
// (reuses existing IMenuItem objects at unchanged positions, preserving identity).
// This matches the renderer's ShellItemRenderer.SetupMenu behavior and avoids
// menu.Clear() which destroys all IMenuItem references on every tab add/remove.
var items = CreateTabList();
Comment thread
Vignesh-SF3580 marked this conversation as resolved.

// Set initial selection using pre-computed index (avoids wrapper comparison issues)
var currentIndex = Element.CurrentTabIndex;
if (currentIndex >= 0 && currentIndex < tabs.Count)
{
int targetId = currentIndex >= end ? BottomNavigationViewUtils.MoreTabId : currentIndex;
_bottomNavigationView.SelectedItemId = targetId;
}
BottomNavigationViewUtils.SetupMenu(
Comment thread
Vignesh-SF3580 marked this conversation as resolved.
Comment thread
Vignesh-SF3580 marked this conversation as resolved.
Comment thread
Vignesh-SF3580 marked this conversation as resolved.
Comment thread
Vignesh-SF3580 marked this conversation as resolved.
menu,
_bottomNavigationView.MaxItemCount,
items,
currentIndex,
_bottomNavigationView,
_context,
onIconLoaded: menuItem => SetupBottomNavigationViewIconColor(menuItem.ItemId, menuItem));

_bottomNavigationView.SetShiftMode(false, false);

Expand All @@ -832,23 +814,20 @@ async void LoadBottomNavIconAsync(IMenuItem menuItem, ITab tab)
{
try
{
if (tab.Icon is not ImageSource icon)
{
return;
}

var result = await icon.GetPlatformImageAsync(_context);
if (result?.Value is not null && menuItem.IsAlive())
{
menuItem.SetIcon(result.Value);

// Route through BottomNavigationViewUtils.SetMenuItemIcon instead of loading
// the icon directly here. SetMenuItemIcon guards against a reused IMenuItem
// being repurposed (by SetupMenu) for a different tab while this load is still
// in flight -- without that guard, this call could win a race against a newer
// SetupMenu-driven icon load and overwrite the wrong tab's icon.
await BottomNavigationViewUtils.SetMenuItemIcon(
menuItem,
tab.Icon as ImageSource,
_context,
// Apply per-item icon tint after loading so that:
// 1. FontImageSource with explicit Color shows its own color (tint cleared)
// 2. FontImageSource without Color gets SelectedTabColor/UnselectedTabColor tint
// Without this, the global ItemIconTintList overrides baked-in colors.
var tabIndex = menuItem.ItemId;
SetupBottomNavigationViewIconColor(tabIndex, menuItem);
}
onIconLoaded: loadedMenuItem => SetupBottomNavigationViewIconColor(loadedMenuItem.ItemId, loadedMenuItem));
}
catch (Exception ex)
{
Comment on lines 832 to 833
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public bool IsNavigationBarVisible(IMauiContext mauiContext)
.LayoutParameters?.Height > 0;
}

protected bool IsBackButtonVisible(IElementHandler handler)
protected virtual bool IsBackButtonVisible(IElementHandler handler)
{
if (GetPlatformToolbar(handler)?.NavigationIcon is DrawerArrowDrawable dad)
return dad.Progress == 1;
Expand Down
3 changes: 2 additions & 1 deletion src/Controls/tests/DeviceTests/Elements/Modal/ModalTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ namespace Microsoft.Maui.DeviceTests
#if ANDROID || IOS || MACCATALYST
[Collection(ControlsHandlerTestBase.RunInNewWindowCollection)]
#endif
[Trait(RendererHandlerVariant.TraitName, RendererHandlerVariant.AndroidShellRenderer)] // See RendererHandlerVariant.cs
public partial class ModalTests : ControlsHandlerTestBase
{
void SetupBuilder()
protected virtual void SetupBuilder()
{
EnsureHandlerCreated(builder =>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace Microsoft.Maui.DeviceTests
{
/// <summary>
/// Trait key/values for Android Shell Renderer/Handler test variants so that, on Android only,
/// xUnitCustomizations.cs can prefix their DisplayName as "[Renderer]"/"[Handler]".
/// Also reused by Modal/Window tests, which share the same Android Renderer/Handler reuse logic as Shell.
/// </summary>
public static class RendererHandlerVariant
{
public const string TraitName = "Variant";
public const string AndroidShellRenderer = "Renderer";
public const string AndroidShellHandler = "Handler";
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Threading.Tasks;
using AndroidX.DrawerLayout.Widget;
using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Platform.Compatibility;
using Xunit;

namespace Microsoft.Maui.DeviceTests
Expand All @@ -15,18 +16,21 @@ await RunShellTest(shell =>
{
shell.FlyoutContent = new VerticalStackLayout() { new Label() { Text = "Flyout Content" } };
},
async (shell, handler) =>
async shell =>
{
#if ANDROID || IOS || MACCATALYST
var shellContext = (IShellContext)shell.Handler;
#endif
// 1. Set FlyoutIsPresented=true to make the Shell Flyout visible.
shell.FlyoutIsPresented = true;

var dl = GetDrawerLayout(handler) as DrawerLayout;
var dl = GetDrawerLayout(shellContext) as DrawerLayout;
Assert.NotNull(dl);

await AssertionExtensions.AssertEventually(() =>
{
// 2. Check that the Flyout has size.
var flyoutFrame = GetFlyoutFrame(handler);
var flyoutFrame = GetFlyoutFrame(shellContext);
return flyoutFrame.Width > 0 && flyoutFrame.Height > 0 && dl.IsOpen;
});
});
Expand Down
Loading
Loading