Skip to content

Commit

Permalink
fix(NavigationBar): Ensure proper iOS back button behavior (#548)
Browse files Browse the repository at this point in the history
  • Loading branch information
kazo0 authored Apr 25, 2023
1 parent 4fbab7f commit d70e30e
Show file tree
Hide file tree
Showing 15 changed files with 276 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,13 @@
mc:Ignorable="wasm xamarin">

<Grid Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">

<Grid.RowDefinitions>
<RowDefinition x:Name="TopPaddingRow"
Height="0" />
<RowDefinition Height="Auto" />
<RowDefinition Height="*" />
</Grid.RowDefinitions>


<!-- We set CompactModeThresholdWidth to a very high value so that it never happens. We don't want to use the compact mode. -->
<muxc:NavigationView Grid.Row="2"
x:Name="NavigationViewControl"
Expand Down
72 changes: 51 additions & 21 deletions src/Uno.Toolkit.RuntimeTests/Tests/NavigationBarTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -215,27 +215,16 @@ public async Task Can_Set_MainCommand_Label_Or_Content()
await UnitTestsUIContentHelper.WaitForIdle();

// FirstPage
var firstNavBar = await LoadNavigationBarFrom<FirstPage>();
Assert.IsNull(firstNavBar?.BackItem);
var firstNavBar = await frame.NavigateAndGetNavBar<FirstPage>();
Assert.IsNull(firstNavBar?.GetNativeNavBar()?.BackItem);

// LabelTitlePage
var labelTitleNavBar = await LoadNavigationBarFrom<LabelTitlePage>();
Assert.AreEqual("Label Title", labelTitleNavBar?.BackItem?.BackButtonTitle);
var labelTitleNavBar = await frame.NavigateAndGetNavBar<LabelTitlePage>();
Assert.AreEqual("Label Title", labelTitleNavBar?.GetNativeNavBar()?.BackItem?.BackButtonTitle);

// ContentTitlePage
var contentTitleNavBar = await LoadNavigationBarFrom<ContentTitlePage>();
Assert.AreEqual("Content Title", contentTitleNavBar?.BackItem?.BackButtonTitle);

async Task<UINavigationBar?> LoadNavigationBarFrom<TPage>() where TPage : Page
{
frame.Navigate(typeof(TPage));
await UnitTestsUIContentHelper.WaitForIdle();

var page = frame.Content as TPage;
await UnitTestsUIContentHelper.WaitForLoaded(page!);
var navBar = page?.FindChild<NavigationBar>();
return navBar?.GetRenderer<NavigationBar, NavigationBarRenderer>(null)?.Native;
}
var contentTitleNavBar = await frame.NavigateAndGetNavBar<ContentTitlePage>();
Assert.AreEqual("Content Title", contentTitleNavBar?.GetNativeNavBar()?.BackItem?.BackButtonTitle);
}

[TestMethod]
Expand Down Expand Up @@ -266,7 +255,7 @@ public async Task MainCommand_Use_LeftBarButtonItem_When_No_BackStack()
var firstPage = frame.Content as NavBarFirstPage;
firstPageNavBar = firstPage?.FindChild<NavigationBar>();

var renderedNativeNavItem = firstPageNavBar?.GetRenderer<NavigationBar, NavigationBarNavigationItemRenderer>(null)?.Native;
var renderedNativeNavItem = firstPageNavBar.GetNativeNavItem();

Assert.IsNotNull(renderedNativeNavItem?.LeftBarButtonItem);

Expand All @@ -277,7 +266,7 @@ public async Task MainCommand_Use_LeftBarButtonItem_When_No_BackStack()
var secondPage = frame.Content as NavBarSecondPage;
secondPageNavBar = secondPage?.FindChild<NavigationBar>();

renderedNativeNavItem = secondPageNavBar?.GetRenderer<NavigationBar, NavigationBarNavigationItemRenderer>(null)?.Native;
renderedNativeNavItem = secondPageNavBar.GetNativeNavItem();

