Skip to content

Commit

Permalink
fix: avoid creating disconnected native commandbars on load
Browse files Browse the repository at this point in the history
  • Loading branch information
kazo0 committed Dec 1, 2023
1 parent 5a505ff commit 2f532af
Show file tree
Hide file tree
Showing 9 changed files with 243 additions and 33 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Page
x:Class="Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls.CommandBarPages.CommandBarFirstPage"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="using:Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls.CommandBarPages"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d"
Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">

<Grid>
<Grid.RowDefinitions>
<RowDefinition Height="Auto" />
<RowDefinition Height="100" />
</Grid.RowDefinitions>
<CommandBar Content="CommandBarFirstPage"
Width="100" />
<TextBlock Grid.Row="1"
Text="Hello from CommandBarFirstPage" />
</Grid>
</Page>
Original file line number Diff line number Diff line change
@@ -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;
/// <summary>
/// An empty page that can be used on its own or navigated to within a Frame.
/// </summary>
public sealed partial class CommandBarFirstPage : Page
{
public CommandBarFirstPage()
{
this.InitializeComponent();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<Page
x:Class="Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls.CommandBarPages.CommandBarSecondPage"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="using:Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls.CommandBarPages"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d"
Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">

<Grid>
<Grid.RowDefinitions>
<RowDefinition Height="Auto" />
<RowDefinition Height="100" />
</Grid.RowDefinitions>
<CommandBar Content="CommandBarSecondPage" />
<TextBlock Grid.Row="1"
Text="Hello from CommandBarSecondPage" />
</Grid>
</Page>
Original file line number Diff line number Diff line change
@@ -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;
/// <summary>
/// An empty page that can be used on its own or navigated to within a Frame.
/// </summary>
public sealed partial class CommandBarSecondPage : Page
{
public CommandBarSecondPage()
{
this.InitializeComponent();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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<CommandBarFirstPage>();

await WindowHelper.WaitForLoaded(firstNavBar);

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

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<CommandBar, CommandBarRenderer, UINavigationBar>(out var native) ?? false ? native : null;

public static UINavigationItem GetNativeNavItem(this CommandBar navBar) => navBar
?.TryGetNative<CommandBar, CommandBarNavigationItemRenderer, UINavigationItem>(out var native) ?? false ? native : null;


public static Task<CommandBar> NavigateAndGetNavBar<TPage>(this Frame frame) where TPage : Page
{
return frame.NavigateAndGetNavBar(typeof(TPage));
}

public static async Task<CommandBar> 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<CommandBar>(page);
}
}
#endif
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ internal partial class CommandBarNavigationItemRenderer : Renderer<CommandBar, U

public CommandBarNavigationItemRenderer(CommandBar 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
9 changes: 1 addition & 8 deletions src/Uno.UI/Controls/CommandBar/CommandBarRenderer.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,7 @@ internal partial class CommandBarRenderer : Renderer<CommandBar, UINavigationBar

public CommandBarRenderer(CommandBar element) : base(element) { }

protected override UINavigationBar CreateNativeInstance()
{
var navigationBar = new UINavigationBar();
var navigationItem = Element.GetRenderer(() => 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<IDisposable> Initialize()
{
Expand Down
52 changes: 30 additions & 22 deletions src/Uno.UI/Controls/CommandBar/NativeCommandBarPresenter.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public partial class NativeCommandBarPresenter : ContentPresenter
private readonly SerialDisposable _orientationSubscription = new SerialDisposable();
private WeakReference<CommandBar?>? _commandBar;

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

private protected override void OnLoaded()
Expand All @@ -40,31 +39,48 @@ private protected override void OnLoaded()
{
commandBar = TemplatedParent as CommandBar;
_commandBar = new WeakReference<CommandBar?>(commandBar);

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

}
else

if (commandBar?.TryGetRenderer<CommandBar, CommandBarRenderer>() 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
// 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 NativeCommandBarPresenter))
{
Content = _navigationBar;
Content = nativeBar;
}

var statusBar = StatusBar.GetForCurrentView();
Expand All @@ -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);
Expand Down
50 changes: 48 additions & 2 deletions src/Uno.UI/Controls/Renderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Uno.UI.Controls
{
public delegate void NativeChangedHandler(object sender, object native);

internal abstract class Renderer<TElement, TNative> : IDisposable
where TElement : DependencyObject
where TNative : class
Expand All @@ -20,6 +22,8 @@ internal abstract class Renderer<TElement, TNative> : IDisposable
private TNative _native;
private bool _isRendering;

public event NativeChangedHandler NativeChanged;

public Renderer(TElement element)
{
if (element == null)
Expand Down Expand Up @@ -56,25 +60,29 @@ 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();

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 @@ -104,6 +112,38 @@ internal static class RendererHelper
{
private static readonly WeakAttachedDictionary<DependencyObject, Type> _renderers = new WeakAttachedDictionary<DependencyObject, Type>();

public static TRenderer TryGetRenderer<TElement, TRenderer>(this TElement element)
where TElement : DependencyObject
where TRenderer : class
{
TRenderer renderer = null;
if (_renderers.GetValue<TRenderer>(element, typeof(TRenderer)) is { } existingRenderer)
{
renderer = existingRenderer;
}

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 TRenderer GetRenderer<TElement, TRenderer>(this TElement element, Func<TRenderer> rendererFactory)
where TElement : DependencyObject
{
Expand All @@ -114,5 +154,11 @@ public static TRenderer ResetRenderer<TElement, TRenderer>(this TElement element
{
return _renderers.GetValue(element, typeof(TRenderer), rendererFactory);
}

public static void SetRenderer<TElement, TRenderer>(this TElement element, TRenderer renderer)
where TElement : DependencyObject
{
_renderers.SetValue(element, typeof(TRenderer), renderer);
}
}
}

0 comments on commit 2f532af

Please sign in to comment.