From 2f532afa40bd5d015775173dec29723a93a7427c Mon Sep 17 00:00:00 2001 From: Steve Bilogan Date: Thu, 30 Nov 2023 16:00:48 -0500 Subject: [PATCH] fix: avoid creating disconnected native commandbars on load --- .../CommandBarPages/CommandBarFirstPage.xaml | 21 ++++++ .../CommandBarFirstPage.xaml.cs | 29 +++++++++ .../CommandBarPages/CommandBarSecondPage.xaml | 20 ++++++ .../CommandBarSecondPage.xaml.cs | 29 +++++++++ .../Given_CommandBar.cs | 64 +++++++++++++++++++ .../CommandBarNavigationItemRenderer.iOS.cs | 2 +- .../CommandBar/CommandBarRenderer.iOS.cs | 9 +-- .../NativeCommandBarPresenter.iOS.cs | 52 ++++++++------- src/Uno.UI/Controls/Renderer.cs | 50 ++++++++++++++- 9 files changed, 243 insertions(+), 33 deletions(-) create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarFirstPage.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarFirstPage.xaml.cs create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarSecondPage.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarSecondPage.xaml.cs diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarFirstPage.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarFirstPage.xaml new file mode 100644 index 000000000000..2a7a54e0e3d9 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarFirstPage.xaml @@ -0,0 +1,21 @@ + + + + + + + + + + + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarFirstPage.xaml.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarFirstPage.xaml.cs new file mode 100644 index 000000000000..568eb76639e5 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarFirstPage.xaml.cs @@ -0,0 +1,29 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices.WindowsRuntime; +using Uno.UI.Samples; +using Windows.Foundation; +using Windows.Foundation.Collections; +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; + +// The Blank Page item template is documented at https://go.microsoft.com/fwlink/?LinkId=234238 + +namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls.CommandBarPages; +/// +/// An empty page that can be used on its own or navigated to within a Frame. +/// +public sealed partial class CommandBarFirstPage : Page +{ + public CommandBarFirstPage() + { + this.InitializeComponent(); + } +} diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarSecondPage.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarSecondPage.xaml new file mode 100644 index 000000000000..c79a37955fb3 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarSecondPage.xaml @@ -0,0 +1,20 @@ + + + + + + + + + + + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarSecondPage.xaml.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarSecondPage.xaml.cs new file mode 100644 index 000000000000..58924c49fb58 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/CommandBarPages/CommandBarSecondPage.xaml.cs @@ -0,0 +1,29 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices.WindowsRuntime; +using Uno.UI.Samples; +using Windows.Foundation; +using Windows.Foundation.Collections; +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; + +// The Blank Page item template is documented at https://go.microsoft.com/fwlink/?LinkId=234238 + +namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls.CommandBarPages; +/// +/// An empty page that can be used on its own or navigated to within a Frame. +/// +public sealed partial class CommandBarSecondPage : Page +{ + public CommandBarSecondPage() + { + this.InitializeComponent(); + } +} diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_CommandBar.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_CommandBar.cs index b6c7cf4e34d8..f7e7b54df911 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_CommandBar.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_CommandBar.cs @@ -7,6 +7,14 @@ using Windows.UI.Xaml.Media; using static Private.Infrastructure.TestServices; +using Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls.CommandBarPages; + +#if __IOS__ +using Uno.UI.Controls; +using Uno.UI.Helpers.WinUI; +using UIKit; +#endif + namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls { [TestClass] @@ -57,5 +65,61 @@ public async Task When_Popup_Open_Then_Click_Outside() Assert.AreEqual(0, VisualTreeHelper.GetOpenPopupsForXamlRoot(SUT.XamlRoot).Count); } + +#if __IOS__ + [TestMethod] + [RunsOnUIThread] + [RequiresFullWindow] + + public async Task Can_Navigate_Forward_And_Backwards() + { + var frame = new Frame() { Width = 400, Height = 400 }; + var content = new Grid { Children = { frame } }; + + WindowHelper.WindowContent = content; + await WindowHelper.WaitForIdle(); + + var firstNavBar = await frame.NavigateAndGetNavBar(); + + await WindowHelper.WaitForLoaded(firstNavBar); + + var secondNavBar = await frame.NavigateAndGetNavBar(); + + await WindowHelper.WaitForLoaded(secondNavBar); + + await Task.Delay(1000); + + frame.GoBack(); + + await WindowHelper.WaitForLoaded(firstNavBar); + } +#endif + } + +#if __IOS__ + public static class NavigationBarTestHelper + { + public static UINavigationBar GetNativeNavBar(this CommandBar navBar) => navBar + ?.TryGetNative(out var native) ?? false ? native : null; + + public static UINavigationItem GetNativeNavItem(this CommandBar navBar) => navBar + ?.TryGetNative(out var native) ?? false ? native : null; + + + public static Task NavigateAndGetNavBar(this Frame frame) where TPage : Page + { + return frame.NavigateAndGetNavBar(typeof(TPage)); + } + + public static async Task NavigateAndGetNavBar(this Frame frame, Type pageType) + { + frame.Navigate(pageType); + await WindowHelper.WaitForIdle(); + + var page = frame.Content as Page; + await WindowHelper.WaitForLoaded(page!); + return SharedHelpers.FindInVisualTreeByType(page); + } } +#endif } diff --git a/src/Uno.UI/Controls/CommandBar/CommandBarNavigationItemRenderer.iOS.cs b/src/Uno.UI/Controls/CommandBar/CommandBarNavigationItemRenderer.iOS.cs index 5dbeb4171dec..b9ec74e13e4c 100644 --- a/src/Uno.UI/Controls/CommandBar/CommandBarNavigationItemRenderer.iOS.cs +++ b/src/Uno.UI/Controls/CommandBar/CommandBarNavigationItemRenderer.iOS.cs @@ -27,7 +27,7 @@ internal partial class CommandBarNavigationItemRenderer : Renderer new UINavigationItem(); + protected override UINavigationItem CreateNativeInstance() => throw new NotSupportedException("The Native instance must be provided."); protected override IEnumerable Initialize() { diff --git a/src/Uno.UI/Controls/CommandBar/CommandBarRenderer.iOS.cs b/src/Uno.UI/Controls/CommandBar/CommandBarRenderer.iOS.cs index 5e5aa11fe4c2..94c1643752bb 100644 --- a/src/Uno.UI/Controls/CommandBar/CommandBarRenderer.iOS.cs +++ b/src/Uno.UI/Controls/CommandBar/CommandBarRenderer.iOS.cs @@ -22,14 +22,7 @@ internal partial class CommandBarRenderer : Renderer new CommandBarNavigationItemRenderer(Element)).Native; - navigationBar.PushNavigationItem(navigationItem, false); - - return navigationBar; - } + protected override UINavigationBar CreateNativeInstance() => throw new NotSupportedException("The Native instance must be provided."); protected override IEnumerable Initialize() { diff --git a/src/Uno.UI/Controls/CommandBar/NativeCommandBarPresenter.iOS.cs b/src/Uno.UI/Controls/CommandBar/NativeCommandBarPresenter.iOS.cs index 0349ea7eeb37..48b302abdf41 100644 --- a/src/Uno.UI/Controls/CommandBar/NativeCommandBarPresenter.iOS.cs +++ b/src/Uno.UI/Controls/CommandBar/NativeCommandBarPresenter.iOS.cs @@ -24,7 +24,6 @@ public partial class NativeCommandBarPresenter : ContentPresenter private readonly SerialDisposable _orientationSubscription = new SerialDisposable(); private WeakReference? _commandBar; - private UINavigationBar? _navigationBar; private readonly bool _isPhone = UIDevice.CurrentDevice.UserInterfaceIdiom == UIUserInterfaceIdiom.Phone; private protected override void OnLoaded() @@ -40,23 +39,40 @@ private protected override void OnLoaded() { commandBar = TemplatedParent as CommandBar; _commandBar = new WeakReference(commandBar); - - _navigationBar = commandBar?.GetRenderer(RendererFactory).Native; - } - else + + if (commandBar?.TryGetRenderer() is { } commandBarRenderer) { - _navigationBar = commandBar?.ResetRenderer(RendererFactory).Native; + if (commandBarRenderer.Native is { } nativeBar) + { + LayoutNativeBar(nativeBar); + } } - - if (_navigationBar == null) + else if (commandBar is { } && FeatureConfiguration.CommandBar.AllowNativePresenterContent) { - throw new InvalidOperationException("No NavigationBar from renderer"); + commandBarRenderer = new CommandBarRenderer(commandBar); + commandBar.SetRenderer(commandBarRenderer); + + + var commandBarItemRenderer = new CommandBarNavigationItemRenderer(commandBar); + commandBar.SetRenderer(commandBarItemRenderer); + + var navigationItem = new UINavigationItem(); + var nativeBar = new UINavigationBar(); + nativeBar.PushNavigationItem(navigationItem, false); + + commandBarItemRenderer.Native = navigationItem; + commandBarRenderer.Native = nativeBar; + + LayoutNativeBar(nativeBar); } + } - _navigationBar.SetNeedsLayout(); + private void LayoutNativeBar(UINavigationBar nativeBar) + { + 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 @@ -64,7 +80,7 @@ private protected override void OnLoaded() // another page is already visible, making this bar overlay on top of another. if (FeatureConfiguration.CommandBar.AllowNativePresenterContent && (navigationBarSuperview == null || navigationBarSuperview is NativeCommandBarPresenter)) { - Content = _navigationBar; + Content = nativeBar; } var statusBar = StatusBar.GetForCurrentView(); @@ -81,18 +97,10 @@ private protected override void OnLoaded() // iOS doesn't automatically update the navigation bar position when the status bar visibility changes. void OnStatusBarChanged(StatusBar sender, object args) { - _navigationBar!.SetNeedsLayout(); - _navigationBar!.Superview.SetNeedsLayout(); + nativeBar.SetNeedsLayout(); + nativeBar.Superview.SetNeedsLayout(); } } - - CommandBarRenderer RendererFactory() - { - CommandBar? target = null; - _commandBar?.TryGetTarget(out target); - return new CommandBarRenderer(target); - } - protected override Size MeasureOverride(Size size) { var measuredSize = base.MeasureOverride(size); diff --git a/src/Uno.UI/Controls/Renderer.cs b/src/Uno.UI/Controls/Renderer.cs index c657db5da82f..7cf9f43f8196 100644 --- a/src/Uno.UI/Controls/Renderer.cs +++ b/src/Uno.UI/Controls/Renderer.cs @@ -11,6 +11,8 @@ namespace Uno.UI.Controls { + public delegate void NativeChangedHandler(object sender, object native); + internal abstract class Renderer : IDisposable where TElement : DependencyObject where TNative : class @@ -20,6 +22,8 @@ internal abstract class Renderer : IDisposable private TNative _native; private bool _isRendering; + public event NativeChangedHandler NativeChanged; + public Renderer(TElement element) { if (element == null) @@ -56,17 +60,21 @@ 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()); } Invalidate(); + + NativeChanged?.Invoke(this, _native); } protected abstract TNative CreateNativeInstance(); @@ -74,7 +82,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) { @@ -104,6 +112,38 @@ internal static class RendererHelper { private static readonly WeakAttachedDictionary _renderers = new WeakAttachedDictionary(); + public static TRenderer TryGetRenderer(this TElement element) + where TElement : DependencyObject + where TRenderer : class + { + TRenderer renderer = null; + if (_renderers.GetValue(element, typeof(TRenderer)) is { } existingRenderer) + { + renderer = existingRenderer; + } + + return renderer; + } + + public static bool TryGetNative(this TElement element, out TNative native) + where TElement : +#if HAS_UNO + class, +#endif + DependencyObject + where TRenderer : Renderer + where TNative : class + { + native = null; + if (TryGetRenderer(element) is { } renderer && renderer.HasNative) + { + native = renderer.Native; + return true; + } + + return false; + } + public static TRenderer GetRenderer(this TElement element, Func rendererFactory) where TElement : DependencyObject { @@ -114,5 +154,11 @@ public static TRenderer ResetRenderer(this TElement element { return _renderers.GetValue(element, typeof(TRenderer), rendererFactory); } + + public static void SetRenderer(this TElement element, TRenderer renderer) + where TElement : DependencyObject + { + _renderers.SetValue(element, typeof(TRenderer), renderer); + } } }