diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Assets/Icons/AppIconFallback.svg b/src/modules/cmdpal/Microsoft.CmdPal.UI/Assets/Icons/AppIconFallback.svg new file mode 100644 index 0000000000..a14a09de99 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Assets/Icons/AppIconFallback.svg @@ -0,0 +1,3 @@ + + + diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/IconBox.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/IconBox.cs index 4f559eae62..d932cf1886 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/IconBox.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/IconBox.cs @@ -20,6 +20,7 @@ public partial class IconBox : ContentControl { private const double DefaultIconFontSize = 16.0; private static long _nextDiagnosticId; + private readonly IconPresentationState _presentation = new(); private double _lastScale; private ElementTheme _lastTheme; @@ -56,6 +57,33 @@ public partial class IconBox : ContentControl public static readonly DependencyProperty SourceProperty = DependencyProperty.Register(nameof(Source), typeof(IconSource), typeof(IconBox), new PropertyMetadata(null, OnSourcePropertyChanged)); + /// + /// Gets or sets the source displayed while is being resolved or cannot produce an icon. + /// A placement fallback takes precedence over a fallback supplied by the source provider. + /// + public IconSource? FallbackSource + { + get => (IconSource?)GetValue(FallbackSourceProperty); + set => SetValue(FallbackSourceProperty, value); + } + + public static readonly DependencyProperty FallbackSourceProperty = + DependencyProperty.Register(nameof(FallbackSource), typeof(IconSource), typeof(IconBox), new PropertyMetadata(null, OnFallbackSourcePropertyChanged)); + + /// + /// Gets or sets a value indicating whether an image-oriented request that resolves to a + /// font icon should use instead. This is intended for image + /// placements, such as application hero images, where a glyph is not appropriate. + /// + public bool PreferFallbackSourceForFontIcons + { + get => (bool)GetValue(PreferFallbackSourceForFontIconsProperty); + set => SetValue(PreferFallbackSourceForFontIconsProperty, value); + } + + public static readonly DependencyProperty PreferFallbackSourceForFontIconsProperty = + DependencyProperty.Register(nameof(PreferFallbackSourceForFontIcons), typeof(bool), typeof(IconBox), new PropertyMetadata(false, OnPreferFallbackSourceForFontIconsPropertyChanged)); + /// /// Gets or sets a value to use as the to retrieve an to set as the . /// @@ -443,6 +471,28 @@ public partial class IconBox : ContentControl } } + private static void OnFallbackSourcePropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) + { + if (d is not IconBox self) + { + return; + } + + self._presentation.PlacementFallback = e.NewValue as IconSource; + if (self.SourceKey is not null) + { + self.UpdatePresentedSource(); + } + } + + private static void OnPreferFallbackSourceForFontIconsPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) + { + if (d is IconBox self && self.SourceKey is not null) + { + self.UpdatePresentedSource(); + } + } + private static void OnSourceKeyPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) { if (d is not IconBox self) @@ -451,6 +501,7 @@ public partial class IconBox : ContentControl } self.AdvanceRequestVersion(); + self._presentation.BeginSourceChange(); if (e.NewValue is null) { @@ -459,6 +510,9 @@ public partial class IconBox : ContentControl return; } + // A recycled IconBox must stop presenting the preceding item's icon before + // the replacement request has a chance to yield or enter the load queue. + self.UpdatePresentedSource(); self.RequestRefresh(IconRequestReason.SourceChanged); } @@ -486,7 +540,13 @@ public partial class IconBox : ContentControl Diagnostics = diagnostics, }; iconBox.TrackActiveRequest(requestVersion, diagnostics, eventArgs); - await sourceRequested.InvokeAsync(iconBox, eventArgs); + var invocation = sourceRequested.InvokeAsync(iconBox, eventArgs); + if (!invocation.IsCompleted) + { + iconBox.SetRequestFallback(requestVersion, sourceKey, eventArgs.FallbackSource); + } + + await invocation; // After the await: // Is the icon we're looking up now, the one we still @@ -501,7 +561,9 @@ public partial class IconBox : ContentControl return; } - iconBox.Source = eventArgs.Value; + iconBox._presentation.SetRequestFallback(eventArgs.FallbackSource); + iconBox._presentation.SetResolvedSource(eventArgs.Value, eventArgs.ExpectsImageSource); + iconBox.UpdatePresentedSource(); diagnostics.Complete( eventArgs.Value is null ? IconRequestStatus.Empty : IconRequestStatus.Applied, eventArgs.Value); @@ -529,4 +591,29 @@ public partial class IconBox : ContentControl } } } + + private void SetRequestFallback(long requestVersion, object sourceKey, IconSource? fallbackSource) + { + if (requestVersion != _requestVersion || !ReferenceEquals(sourceKey, SourceKey)) + { + return; + } + + _presentation.SetRequestFallback(fallbackSource); + UpdatePresentedSource(); + } + + private void UpdatePresentedSource() + { + var resolvedSource = _presentation.ResolvedSource; + + // Replacing a valid glyph requires both opt-ins: the placement must prefer + // an image fallback, and the provider must identify this as an image request. + // This keeps app hero images image-only without replacing emoji or other glyph heroes. + var preferFallback = resolvedSource is null + || (PreferFallbackSourceForFontIcons && _presentation.ResolvedSourceExpectsImage && resolvedSource is FontIconSource) + || resolvedSource is BitmapIconSource { UriSource: null } + || resolvedSource is ImageIconSource { ImageSource: null }; + Source = _presentation.SelectSource(preferFallback); + } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/IconPresentationState`1.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/IconPresentationState`1.cs new file mode 100644 index 0000000000..46993f2c17 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/IconPresentationState`1.cs @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace Microsoft.CmdPal.UI.Controls; + +internal sealed class IconPresentationState + where T : class +{ + public T? PlacementFallback { get; set; } + + public T? RequestFallback { get; private set; } + + public T? ResolvedSource { get; private set; } + + public bool HasResolvedSource { get; private set; } + + public bool ResolvedSourceExpectsImage { get; private set; } + + public void BeginSourceChange() + { + RequestFallback = null; + ResolvedSource = null; + HasResolvedSource = false; + ResolvedSourceExpectsImage = false; + } + + public void SetRequestFallback(T? source) => RequestFallback = source; + + public void SetResolvedSource(T? source, bool expectsImageSource) + { + ResolvedSource = source; + HasResolvedSource = true; + ResolvedSourceExpectsImage = expectsImageSource; + } + + public T? SelectSource(bool preferFallbackForResolvedSource) + { + var fallback = PlacementFallback ?? RequestFallback; + return !HasResolvedSource || (preferFallbackForResolvedSource && fallback is not null) + ? fallback + : ResolvedSource; + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/SourceRequestedEventArgs.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/SourceRequestedEventArgs.cs index b320605669..8bb5711d56 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/SourceRequestedEventArgs.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/SourceRequestedEventArgs.cs @@ -20,6 +20,19 @@ public class SourceRequestedEventArgs(object? key, ElementTheme requestedTheme, public IconSource? Value { get; set; } + /// + /// Gets or sets an optional source to display while is being resolved. + /// Handlers should set this before their first asynchronous suspension so the control can present it immediately. + /// + public IconSource? FallbackSource { get; set; } + + /// + /// Gets or sets a value indicating whether this request is expected to resolve to an image. + /// This lets image-oriented placements reject a glyph produced by a final fallback path + /// without affecting ordinary glyph requests. + /// + internal bool ExpectsImageSource { get; set; } + public ElementTheme Theme => requestedTheme; public double Scale => scale; diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconProvider.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconProvider.cs index 8b3371c3dd..85ac92983d 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconProvider.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconProvider.cs @@ -5,7 +5,11 @@ using ManagedCommon; using Microsoft.CmdPal.UI.Controls; using Microsoft.CmdPal.UI.ViewModels; +using Microsoft.CommandPalette.Extensions.Toolkit; using Microsoft.Extensions.DependencyInjection; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Media.Imaging; namespace Microsoft.CmdPal.UI.Helpers; @@ -14,12 +18,15 @@ namespace Microsoft.CmdPal.UI.Helpers; /// public static partial class IconProvider { + private static readonly Uri AppIconFallbackUri = new("ms-appx:///Assets/Icons/AppIconFallback.svg"); + private static IIconSourceProvider _provider16 = null!; private static IIconSourceProvider _provider20 = null!; private static IIconSourceProvider _provider32 = null!; private static IIconSourceProvider _provider64 = null!; private static IIconSourceProvider _provider256 = null!; private static IIconSourceProvider _providerUnbound = null!; + private static ImageIconSource? _appIconFallbackSource; public static void Initialize(IServiceProvider serviceProvider) { @@ -42,22 +49,29 @@ public static partial class IconProvider try { - args.Value = args.Key switch + var iconData = args.Key switch { - IconDataViewModel iconData => await service.GetIconSource( + IconDataViewModel value => value, + IconInfoViewModel value => value.IconForTheme(args.Theme == ElementTheme.Light), + _ => null, + }; + if (iconData is not null && AppIconProtocol.IsProtocol(iconData.Icon)) + { + args.FallbackSource = _appIconFallbackSource ??= new ImageIconSource + { + ImageSource = new SvgImageSource(AppIconFallbackUri), + }; + args.ExpectsImageSource = true; + } + + args.Value = iconData is null + ? null + : await service.GetIconSource( iconData, args.Scale, args.Diagnostics, args, - args.Theme), - IconInfoViewModel iconInfo => await service.GetIconSource( - args.Theme == Microsoft.UI.Xaml.ElementTheme.Light ? iconInfo.Light : iconInfo.Dark, - args.Scale, - args.Diagnostics, - args, - args.Theme), - _ => null, - }; + args.Theme); } catch (Exception ex) { diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Microsoft.CmdPal.UI.csproj b/src/modules/cmdpal/Microsoft.CmdPal.UI/Microsoft.CmdPal.UI.csproj index 3499278a87..75088ee7dd 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Microsoft.CmdPal.UI.csproj +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Microsoft.CmdPal.UI.csproj @@ -234,6 +234,10 @@ PreserveNewest + + PreserveNewest + true + Never diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml index 4d9b0f2ce1..6b16b75691 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml @@ -473,10 +473,17 @@ HorizontalAlignment="Left" AutomationProperties.AccessibilityView="Raw" DiagnosticScope="HeroImage" + PreferFallbackSourceForFontIcons="True" RequestSite="Details" SourceKey="{x:Bind ViewModel.Details.HeroImage, Mode=OneWay}" SourceRequested="{x:Bind help:IconProvider.SourceRequested64}" - Visibility="{x:Bind HasHeroImage, Mode=OneWay}" /> + Visibility="{x:Bind HasHeroImage, Mode=OneWay}"> + + + + + SourceRequested="{x:Bind helpers:IconProvider.SourceRequested20}"> + + + + diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml index e4e324cca6..7291cf2924 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml @@ -156,7 +156,13 @@ DiagnosticScope="ExtensionList" RequestSite="Settings" SourceKey="{x:Bind Icon, Mode=OneWay}" - SourceRequested="{x:Bind helpers:IconProvider.SourceRequested20}" /> + SourceRequested="{x:Bind helpers:IconProvider.SourceRequested20}"> + + + + + { + PlacementFallback = "placement", + }; + state.SetResolvedSource("preceding item", expectsImageSource: true); + + Assert.IsTrue(state.ResolvedSourceExpectsImage); + + state.BeginSourceChange(); + + Assert.AreEqual("placement", state.SelectSource(preferFallbackForResolvedSource: false)); + Assert.IsFalse(state.HasResolvedSource); + Assert.IsFalse(state.ResolvedSourceExpectsImage); + } + + [TestMethod] + public void PlacementFallbackTakesPrecedenceOverRequestFallback() + { + var state = new IconPresentationState + { + PlacementFallback = "placement", + }; + state.SetRequestFallback("request"); + + Assert.AreEqual("placement", state.SelectSource(preferFallbackForResolvedSource: false)); + } + + [TestMethod] + public void ResolvedSourceReplacesFallbackUnlessPlacementPrefersFallback() + { + var state = new IconPresentationState + { + PlacementFallback = "fallback", + }; + state.SetResolvedSource("resolved", expectsImageSource: false); + + Assert.AreEqual("resolved", state.SelectSource(preferFallbackForResolvedSource: false)); + Assert.AreEqual("fallback", state.SelectSource(preferFallbackForResolvedSource: true)); + } + + [TestMethod] + public void NewSourceDoesNotRetainPreviousRequestFallback() + { + var state = new IconPresentationState(); + state.SetRequestFallback("preceding request"); + + state.BeginSourceChange(); + + Assert.IsNull(state.RequestFallback); + Assert.IsNull(state.SelectSource(preferFallbackForResolvedSource: false)); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/Microsoft.CmdPal.UI.UnitTests.csproj b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/Microsoft.CmdPal.UI.UnitTests.csproj index 62a724042c..505139e1d4 100644 --- a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/Microsoft.CmdPal.UI.UnitTests.csproj +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/Microsoft.CmdPal.UI.UnitTests.csproj @@ -28,6 +28,7 @@ +