Skip to content

Commit

Permalink
fix: avoid creating disconnected native navbars on load
Browse files Browse the repository at this point in the history
  • Loading branch information
kazo0 committed Oct 28, 2023
1 parent 115cb2e commit 2890f8d
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 51 deletions.
35 changes: 29 additions & 6 deletions src/Uno.Toolkit.RuntimeTests/Tests/NavigationBarTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,32 @@ public async Task NavigationBar_Renders_With_Invalid_AppBarButton_IconElement(Ty
AssertNavigationBar(frame);
}

[TestMethod]
[RequiresFullWindow]
public async Task Can_Navigate_Forward_And_Backwards()
{
var frame = new Frame() { Width = 400, Height = 400 };
var content = new Grid { Children = { frame } };

await UnitTestUIContentHelperEx.SetContentAndWait(content);

await UnitTestsUIContentHelper.WaitForIdle();

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

await UnitTestsUIContentHelper.WaitForLoaded(firstNavBar!);

var secondNavBar = await frame.NavigateAndGetNavBar<NavBarSecondPage>();

await UnitTestsUIContentHelper.WaitForLoaded(secondNavBar!);

await Task.Delay(1000);

frame.GoBack();

await UnitTestsUIContentHelper.WaitForLoaded(firstNavBar!);
}


#if __ANDROID__
private static void AssertNavigationBar(Frame frame)
Expand Down Expand Up @@ -534,17 +560,14 @@ public static class NavigationBarTestHelper
{
#if __IOS__
public static UINavigationBar? GetNativeNavBar(this NavigationBar? navBar) => navBar
?.TryGetRenderer<NavigationBar, NavigationBarRenderer>()
?.Native;
?.TryGetNative<NavigationBar, NavigationBarRenderer, UINavigationBar>(out var native) ?? false ? native : null;

public static UINavigationItem? GetNativeNavItem(this NavigationBar? navBar) => navBar
?.TryGetRenderer<NavigationBar, NavigationBarNavigationItemRenderer>()
?.Native;
?.TryGetNative<NavigationBar, NavigationBarNavigationItemRenderer, UINavigationItem>(out var native) ?? false ? native : null;

#elif __ANDROID__
public static AndroidX.AppCompat.Widget.Toolbar? GetNativeNavBar(this NavigationBar? navBar) => navBar
?.TryGetRenderer<NavigationBar, NavigationBarRenderer>()
?.Native;
?.TryGetNative<NavigationBar, NavigationBarRenderer, AndroidX.AppCompat.Widget.Toolbar>(out var native) ?? false ? native : null;
#endif
public static Task<NavigationBar?> NavigateAndGetNavBar<TPage>(this Frame frame) where TPage : Page
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ public partial class NativeNavigationBarPresenter : ContentPresenter, INavigatio
private SerialDisposable _mainCommandClickHandler = new SerialDisposable();
private WeakReference<NavigationBar?>? _navBarRef;

private UINavigationBar? _navigationBar;
private readonly bool _isPhone = UIDevice.CurrentDevice.UserInterfaceIdiom == UIUserInterfaceIdiom.Phone;

public NativeNavigationBarPresenter()
Expand Down Expand Up @@ -73,52 +72,54 @@ private void OnLoaded(object sender, RoutedEventArgs e)
_navBarRef = new WeakReference<NavigationBar?>(navBar);
}

_navigationBar = navBar?.GetOrAddDefaultRenderer().Native;

if (navBar is { })
{
navBar.MainCommand.Click += OnMainCommandClick;
_mainCommandClickHandler.Disposable = null;
_mainCommandClickHandler.Disposable = Disposable.Create(() => navBar.MainCommand.Click -= OnMainCommandClick);
}

if (_navigationBar == null)
{
throw new InvalidOperationException("No NavigationBar from renderer");
LayoutNativeNavBar(navBar);
}
}

