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
82 changes: 55 additions & 27 deletions src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Android.cs
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,15 @@ internal void SetupViewPagerAdapter()
/// </summary>
internal void SetupTabbedViewManager()
{
if (_viewPager is null || VirtualView is null || MauiContext is null)
if (_tabbedViewManager is not null || _viewPager is null || VirtualView is null || MauiContext is null)
{
return;
}

var shellSections = ((IShellItemController)VirtualView).GetItems();
var shellItemController = (IShellItemController)VirtualView;
var shellSections = shellItemController.GetItems();

if (shellSections is null || shellSections.Count == 0)
if (shellSections is null || shellSections.Count == 0 || !shellItemController.ShowTabs)
Comment thread
jonathanpeppers marked this conversation as resolved.
{
return;
}
Expand Down Expand Up @@ -217,6 +218,22 @@ internal void SetupTabbedViewManager()

// Get BNV reference for appearance tracker
_bottomNavigationView = _tabbedViewManager.BottomNavigationView;
RecreateBottomNavigationAppearanceTracker();
Comment thread
jonathanpeppers marked this conversation as resolved.

// Initial setup registers the appearance observer immediately afterward. When
// setup was deferred, replay state for the newly-created bottom tabs.
if (_registeredShell is not null && _displayedPage is not null)
{
UpdateAllBadges();
((IShellController)_registeredShell).AppearanceChanged(_displayedPage, false);
Comment thread
jonathanpeppers marked this conversation as resolved.
}
}

void RecreateBottomNavigationAppearanceTracker()
Comment thread
jonathanpeppers marked this conversation as resolved.
{
_appearanceTracker?.Dispose();
_shellContext ??= GetShellContext();
_appearanceTracker = _shellContext.CreateBottomNavViewAppearanceTracker(VirtualView);
}

/// <summary>
Expand All @@ -236,6 +253,15 @@ internal void RebuildBottomNavigation()

// Update BNV reference (SetElement creates a new BNV)
_bottomNavigationView = _tabbedViewManager.BottomNavigationView;
RecreateBottomNavigationAppearanceTracker();
}

void RemoveBottomNavigationInfrastructure()
{
_tabbedViewManager?.SetElement(null);
_tabbedViewManager = null;
_shellItemAdapter = null;
_bottomNavigationView = null;
}

/// <summary>
Expand Down Expand Up @@ -342,22 +368,26 @@ internal void SwitchToShellItem(ShellItem newItem)
// Rebuild ViewPager2 adapter for new ShellItem's sections
SetupViewPagerAdapter();

// Rebuild bottom navigation for new ShellItem's sections via TabbedViewManager
RebuildBottomNavigation();

// Apply badges to the rebuilt bottom navigation
UpdateAllBadges();

// Update tab visibility for new ShellItem (may need to show/hide bottom tabs)
var showTabs = ((IShellItemController)newItem).ShowTabs;

if (showTabs)
{
if (_tabbedViewManager is null)
{
SetupTabbedViewManager();
}
else
{
// Rebuild bottom navigation for new ShellItem's sections via TabbedViewManager
RebuildBottomNavigation();
}

UpdateAllBadges();
_tabbedViewManager?.SetTabLayout();
}
else
{
_tabbedViewManager?.RemoveTabs();
RemoveBottomNavigationInfrastructure();
Comment thread
jonathanpeppers marked this conversation as resolved.
}

// Re-register appearance observer with new ShellItem
Expand Down Expand Up @@ -616,7 +646,7 @@ void UpdateDisplayedPage(Page page)

void UpdateTabBarVisibility()
{
if (_tabbedViewManager is null || _displayedPage is null || ((ElementHandler)this).VirtualView is null)
if (_switchingShellItem || _displayedPage is null || ((ElementHandler)this).VirtualView is null)
{
return;
}
Expand All @@ -625,11 +655,12 @@ void UpdateTabBarVisibility()

if (showTabs)
{
_tabbedViewManager.SetTabLayout();
SetupTabbedViewManager();
_tabbedViewManager?.SetTabLayout();
}
else
{
_tabbedViewManager.RemoveTabs();
_tabbedViewManager?.RemoveTabs();
}
}

Expand Down Expand Up @@ -727,9 +758,9 @@ protected override void ConnectHandler(ViewPager2 platformView)
((IShellItemController)VirtualView).ItemsCollectionChanged += OnShellItemsChanged;
}

