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: enable any iconelement for navigationbar on mobile #1045

Merged
merged 2 commits into from
Feb 16, 2024
Merged

fix: enable any iconelement for navigationbar on mobile #1045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8bf1225
fix: enable any iconelement for navigationbar on mobile
kazo0 Feb 14, 2024
1d73e91
fix: fix android parent for appbarbuttonwrapper
kazo0 Feb 16, 2024
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
19 changes: 6 additions & 13 deletions doc/controls/NavigationBar.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,7 @@ When `Content` is a `string`, it's displayed using the platform's default font f
<utu:NavigationBar.PrimaryCommands>
<AppBarButton Label="Share">
<AppBarButton.Icon>
<BitmapIcon ShowAsMonochrome="False"
UriSource="ms-appx:///Assets/Share.png" />
<BitmapIcon UriSource="ms-appx:///Assets/Share.png" />
</AppBarButton.Icon>
</AppBarButton>
</utu:NavigationBar.PrimaryCommands>
Expand Down Expand Up @@ -182,8 +181,7 @@ When `Content` is a `FrameworkElement`, it's displayed within the available area
<utu:NavigationBar.PrimaryCommands>
<AppBarButton Label="Share">
<AppBarButton.Icon>
<BitmapIcon ShowAsMonochrome="False"
UriSource="ms-appx:///Assets/Share.png" />
<BitmapIcon UriSource="ms-appx:///Assets/Share.png" />
</AppBarButton.Icon>
</AppBarButton>
</utu:NavigationBar.PrimaryCommands>
Expand Down Expand Up @@ -214,7 +212,7 @@ Remarks:

* This is typically used to change the text color of the `Content`.
* Only supports `SolidColorBrush`.
* Setting this property will not affect the tint color of the `PrimaryCommands` or `SecondaryCommands`. If you need to change the `AppBarButton` tint, this is possible by setting the `ShowAsMonochrome` property to true as well as setting the `Foreground` on the `BitmapIcon`.
* Setting this property will not affect the tint color of the `PrimaryCommands` or `SecondaryCommands`. If you need to change the `AppBarButton` color, you should set the `Foreground` property on each `AppBarButton`s individually.

### PrimaryCommands

Expand All @@ -229,8 +227,7 @@ Gets the collection of primary command elements for the `NavigationBar`.
<utu:NavigationBar.PrimaryCommands>
<AppBarButton Label="Search">
<AppBarButton.Icon>
<BitmapIcon ShowAsMonochrome="False"
UriSource="ms-appx:///Assets/Search.png" />
<BitmapIcon UriSource="ms-appx:///Assets/Search.png" />
</AppBarButton.Icon>
</AppBarButton>
</utu:NavigationBar.PrimaryCommands>
Expand Down Expand Up @@ -493,7 +490,7 @@ When `AppBarButton` is used within a native `NavigationBar`, its control templat
| `Command` | x | x | x | |
| `Content` | x | x* | x* | Supports `string` and `FrameworkElement` |
| `Foreground` | x | x | x* | **Android**: See details below |
| `Icon` | x | x* | x* | Only supports `BitmapIcon` |
| `Icon` | x | x* | x* | See details below |
| `IsEnabled` | x | x | x* | **Android**: Not supported with `Content` |
| `Label` | x | x* | x* | See details below |
| `Opacity` | x | x | x | |
Expand Down Expand Up @@ -543,7 +540,7 @@ Gets or sets the graphic content of the `AppBarButton`

Remarks:

* Only supports `BitmapIcon` (with PNG).
* On **Android** and **iOS**, the `MainCommand` Icon only supports `BitmapIcon`s. `PrimaryCommands` and `SecondaryCommands` support any `IconElement` type.

### Recommended icon sizes (by scale)

Expand Down Expand Up @@ -707,10 +704,6 @@ You can't place your `NavigationBar` anywhere other than at the top of the `Page

You can't currently change the height of the `NavigationBar`. It is dictated by the platform and the device.

### How can I use a Path for the AppBarButton Icon?

`AppBarButton` doesn't currently support `PathIcon`. Only `BitmapIcon` with PNGs is supported. Please refer to the **Icon** section.

### How can I customize the pressed/disabled visual states of my AppBarButton?