_navigationBar.SetNeedsLayout();
private void LayoutNativeNavBar(NavigationBar navBar)
{
if (navBar.TryGetNative<NavigationBar, NavigationBarRenderer, UINavigationBar>(out var nativeBar)
&& nativeBar is { })
{
nativeBar.SetNeedsLayout();

var navigationBarSuperview = _navigationBar?.Superview;
var navigationBarSuperview = nativeBar.Superview;

// Allows the UINavigationController's NavigationBar instance to be moved to the Page. This feature
// is used in the context of the sample application to test NavigationBars outside of a NativeFramePresenter for
// UI Testing. In general cases, this should not happen as the bar may be moved back to to this presenter while
// another page is already visible, making this bar overlay on top of another.
if (FeatureConfiguration.CommandBar.AllowNativePresenterContent && (navigationBarSuperview == null || navigationBarSuperview is NativeNavigationBarPresenter))
{
Content = _navigationBar;
}
// Allows the UINavigationController's NavigationBar instance to be moved to the Page. This feature
// is used in the context of the sample application to test NavigationBars outside of a NativeFramePresenter for
// UI Testing. In general cases, this should not happen as the bar may be moved back to to this presenter while
// another page is already visible, making this bar overlay on top of another.
if (FeatureConfiguration.CommandBar.AllowNativePresenterContent && (navigationBarSuperview == null || navigationBarSuperview is NativeNavigationBarPresenter))
{
Content = nativeBar;
}

var statusBar = XamlStatusBar.GetForCurrentView();
var statusBar = XamlStatusBar.GetForCurrentView();

statusBar.Showing += OnStatusBarChanged;
statusBar.Hiding += OnStatusBarChanged;
statusBar.Showing += OnStatusBarChanged;
statusBar.Hiding += OnStatusBarChanged;

_statusBarSubscription.Disposable = Disposable.Create(() =>
{
statusBar.Showing -= OnStatusBarChanged;
statusBar.Hiding -= OnStatusBarChanged;
});
_statusBarSubscription.Disposable = Disposable.Create(() =>
{
statusBar.Showing -= OnStatusBarChanged;
statusBar.Hiding -= OnStatusBarChanged;
});

// iOS doesn't automatically update the navigation bar position when the status bar visibility changes.
void OnStatusBarChanged(XamlStatusBar sender, object args)
{
_navigationBar!.SetNeedsLayout();
_navigationBar!.Superview.SetNeedsLayout();
// iOS doesn't automatically update the navigation bar position when the status bar visibility changes.
void OnStatusBarChanged(XamlStatusBar sender, object args)
{
nativeBar.SetNeedsLayout();
nativeBar.Superview.SetNeedsLayout();
}
}
}

private void OnMainCommandClick(object sender, RoutedEventArgs e)
{
NavigationBar? navBar = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ internal partial class NavigationBarNavigationItemRenderer : Renderer<Navigation

public NavigationBarNavigationItemRenderer(NavigationBar element) : base(element) { }

protected override UINavigationItem CreateNativeInstance() => new UINavigationItem();
protected override UINavigationItem 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 @@ -36,17 +36,7 @@ internal partial class NavigationBarRenderer : Renderer<NavigationBar, UINavigat
{
public NavigationBarRenderer(NavigationBar element) : base(element) { }

protected override UINavigationBar CreateNativeInstance()
{
var navigationBar = new UINavigationBar();
if (Element is { } element)
{
var renderer = element.GetOrAddRenderer(navBar => new NavigationBarNavigationItemRenderer(navBar));
navigationBar.PushNavigationItem(renderer.Native, false);
}

return navigationBar;
}
protected override UINavigationBar CreateNativeInstance() => throw new NotSupportedException("The Native instance must be provided.");

protected override IEnumerable<IDisposable> Initialize()
{
Expand Down
25 changes: 23 additions & 2 deletions src/Uno.Toolkit.UI/Controls/NavigationBar/Renderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,14 @@ public TNative Native
}
}

public bool HasNative => _native != null;

private void OnNativeChanged()
{
// We remove subscriptions to the previous pair of element and native
_subscriptions.Dispose();

if (_native != null)
if (HasNative)
{
_subscriptions = new CompositeDisposable(Initialize());
}
Expand All @@ -109,7 +111,7 @@ private void OnNativeChanged()
public void Invalidate()
{
// We don't render anything if there's no rendering target
if (_native != null
if (HasNative
// Prevent Render() being called reentrantly - this can happen when the Element's parent changes within the Render() method
&& !_isRendering)
{
Expand Down Expand Up @@ -152,6 +154,25 @@ internal static class RendererHelper
return renderer;
}

public static bool TryGetNative<TElement, TRenderer, TNative>(this TElement element, out TNative? native)
where TElement :
#if HAS_UNO
class,
#endif
DependencyObject
where TRenderer : Renderer<TElement, TNative>
where TNative : class
{
native = null;
if (TryGetRenderer<TElement, TRenderer>(element) is { } renderer && renderer.HasNative)
{
native = renderer.Native;
return true;
}

return false;
}

public static void SetRenderer<TElement, TRenderer>(this TElement element, TRenderer? renderer)
where TElement : DependencyObject
{
Expand Down

0 comments on commit 2890f8d

Please sign in to comment.