// Initialize shell context and appearance tracker early
// Initialize the Shell context early. The bottom navigation appearance tracker
// is created with the deferred TabbedViewManager when tabs are actually needed.
_shellContext ??= GetShellContext();
_appearanceTracker = _shellContext.CreateBottomNavViewAppearanceTracker(VirtualView);

// NOTE: Appearance observer registration is deferred to RegisterAppearanceObserver()
// called from OnViewCreated in the wrapper fragment. At ConnectHandler time,
Expand Down Expand Up @@ -776,11 +807,14 @@ void OnShellItemsChanged(object? sender, System.Collections.Specialized.NotifyCo
// 0→N transition: adapter/manager were not created during initial setup
// because there were no sections. Now that sections exist, create them.
SetupViewPagerAdapter();
SetupTabbedViewManager();
}

// Rebuild the bottom navigation menu for the updated sections via TabbedViewManager
_tabbedViewManager?.RefreshTabs();
var existingTabbedViewManager = _tabbedViewManager;
SetupTabbedViewManager();

// Rebuild an existing bottom navigation menu. A newly-created manager was
// already populated by SetElement in SetupTabbedViewManager.
existingTabbedViewManager?.RefreshTabs();
UpdateTabBarVisibility();

// Signal that the adapter was just rebuilt. The next SwitchToSection call
Expand Down Expand Up @@ -837,7 +871,7 @@ protected override void DisconnectHandler(ViewPager2 platformView)
_registeredShell = null;
}

// Dispose per-item appearance tracker (ConnectHandler recreates for new item)
// Dispose per-item appearance tracker; tab setup or rebuild recreates it as needed.
_appearanceTracker?.Dispose();
_appearanceTracker = null;

Expand All @@ -851,13 +885,7 @@ protected override void DisconnectHandler(ViewPager2 platformView)
if (!_preserveFragmentResources)
{
// Full disconnect: fragment is being destroyed — clean everything
if (_tabbedViewManager is not null)
{
_tabbedViewManager.RemoveTabs();
_tabbedViewManager.SetElement(null);
_tabbedViewManager = null;
}
_shellItemAdapter = null;
RemoveBottomNavigationInfrastructure();

_toolbarAppearanceTracker?.Dispose();
_toolbarAppearanceTracker = null;
Expand Down
84 changes: 51 additions & 33 deletions src/Controls/src/Core/Handlers/Shell/ShellSectionHandler.Android.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,21 +149,6 @@ protected override AView CreatePlatformElement()
_rootLayout = rootView.FindViewById<LinearLayout>(Resource.Id.shellsection_coordinator);
_viewPager = rootView.FindViewById<ViewPager2>(Resource.Id.shellsection_viewpager);

// Create TabLayout programmatically (no longer from XML layout).
// It will be placed into navigationlayout_toptabs via PlaceTopTabs().
var context = MauiContext?.Context
?? throw new InvalidOperationException("MauiContext.Context cannot be null");

int actionBarHeight = context.GetActionBarHeight();

_contentTabLayout = new TabLayout(context)
{
Id = AView.GenerateViewId(),
LayoutParameters = new LP(LP.MatchParent, actionBarHeight),
Visibility = ViewStates.Gone, // Hidden by default (shown when > 1 tab)
TabMode = TabLayout.ModeScrollable
};

return rootView;
}

Expand Down Expand Up @@ -253,18 +238,7 @@ internal void SetupViewPagerAdapter()
var visibleItems = SectionController.GetItems();
_viewPager.OffscreenPageLimit = Math.Max(visibleItems.Count, 1);

// Setup TabbedViewManager for top tab management.
// Pre-assign Shell's TabLayout (specific sizing) before SetElement.
_shellSectionAdapter = new ShellSectionTabbedViewAdapter(VirtualView);
_tabbedViewManager = new TabbedViewManager(MauiContext, _viewPager)
{
TabLayout = _contentTabLayout
};
_tabbedViewManager.SetElement(_shellSectionAdapter);

// Register page change callback (stored in field for cleanup in DisconnectHandler)
_pageChangedCallback = new ViewPagerPageChangeCallback(this);
_viewPager.RegisterOnPageChangeCallback(_pageChangedCallback);
SetupTabbedViewManager();