You can't currently customize the visual states of `AppBarButton` when using `NavigationBar` in native mode.
Expand Down
113 changes: 49 additions & 64 deletions src/Uno.Toolkit.UI/Controls/NavigationBar/AppBarButtonRenderer.Android.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,12 @@ protected override IEnumerable<IDisposable> Initialize()
{
// Content
_appBarButtonWrapper = new AppBarButtonWrapper();
if (Element?.Content is FrameworkElement content && content.Visibility == Visibility.Visible)
{
var elementsParent = Element.Parent;
_appBarButtonWrapper.SetParent(elementsParent);

yield return Disposable.Create(() =>
{
Element.ViewAttachedToWindow -= OnElementAttachedToWindow;
});
var iconOrContent = Element?.Icon ?? Element?.Content;
if (Element is { } && iconOrContent is FrameworkElement content && content.Visibility == Visibility.Visible)
{
_elementParent = Element.Parent;
_appBarButtonWrapper.SetParent(_elementParent);
}

yield return Disposable.Create(() => _appBarButtonWrapper = null);
Expand All @@ -99,6 +96,8 @@ protected override IEnumerable<IDisposable> Initialize()
new[] { AppBarButton.LabelProperty },
new[] { AppBarButton.IconProperty },
new[] { AppBarButton.IconProperty, BitmapIcon.UriSourceProperty },
new[] { AppBarButton.IconProperty, IconElement.ForegroundProperty },
new[] { AppBarButton.IconProperty, IconElement.ForegroundProperty, SolidColorBrush.ColorProperty },
new[] { AppBarButton.ContentProperty },
new[] { AppBarButton.ContentProperty, FrameworkElement.VisibilityProperty },
new[] { AppBarButton.OpacityProperty },
Expand All @@ -109,13 +108,18 @@ protected override IEnumerable<IDisposable> Initialize()
new[] { AppBarButton.IsEnabledProperty },
new[] { AppBarButton.IsInOverflowProperty }
);

yield return Disposable.Create(() =>
{
element.ViewAttachedToWindow -= OnElementAttachedToWindow;
});
}
}

protected override void Render()
{
// NavigationBar::PrimaryCommands -> !IsInOverflow -> AsAction.Never -> displayed directly on navbar
// NavigationBar::SecondaryCommands -> IsInOverflow -> AsAction.Awalys -> (displayed as flyout menu items under [...])
// NavigationBar::SecondaryCommands -> IsInOverflow -> AsAction.Always -> (displayed as flyout menu items under [...])

if (Native is not { } native)
{
Expand All @@ -132,24 +136,44 @@ protected override void Render()

string? titleText = element.Label;

native.SetTitle(null);
native.SetActionView(null);
native.SetIcon(null);

// (Icon ?? Content) and Label
if (!TrySetIcon() && element.Content is { } content)
if (_isInOverflow)
{
if (content is string text)
{
titleText = text;
}
else if (content is FrameworkElement { Visibility: Visibility.Visible } fe &&
_appBarButtonWrapper is { } wrapper)
native.SetTitle(null);
native.SetActionView(null);
native.SetIcon(null);
}
else
{
var iconOrContent = element.Icon ?? element.Content;
switch (iconOrContent)
{
_elementParent = element.Parent;
wrapper.Child = element;
element.SetParent(_elementParent);
native.SetActionView(wrapper);
case string text:
native.SetIcon(null);
native.SetActionView(null);
native.SetTitle(text);
break;

case FrameworkElement fe:
if (fe.Visibility == Visibility.Visible && _appBarButtonWrapper is { } wrapper)
{
wrapper.Child = element;

//Restore the original parent if any, as we
// want the DataContext to flow properly from the
// CommandBar.
element.SetParent(_elementParent);

native.SetIcon(null);
native.SetActionView(wrapper);
native.SetTitle(null);
}
break;

default:
native.SetIcon(null);
native.SetActionView(null);
native.SetTitle(null);
break;
}
}

Expand All @@ -171,19 +195,6 @@ protected override void Render()
var foregroundColor = foreground?.Color;
var foregroundOpacity = 0d;

if (native.Icon != null)
{
if (element.TryGetIconColor(out var iconColor))
{
foregroundOpacity = iconColor.A / 255d;
DrawableCompat.SetTint(native.Icon, (Android.Graphics.Color)iconColor);
}
else
{
DrawableCompat.SetTintList(native.Icon, null);
}
}

if (titleText != null)
{
if (foregroundColor != null)
Expand All @@ -210,34 +221,8 @@ protected override void Render()
var opacity = element.Opacity;
var finalOpacity = isEnabledOpacity * foregroundOpacity * opacity;
var alpha = (int)(finalOpacity * 255);
native.Icon?.SetAlpha(alpha);
}

bool TrySetIcon()
{
if (element?.Icon is { } icon)
{
if (icon is BitmapIcon bitmap)
{
if (bitmap.UriSource is { } uriSource)
{
var drawable = DrawableHelper.FromUri(uriSource);
native.SetIcon(drawable);
}

return true;
}
else
{
this.Log().WarnIfEnabled(() => $"{icon.GetType().Name ?? "FontIcon, PathIcon and SymbolIcon"} are not supported. Use BitmapIcon instead with UriSource.");
native.SetIcon(null);
}
}

return false;
}
}

}

