Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,5 @@ private void Tabs_TabDraggedOutside(object sender, TabDraggedOutsideEventArgs e)
TabViewNotification.Show("Tore Tab '" + str + "' Outside of TabView.", 2000);
}
#pragma warning restore CS0618 // Type or member is obsolete
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<controls:TokenizingTextBox
x:Name="TokenBox"
PlaceholderText="Add Actions"
QueryIcon="{ex:SymbolIconSource Glyph=Setting, FontSize=16}"
QueryIcon="{ex:SymbolIconSource Glyph=Setting}"
MaxHeight="104"
HorizontalAlignment="Stretch"
TextMemberPath="Text"
Expand Down Expand Up @@ -52,7 +52,7 @@
PlaceholderText="Select Names"
MaxHeight="104"
HorizontalAlignment="Stretch"
QueryIcon="{ex:SymbolIconSource Glyph=Find, FontSize=16}"
QueryIcon="{ex:SymbolIconSource Glyph=Find}"
TextMemberPath="Text"
TokenDelimiter=","
IsItemClickEnabled="True">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
using System.Threading.Tasks;
using Microsoft.Toolkit.Uwp.Deferred;
using Microsoft.Toolkit.Uwp.Extensions;
using Microsoft.Toolkit.Uwp.UI.Extensions;
using Microsoft.Toolkit.Uwp.UI.Helpers;
using Windows.System;
using Windows.UI.Core;
using Windows.UI.Xaml;
Expand Down Expand Up @@ -435,6 +437,8 @@ internal async Task AddTokenAsync(object data, bool? atEnd = null)
last?._autoSuggestTextBox.Focus(FocusState.Keyboard);

TokenItemAdded?.Invoke(this, data);

GuardAgainstPlaceholderTextLayoutIssue();
}

private void UpdateCurrentTextEdit(PretokenStringContainer edit)
Expand Down Expand Up @@ -475,7 +479,34 @@ private async Task<bool> RemoveTokenAsync(TokenizingTextBoxItem item, object dat

TokenItemRemoved?.Invoke(this, data);

GuardAgainstPlaceholderTextLayoutIssue();

return true;
}

