Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: avoid creating disconnected native navbars on load #886

Merged
merged 1 commit into from
Oct 28, 2023
Merged

fix: avoid creating disconnected native navbars on load #886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 29 additions & 6 deletions src/Uno.Toolkit.RuntimeTests/Tests/NavigationBarTests.cs
View file
Open in desktop
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
63 changes: 32 additions & 31 deletions src/Uno.Toolkit.UI/Controls/NavigationBar/NativeNavigationBarPresenter.iOS.cs
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/Uno.Toolkit.UI/Controls/NavigationBar/NavigationBarNavigationItemRenderer.iOS.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ internal UINavigationItem? BackItem

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
12 changes: 1 addition & 11 deletions src/Uno.Toolkit.UI/Controls/NavigationBar/NavigationBarRenderer.iOS.cs
View file
Open in desktop
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
View file
Open in desktop
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
Loading