internal partial class AppBarButtonWrapper : Border
Expand Down
73 changes: 35 additions & 38 deletions src/Uno.Toolkit.UI/Controls/NavigationBar/AppBarButtonRenderer.iOS.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,47 +118,44 @@ protected override void Render()
var native = Native;
var element = Element ?? throw new InvalidOperationException("Element is null.");

native.Image = null;
native.ClearCustomView();
native.Title = element.Content is string content ? content : element.Label;
if (element.Icon is { } icon)
var iconOrContent = element.Icon ?? element.Content;
switch (iconOrContent)
{
switch (icon)
{
case BitmapIcon bitmap:
native.Image = ImageHelper.FromUri(bitmap.UriSource);
native.ClearCustomView();
break;

case FontIcon: // not supported
case PathIcon: // not supported
case SymbolIcon: // not supported
default:
this.Log().WarnIfEnabled(() => $"{icon.GetType().Name ?? "FontIcon, PathIcon and SymbolIcon"} are not supported. Use BitmapIcon instead with UriSource.");
native.Image = null;
native.ClearCustomView();
// iOS doesn't add the UIBarButtonItem to the native logical tree unless it has an Image or Title set.
// We default to an empty string to ensure it is added.
native.Title ??= string.Empty;
break;
}
}
else if (element.Content is FrameworkElement fe)
{
var currentParent = element.Parent;
_appBarButtonWrapper.Child = element;

//Restore the original parent if any, as we
//want the DataContext to flow properly from the
//NavigationBar.
element.SetParent(currentParent);
native.Image = null;
native.CustomView = fe.Visibility == Visibility.Visible ? _appBarButtonWrapper : null;
// iOS doesn't add the UIBarButtonItem to the native logical tree unless it has an Image or Title set.
// We default to an empty string to ensure it is added, in order to support late-bound Content.
native.Title ??= string.Empty;
case string text:
native.Image = null;
native.ClearCustomView();
native.Title = text;
break;

case FrameworkElement fe:
var currentParent = element.Parent;
_appBarButtonWrapper.Child = element;

//Restore the original parent if any, as we
// want the DataContext to flow properly from the
// CommandBar.
element.SetParent(currentParent);

native.Image = null;
native.CustomView = fe.Visibility == Visibility.Visible ? _appBarButtonWrapper : null;
// iOS doesn't add the UIBarButtonItem to the native logical tree unless it has an Image or Title set.
// We default to an empty string to ensure it is added, in order to support late-bound Content.
native.Title = string.Empty;
break;

default:
native.Image = null;
native.ClearCustomView();
// iOS doesn't add the UIBarButtonItem to the native logical tree unless it has an Image or Title set.
// We default to an empty string to ensure it is added.
native.Title = string.Empty;
break;
}

// Label
native.AccessibilityHint = element.Label;
native.AccessibilityLabel = element.Label;

// Foreground
if (element.TryGetIconColor(out var iconColor))
{
Expand Down
6 changes: 1 addition & 5 deletions 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 @@ -169,11 +169,7 @@ protected override void Render()
backItem.BackButtonTitle = mainCommand.Content as string ?? mainCommand.Label;
}

var mainCommandIcon = mainCommand.Icon is BitmapIcon bitmapIcon
? ImageHelper.FromUri(bitmapIcon.UriSource)?.ImageWithRenderingMode(UIImageRenderingMode.AlwaysOriginal)
: null;

if (mainCommandIcon is { } icon && string.IsNullOrEmpty(_backItem?.BackButtonTitle))
if (mainCommand.Icon is { } icon && string.IsNullOrEmpty(_backItem?.BackButtonTitle))
{
native.LeftBarButtonItem = mainCommand.GetOrAddDefaultRenderer().Native;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Uno.Toolkit.UITest/Controls/NavigationBar/Given_NavigationBar.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void NavBar_Can_Close_From_First_Page()
App.WaitForElementWithMessage("M3Page1NavBar");

PlatformHelpers.On(
iOS: () => App.FastTap("CloseIcon"),
iOS: () => App.FastTap("M3_NavBar_Close_Button"),
Android: () => App.FastTap(q => q.Marked("M3Page1NavBar").Descendant("AppCompatImageButton"))
);
;
Expand Down
Loading