Assert.IsNull(renderedNativeNavItem?.LeftBarButtonItem);
}
Expand Down Expand Up @@ -313,14 +302,30 @@ public async Task NavigationBar_Does_Render_Within_AutoLayout()
AssertNavigationBar(frame);
}

[TestMethod]
public async Task NavigationBar_Renders_BackItem_Within_AutoLayout()
{
var frame = new Frame { Width = 600, Height = 200 };;
await UnitTestUIContentHelperEx.SetContentAndWait(frame);

var firstNavBar = await frame.NavigateAndGetNavBar<NavBarAutoLayoutPage>();

await UnitTestsUIContentHelper.WaitForIdle();

frame.Navigate(typeof(NavBarAutoLayoutPage2));
await UnitTestsUIContentHelper.WaitForIdle();

await UnitTestUIContentHelperEx.WaitFor(() => firstNavBar?.GetNativeNavItem()?.BackButtonTitle == "Hello");
}

private static void AssertNavigationBar(Frame frame)
{
var page = frame.Content as Page;
var presenter = frame.FindChild<NativeFramePresenter>();
var navBar = page?.FindChild<NavigationBar>();

var renderedNativeNavItem = navBar?.GetRenderer<NavigationBar, NavigationBarNavigationItemRenderer>(null)?.Native;
var renderedNativeNavBar = navBar?.GetRenderer<NavigationBar, NavigationBarRenderer>(null)?.Native;
var renderedNativeNavItem = navBar.GetNativeNavItem();
var renderedNativeNavBar = navBar.GetNativeNavBar();

Assert.IsNotNull(presenter);
Assert.IsFalse(presenter!.NavigationController.NavigationBarHidden);
Expand All @@ -329,6 +334,8 @@ private static void AssertNavigationBar(Frame frame)
Assert.AreSame(renderedNativeNavBar, presenter.NavigationController.NavigationBar);
}



private sealed partial class FirstPage : Page
{
public FirstPage()
Expand Down Expand Up @@ -383,5 +390,28 @@ public NavBarTestPage()
}
#endif
}

#if __IOS__
public static class NavigationBarTestHelper
{
public static UINavigationBar? GetNativeNavBar(this NavigationBar? navBar) => navBar
?.TryGetRenderer<NavigationBar, NavigationBarRenderer>()
?.Native;

public static UINavigationItem? GetNativeNavItem(this NavigationBar? navBar) => navBar
?.TryGetRenderer<NavigationBar, NavigationBarNavigationItemRenderer>()
?.Native;

public static async Task<NavigationBar?> NavigateAndGetNavBar<TPage>(this Frame frame) where TPage : Page
{
frame.Navigate(typeof(TPage));
await UnitTestsUIContentHelper.WaitForIdle();

var page = frame.Content as TPage;
await UnitTestsUIContentHelper.WaitForLoaded(page!);
return page?.FindChild<NavigationBar>();
}
}
#endif
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<Page x:Class="Uno.Toolkit.RuntimeTests.Tests.TestPages.NavBarAutoLayoutPage2"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="using:Uno.Toolkit.RuntimeTests.Tests.TestPages"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:utu="using:Uno.Toolkit.UI"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d"
Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">

<utu:AutoLayout>
<utu:NavigationBar Content="Testing 2">
<utu:NavigationBar.MainCommand>
<AppBarButton Label="Hello" />
</utu:NavigationBar.MainCommand>
</utu:NavigationBar>
</utu:AutoLayout>
</Page>
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices.WindowsRuntime;
using Windows.Foundation;
using Windows.Foundation.Collections;
using Uno.Toolkit.UI;