// Update TabLayout visibility based on item count
UpdateTabLayoutVisibility();
Expand All @@ -275,10 +249,6 @@ internal void SetupViewPagerAdapter()
// Set initial position
SetInitialPosition();

// Setup TabLayout appearance tracker
_tabLayoutAppearanceTracker = _shellContext.CreateTabLayoutAppearanceTracker(VirtualView);

// Register as appearance observer for TabLayout updates
var shell = VirtualView.FindParentOfType<Shell>();
if (shell is not null)
{
Expand All @@ -305,6 +275,53 @@ internal void SetupViewPagerAdapter()
}
}

void SetupTabbedViewManager()
{
if (_tabbedViewManager is not null || VirtualView is null || _viewPager is null || MauiContext is null || _shellContext is null)
{
return;
}

if (SectionController.GetItems().Count <= 1)
Comment thread
jonathanpeppers marked this conversation as resolved.
{
return;
}

var context = MauiContext.Context
?? throw new InvalidOperationException("MauiContext.Context cannot be null");

_contentTabLayout = new TabLayout(context)
{
Id = AView.GenerateViewId(),
LayoutParameters = new LP(LP.MatchParent, context.GetActionBarHeight()),
Visibility = ViewStates.Gone,
TabMode = TabLayout.ModeScrollable
};

_shellSectionAdapter = new ShellSectionTabbedViewAdapter(VirtualView);
_tabbedViewManager = new TabbedViewManager(MauiContext, _viewPager)
{
TabLayout = _contentTabLayout
};
_tabbedViewManager.SetElement(_shellSectionAdapter);

_pageChangedCallback = new ViewPagerPageChangeCallback(this);
_viewPager.RegisterOnPageChangeCallback(_pageChangedCallback);

_tabLayoutAppearanceTracker = _shellContext.CreateTabLayoutAppearanceTracker(VirtualView);

// Initial setup registers the appearance observer immediately afterward. When
// setup was deferred, replay the appearance for the newly-created TabLayout.
if (_registeredShell is not null && IsCurrentlyActiveSection() && VirtualView.CurrentItem is ShellContent currentContent)
{
var page = ((IShellContentController)currentContent).GetOrCreateContent();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness — The deferred-setup appearance replay uses the section's root content page (CurrentItemGetOrCreateContent()) as the AppearanceChanged source, not the page currently displayed by this section. IShellController.AppearanceChanged(source, false) calls UpdateToolbarAppearanceFeatures(source, null) with that source directly (Shell.cs), so the toolbar features are recomputed from a page that may not be on screen. Concrete scenario: an active ShellSection with one ShellContent where the app has pushed a detail page onto the section's stack, and then adds a second ShellContent (the deferred-creation trigger). The replay fires with the section root ContentPage while the pushed detail page is displayed, so Shell-level toolbar appearance is resolved against the wrong page. The sibling ShellItemHandler.SetupTabbedViewManager replay avoids this by using _displayedPage; this path should use the section's currently displayed page (navigation-stack top) and skip the replay when it cannot be resolved.

if (page is not null)
{
((IShellController)_registeredShell).AppearanceChanged(page, false);
}
}
}

void SetInitialPosition()
{
if (VirtualView?.CurrentItem is null || _viewPager is null)
Expand All @@ -322,7 +339,7 @@ void SetInitialPosition()

void UpdateTabLayoutVisibility()
{
if (_tabbedViewManager is null || VirtualView is null)
if (VirtualView is null)
{
return;
}
Expand Down Expand Up @@ -646,6 +663,7 @@ void OnItemsCollectionChanged(object? sender, NotifyCollectionChangedEventArgs e
var visibleCount = SectionController.GetItems().Count;
_viewPager?.OffscreenPageLimit = Math.Max(visibleCount, 1);

SetupTabbedViewManager();
UpdateTabLayoutVisibility();
UpdateViewPagerUserInput();
}
Expand Down Expand Up @@ -1017,7 +1035,7 @@ public override void OnPageSelected(int position)
toolbarTracker?.Page = page;

// Update CurrentItem
virtualView.CurrentItem = newCurrentItem;
virtualView.SetValueFromRenderer(ShellSection.CurrentItemProperty, newCurrentItem);
Comment thread
jonathanpeppers marked this conversation as resolved.

// Trigger appearance update
if (shell is not null)
Expand Down
Loading
Loading