private void GuardAgainstPlaceholderTextLayoutIssue()
{
// If the *PlaceholderText is visible* on the last AutoSuggestBox, it can incorrectly layout itself
// when the *ASB has focus*. We think this is an optimization in the platform, but haven't been able to
// isolate a straight-reproduction of this issue outside of this control (though we have eliminated
// most Toolkit influences like ASB/TextBox Style, the InterspersedObservableCollection, etc...).
// The only Toolkit component involved here should be WrapPanel (which is a straight-forward Panel).
// We also know the ASB itself is adjusting it's size correctly, it's the inner component.
//
// To combat this issue:
// We toggle the visibility of the Placeholder ContentControl in order to force it's layout to update properly
var placeholder = ContainerFromItem(_lastTextEdit).FindDescendantByName("PlaceholderTextContentPresenter");

if (placeholder?.Visibility == Visibility.Visible)
{
placeholder.Visibility = Visibility.Collapsed;

// After we ensure we've hid the control, make it visible again (this is inperceptable to the user).
_ = CompositionTargetHelper.ExecuteAfterCompositionRenderingAsync(() =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

{
placeholder.Visibility = Visibility.Visible;
});
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<Thickness x:Key="TokenizingTextBoxPresenterMargin">0,0,6,0</Thickness>
<x:Double x:Key="TokenizingTextBoxTokenSpacing">2</x:Double>
<Thickness x:Key="TextControlBorderThemeThicknessFocused">2</Thickness>

<controls:TokenizingTextBoxStyleSelector x:Key="TokenizingTextBoxStyleSelector"
TextStyle="{StaticResource TokenizingTextBoxItemTextStyle}"
TokenStyle="{StaticResource TokenizingTextBoxItemTokenStyle}"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ private void OnApplyTemplateAutoSuggestBox(AutoSuggestBox auto)
// Setup a binding to the QueryIcon of the Parent if we're the last box.
if (Content is PretokenStringContainer str && str.IsLast)
{
// Workaround for https://github.com/microsoft/microsoft-ui-xaml/issues/2568
if (Owner.QueryIcon is FontIconSource fis &&
fis.ReadLocalValue(FontIconSource.FontSizeProperty) == DependencyProperty.UnsetValue)
{
// This can be expensive, could we optimize?
// Also, this is changing the FontSize on the IconSource (which could be shared?)
fis.FontSize = Owner.TryFindResource("TokenizingTextBoxIconFontSize") as double? ?? 16;
}

var iconBinding = new Binding()
{
Source = Owner,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

<!--<Thickness x:Key="ButtonPadding">8,4,8,5</Thickness> Not sure if we'll also need this later-->
<Thickness x:Key="TextControlThemePadding">10,3,6,6</Thickness><!-- Need local copy of this, as including WinUI overrides this to something that adds too much padding for our inner box -->

<x:Double x:Key="TokenizingTextBoxIconFontSize">16</x:Double><!-- 'Example' hard-coded currently in link:TokenizingTextBoxItem.AutoSuggestBox.cs#L110, but can be overwritten in developer's resources as 'normal' -->

<!--#region Button Styles -->
<Style x:Key="TokenizingTextBoxDeleteButtonStyle"
TargetType="Button">
Expand All @@ -15,12 +16,14 @@
Background="{ThemeResource TextControlButtonBackground}"
BorderBrush="{ThemeResource TextControlButtonBorderBrush}"
BorderThickness="{TemplateBinding BorderThickness}">
<!-- FontSize is ignored here, see https://github.com/microsoft/microsoft-ui-xaml/issues/2568 -->
<!-- Set in code-behind link:TokenizingTextBoxItem.AutoSuggestBox.cs#L104 -->
<TextBlock x:Name="GlyphElement"
HorizontalAlignment="Center"
VerticalAlignment="Center"
AutomationProperties.AccessibilityView="Raw"
FontFamily="{ThemeResource SymbolThemeFontFamily}"
FontSize="{ThemeResource AutoSuggestBoxIconFontSize}"
FontSize="{ThemeResource TokenizingTextBoxIconFontSize}"
FontStyle="Normal"
Foreground="{ThemeResource TextControlButtonForeground}"
Text="&#xE10A;" />
Expand Down Expand Up @@ -290,17 +293,12 @@
<Style x:Key="TokenizingTextBoxItemTextStyle" TargetType="controls:TokenizingTextBoxItem">
<Setter Property="Background" Value="{ThemeResource SystemControlBackgroundChromeMediumLowBrush}" />
<Setter Property="BorderBrush" Value="{ThemeResource SystemControlTransparentBrush}" />
<!-- <Setter Property="BorderThickness" Value="{StaticResource TokenizingTextBoxItemBorderThickness}" />
<Setter Property="HorizontalContentAlignment" Value="{StaticResource TokenizingTextBoxItemHorizontalContentAlignment}" />
<Setter Property="Padding" Value="{StaticResource TokenizingTextBoxItemPadding}" />
<Setter Property="VerticalContentAlignment" Value="{StaticResource TokenizingTextBoxItemVerticalContentAlignment}" />-->
<Setter Property="IsTabStop" Value="False" />
<Setter Property="UseSystemFocusVisuals" Value="False" />
<Setter Property="MinWidth" Value="128" />
<Setter Property="Template">
<Setter.Value>
<ControlTemplate TargetType="controls:TokenizingTextBoxItem">
<!-- TODO: Thinking we should just give a reference to the parent for each item when we construct the container? -->
<AutoSuggestBox Name="PART_AutoSuggestBox"
UpdateTextOnSelect="False"
DisplayMemberPath="{Binding Path=Owner.DisplayMemberPath, RelativeSource={RelativeSource Mode=TemplatedParent}}"
Expand All @@ -311,7 +309,6 @@
Style="{StaticResource SystemAutoSuggestBoxStyle}"
Text="{Binding Text, Mode=TwoWay}"
TextBoxStyle="{StaticResource TokenizingTextBoxTextBoxStyle}"/>
<!-- TODO: Visual State to style autosuggestbox based on focus? -->
</ControlTemplate>
</Setter.Value>
</Setter>
Expand Down
32 changes: 32 additions & 0 deletions Microsoft.Toolkit.Uwp.UI/Extensions/Tree/LogicalTree.cs
Original file line number Diff line number Diff line change
Expand Up @@ -286,5 +286,37 @@ private static string ContentPropertySearch(Type type)

return ContentPropertySearch(type.GetTypeInfo().BaseType);
}

/// <summary>
/// Provides a WPF compatible version of TryFindResource to provide a static resource lookup.
/// If the key is not found in the current element's resources, the logical tree is then searched element-by-element to look for the resource in each element's resources.
/// If none of the elements contain the resource, the Application's resources are then searched.
/// <seealso href="https://docs.microsoft.com/en-us/dotnet/api/system.windows.frameworkelement.tryfindresource"/>
/// <seealso href="https://docs.microsoft.com/en-us/dotnet/desktop-wpf/fundamentals/xaml-resources-define#static-resource-lookup-behavior"/>
/// </summary>
/// <param name="start"><see cref="FrameworkElement"/> to start searching for Resource.</param>
/// <param name="resourceKey">Key to search for.</param>
/// <returns>Requested resource or null.</returns>
public static object TryFindResource(this FrameworkElement start, object resourceKey)
{
object value = null;
var current = start;

// Look in our dictionary and then walk-up parents
while (current != null)
{
if (current.Resources?.TryGetValue(resourceKey, out value) == true)
{
return value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not keen on multiple return points from a function as it can make maintenance more tricky.
however this looks pretty simple and easy to read in this case.

}

current = current.Parent as FrameworkElement;
}

// Finally try application resources.
Application.Current?.Resources?.TryGetValue(resourceKey, out value);

return value;
}
}
}
50 changes: 50 additions & 0 deletions Microsoft.Toolkit.Uwp.UI/Helpers/CompositionTargetHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Threading.Tasks;
using Microsoft.Toolkit.Diagnostics;
using Windows.UI.Xaml.Media;

namespace Microsoft.Toolkit.Uwp.UI.Helpers
{
/// <summary>
/// Provides helpers for the <see cref="CompositionTarget"/> class.
/// </summary>
public static class CompositionTargetHelper
{
/// <summary>
/// Provides a method to execute code after the rendering pass is completed.
/// <seealso href="https://github.com/microsoft/microsoft-ui-xaml/blob/c045cde57c5c754683d674634a0baccda34d58c4/dev/dll/SharedHelpers.cpp#L399"/>
/// </summary>
/// <param name="action">Action to be executed after render pass</param>
/// <returns>Awaitable Task</returns>
public static Task<bool> ExecuteAfterCompositionRenderingAsync(Action action)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like a Task based version (for the action parameter, Func) so the returned task from this method can await for the action to executed. Not sure its part of this PR, just something I noticed when I was reviewing this.

{
Guard.IsNotNull(action, nameof(action));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could/should this just be a no-op (Task.CompletedTask) in this case? Do we want to throw?


var taskCompletionSource = new TaskCompletionSource<bool>();

try
{
void Callback(object sender, object args)
{
CompositionTarget.Rendering -= Callback;

action();

taskCompletionSource.SetResult(true);
}

CompositionTarget.Rendering += Callback;
}
catch (Exception e)
{
taskCompletionSource.SetException(e); // Note this can just sometimes be a wrong thread exception, see WinUI function notes.
}

return taskCompletionSource.Task;
}
}
}