#if IS_WINUI
using Microsoft.UI;
using Microsoft.UI.Xaml;
using Microsoft.UI.Xaml.Controls;
using Microsoft.UI.Xaml.Controls.Primitives;
using Microsoft.UI.Xaml.Data;
using Microsoft.UI.Xaml.Input;
using Microsoft.UI.Xaml.Media;
using Microsoft.UI.Xaml.Navigation;
#else
using Windows.UI;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Controls;
using Windows.UI.Xaml.Controls.Primitives;
using Windows.UI.Xaml.Data;
using Windows.UI.Xaml.Input;
using Windows.UI.Xaml.Media;
using Windows.UI.Xaml.Navigation;
#endif

namespace Uno.Toolkit.RuntimeTests.Tests.TestPages
{
public sealed partial class NavBarAutoLayoutPage2 : Page
{
public NavBarAutoLayoutPage2()
{
this.InitializeComponent();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,18 @@ internal class AppBarButtonRenderer : Renderer<AppBarButton, IMenuItem>
private bool _isInOverflow;
private DependencyObject? _elementParent;

public AppBarButtonRenderer(AppBarButton element, bool isInOverflow = false) : base(element)
public bool IsInOverflow
{
get => _isInOverflow;
set
{
_isInOverflow = value;
Invalidate();
}
}

public AppBarButtonRenderer(AppBarButton element) : base(element)
{
_isInOverflow = isInOverflow;
element.ViewAttachedToWindow += OnElementAttachedToWindow;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ protected override void Render()
native.Image = null;
native.ClearCustomView();
native.Title = element.Content is string content ? content : element.Label;

if (element.Icon != null)
{
switch (element.Icon)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -861,8 +861,7 @@ public FrameNavigationController() : base(typeof(UnoNavigationBar), typeof(UIToo
public override UIViewController PopViewController(bool animated)
{
var lowerNavigationBar = LowerController?.GetNavigationBar();
var renderer = lowerNavigationBar?.GetRenderer(() => (NavigationBarRenderer?)null);
if (renderer != null)
if (lowerNavigationBar?.TryGetRenderer<NavigationBar, NavigationBarRenderer>() is { } renderer)
{
// Set navigation bar properties for page about to become visible. This gives a nice animation and works around bug on
// iOS 11.2 where TitleTextAttributes aren't updated properly (https://openradar.appspot.com/37567828)
Expand All @@ -878,10 +877,11 @@ public override void PushViewController(UIViewController viewController, bool an

if (viewController is PageViewController pvc)
{
var pushedNavBar = pvc.GetNavigationBar();

var renderer = pushedNavBar?.GetRenderer(() => (NavigationBarNavigationItemRenderer?)null);
renderer?.SetBackItem(LowerController?.NavigationItem);
var pushedNavBar = pvc.View?.FindTopNavigationBar();
if (pushedNavBar?.TryGetRenderer<NavigationBar, NavigationBarNavigationItemRenderer>() is { } renderer)
{
renderer.BackItem = LowerController?.NavigationItem;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ private void OnLoaded(object sender, RoutedEventArgs e)
var navBar = TemplatedParent as NavigationBar;
if (navBar is { })
{
Content = navBar.GetRenderer(() => new NavigationBarRenderer(navBar)).Native;
Content = navBar.GetOrAddDefaultRenderer().Native;
navBar.MainCommand.Click += OnMainCommandClicked;
_mainCommandClickHandler.Disposable = null;
_mainCommandClickHandler.Disposable = Disposable.Create(() => navBar.MainCommand.Click -= OnMainCommandClicked);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,10 @@ private void OnLoaded(object sender, RoutedEventArgs e)
{
navBar = TemplatedParent as NavigationBar;
_navBarRef = new WeakReference<NavigationBar?>(navBar);

_navigationBar = navBar?.GetRenderer(RendererFactory).Native;

}
else
{
_navigationBar = navBar?.ResetRenderer(RendererFactory).Native;
}

_navigationBar = navBar?.GetOrAddDefaultRenderer().Native;

if (navBar is { })
{
navBar.MainCommand.Click += OnMainCommandClick;
Expand Down Expand Up @@ -133,15 +128,6 @@ private void OnMainCommandClick(object sender, RoutedEventArgs e)
}
}

NavigationBarRenderer RendererFactory()
{
NavigationBar? navBar = null;

_navBarRef?.TryGetTarget(out navBar);

return new NavigationBarRenderer(navBar!);
}

protected override Size MeasureOverride(Size size)
{
var measuredSize = base.MeasureOverride(size);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,19 @@ namespace Uno.Toolkit.UI
{
internal class NavigationAppBarButtonRenderer : Renderer<AppBarButton, Toolbar>
{
private readonly MainCommandMode _mode;

public NavigationAppBarButtonRenderer(AppBarButton element, MainCommandMode mode) : base(element)
private MainCommandMode _mode;
public MainCommandMode Mode
{
_mode = mode;
get => _mode;
set
{
_mode = value;
Invalidate();
}
}

public NavigationAppBarButtonRenderer(AppBarButton element) : base(element) { }

protected override Toolbar CreateNativeInstance() => throw new NotSupportedException("The Native instance must be provided.");

protected override IEnumerable<IDisposable> Initialize()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using System;
using System.Linq;
using UIKit;
using SpriteKit;
using Microsoft.UI.Xaml.Automation.Peers;

#if IS_WINUI
using Microsoft.UI.Xaml;
Expand All @@ -15,14 +17,18 @@ namespace Uno.Toolkit.UI
{
internal static class NavigationBarHelper
{
internal static void SetNavigationBar(NavigationBar NavigationBar, UIKit.UINavigationBar? navigationBar)
internal static void SetNavigationBar(NavigationBar navigationBar, UIKit.UINavigationBar? uiNavigationBar)
{
NavigationBar.GetRenderer(() => new NavigationBarRenderer(NavigationBar)).Native = navigationBar;
navigationBar.AddOrUpdateRenderer(
onCreate: navBar => new NavigationBarRenderer(navBar) { Native = uiNavigationBar },
onUpdate: (navBar, renderer) => renderer.Native = uiNavigationBar);
}

internal static void SetNavigationItem(NavigationBar NavigationBar, UIKit.UINavigationItem? navigationItem)
internal static void SetNavigationItem(NavigationBar navigationBar, UIKit.UINavigationItem? navigationItem)
{
NavigationBar.GetRenderer(() => new NavigationBarNavigationItemRenderer(NavigationBar)).Native = navigationItem;
navigationBar.AddOrUpdateRenderer(
onCreate: navBar => new NavigationBarNavigationItemRenderer(navBar) { Native = navigationItem },
onUpdate: (navBar, renderer) => renderer.Native = navigationItem);
}

/// <summary>
Expand Down Expand Up @@ -139,11 +145,25 @@ private static void EnsureNavigationItem(NavigationBar topNavigationBar, UIViewC
{
// The Native view from the renderer may have been set to a dummy NavigationItem if we were not able to find
// a NavigationBar in the PageCreated method. If that is the case, set it to the pageController's NavigationItem
var nativeItem = topNavigationBar.GetRenderer(() => (NavigationBarNavigationItemRenderer?)null)?.Native;
if (!ReferenceEquals(nativeItem, pageController.NavigationItem))
var renderer = topNavigationBar.TryGetRenderer<NavigationBar, NavigationBarNavigationItemRenderer>();

if (!ReferenceEquals(renderer?.Native, pageController.NavigationItem))
{
SetNavigationItem(topNavigationBar, pageController.NavigationItem);
}

// We should also ensure that the renderer's BackItem is set to the previous page's NavigationItem
// in order to ensure that the back button icon and label are properly rendered.
var vcs = pageController.NavigationController?.ViewControllers;
var lowerVc = vcs?.Length > 1 ? vcs[vcs.Length - 2] : null;

if (lowerVc != null && renderer is { })
{
if (!ReferenceEquals(renderer.BackItem, lowerVc.NavigationItem))
{
renderer.BackItem = lowerVc.NavigationItem;
}
}
}

/// <summary>
Expand Down
Loading

0 comments on commit d70e30e

Please sign in to comment.