Skip to content

Commit

Permalink
Merge pull request unoplatform#15092 from ramezgerges/skia_textbox_tests
Browse files Browse the repository at this point in the history
fix: skia textbox overlay not disappearing on unfocus when FeatureConfiguration changes
  • Loading branch information
MartinZikmund authored Jan 22, 2024
2 parents 2770c88 + 4dca7b6 commit 70255bb
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.UI.Xaml.Media;
using FluentAssertions;
using MUXControlsTestApp.Utilities;
using Uno.Disposables;
using Uno.Extensions;
using Uno.UI.RuntimeTests.Helpers;
using Uno.UI.Xaml.Core;
Expand Down Expand Up @@ -3182,6 +3183,34 @@ public async Task When_Paste_Does_Not_Change_Text()
Assert.AreEqual(0, SUT.SelectionLength);
}

[TestMethod]
public async Task When_FeatureConfiguration_Changes()
{
var useOverlay = FeatureConfiguration.TextBox.UseOverlayOnSkia;
using var _ = Disposable.Create(() => FeatureConfiguration.TextBox.UseOverlayOnSkia = useOverlay);

FeatureConfiguration.TextBox.UseOverlayOnSkia = true;

var SUT = new TextBox
{
Width = 150,
Text = "hello world"
};

await UITestHelper.Load(SUT);

SUT.Focus(FocusState.Programmatic);
await WindowHelper.WaitForIdle();

Assert.AreEqual(0, SUT.TextBoxView.DisplayBlock.Opacity);

FeatureConfiguration.TextBox.UseOverlayOnSkia = false;

await UITestHelper.Load(new Button()); // a random control to unload SUT

Assert.AreEqual(1, SUT.TextBoxView.DisplayBlock.Opacity);
}

private class TextBoxFeatureConfigDisposable : IDisposable
{
private bool _useOverlay;
Expand Down
15 changes: 6 additions & 9 deletions src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,6 @@ public TextBox()
#endif
}

private bool IsSkiaTextBox =>
#if __SKIA__
!FeatureConfiguration.TextBox.UseOverlayOnSkia;
#else
false;
#endif

private protected override void OnLoaded()
{
base.OnLoaded();
Expand Down Expand Up @@ -162,7 +155,9 @@ private protected override void OnLoaded()
// When support for TemplateBinding for attached DPs was added, TextBox broke (test: TextBox_AutoGrow_Vertically_Wrapping_Test) because of
// change in the values of these properties. The following code serves as a workaround to set the values to what they used to be
// before the support for TemplateBinding for attached DPs.
if (!IsSkiaTextBox)
#if __SKIA__
if (!_isSkiaTextBox)
#endif
{
scrollViewer.HorizontalScrollMode = ScrollMode.Enabled; // The template sets this to Auto
scrollViewer.VerticalScrollMode = ScrollMode.Enabled; // The template sets this to Auto
Expand Down Expand Up @@ -379,10 +374,12 @@ private object CoerceText(object baseValue)
{
baseString = GetFirstLine(baseString);
}
else if (IsSkiaTextBox)
#if __SKIA__
else if (_isSkiaTextBox)
{
baseString = baseString.Replace("\r\n", "\r").Replace("\n", "\r");
}
#endif

var args = new TextBoxBeforeTextChangingEventArgs(baseString);
BeforeTextChanging?.Invoke(this, args);
Expand Down
38 changes: 20 additions & 18 deletions src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.skia.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ private enum ContextMenuItem
SelectAll
}

private readonly bool _isSkiaTextBox = !FeatureConfiguration.TextBox.UseOverlayOnSkia;

private TextBoxView _textBoxView;

private readonly Rectangle _caretRect = new Rectangle { Fill = new SolidColorBrush(Colors.Black) };
Expand Down Expand Up @@ -116,7 +118,7 @@ private void TrySetCurrentlyTyping(bool newValue)
}
else
{
global::System.Diagnostics.Debug.Assert(!IsSkiaTextBox || _selection.length == 0);
global::System.Diagnostics.Debug.Assert(!_isSkiaTextBox || _selection.length == 0);
_historyIndex++;
_history.RemoveAllAt(_historyIndex);
_history.Add(new HistoryRecord(
Expand Down Expand Up @@ -169,7 +171,7 @@ private void UpdateTextBoxView()
if (ContentElement != null)
{
var displayBlock = TextBoxView.DisplayBlock;
if (!IsSkiaTextBox)
if (!_isSkiaTextBox)
{
if (ContentElement.Content != displayBlock)
{
Expand Down Expand Up @@ -249,7 +251,7 @@ private void UpdateTextBoxView()

partial void OnFocusStateChangedPartial(FocusState focusState)
{
if (!IsSkiaTextBox)
if (!_isSkiaTextBox)
{
TextBoxView?.OnFocusStateChanged(focusState);
}
Expand All @@ -275,7 +277,7 @@ partial void SelectPartial(int start, int length)
TrySetCurrentlyTyping(false);
_selectionEndsAtTheStart = false;
_selection = (start, length);
if (!IsSkiaTextBox)
if (!_isSkiaTextBox)
{
TextBoxView?.Select(start, length);
}
Expand All @@ -293,19 +295,19 @@ partial void SelectPartial(int start, int length)

public int SelectionStart
{
get => IsSkiaTextBox ? _selection.start : TextBoxView?.GetSelectionStart() ?? 0;
get => _isSkiaTextBox ? _selection.start : TextBoxView?.GetSelectionStart() ?? 0;
set => Select(start: value, length: SelectionLength);
}

public int SelectionLength
{
get => IsSkiaTextBox ? _selection.length : TextBoxView?.GetSelectionLength() ?? 0;
get => _isSkiaTextBox ? _selection.length : TextBoxView?.GetSelectionLength() ?? 0;
set => Select(SelectionStart, value);
}

internal void UpdateDisplaySelection()
{
if (IsSkiaTextBox && TextBoxView?.DisplayBlock.Inlines is { } inlines)
if (_isSkiaTextBox && TextBoxView?.DisplayBlock.Inlines is { } inlines)
{
inlines.Selection = (SelectionStart, SelectionStart + SelectionLength);
inlines.RenderSelection = FocusState != FocusState.Unfocused || (_contextMenu?.IsOpen ?? false);
Expand All @@ -316,7 +318,7 @@ internal void UpdateDisplaySelection()

private void UpdateScrolling()
{
if (IsSkiaTextBox && _contentElement is ScrollViewer sv)
if (_isSkiaTextBox && _contentElement is ScrollViewer sv)
{
var selectionEnd = _selectionEndsAtTheStart ? _selection.start : _selection.start + _selection.length;

Expand All @@ -335,7 +337,7 @@ private void UpdateScrolling()

partial void OnKeyDownPartial(KeyRoutedEventArgs args)
{
if (!IsSkiaTextBox)
if (!_isSkiaTextBox)
{
OnKeyDownInternal(args);
return;
Expand Down Expand Up @@ -788,7 +790,7 @@ protected override void OnPointerMoved(PointerRoutedEventArgs e)
base.OnPointerMoved(e);
e.Handled = true;

if (IsSkiaTextBox && _isPressed)
if (_isSkiaTextBox && _isPressed)
{
var displayBlock = TextBoxView.DisplayBlock;
var point = e.GetCurrentPoint(displayBlock);
Expand Down Expand Up @@ -831,7 +833,7 @@ protected override void OnRightTapped(RightTappedRoutedEventArgs e)
base.OnRightTapped(e);
e.Handled = true;

if (IsSkiaTextBox)
if (_isSkiaTextBox)
{
if (_contextMenu is null)
{
Expand Down Expand Up @@ -896,7 +898,7 @@ private static bool IsMultiTapGesture((ulong id, ulong ts, Point position) previ
partial void OnPointerPressedPartial(PointerRoutedEventArgs args)
{
TrySetCurrentlyTyping(false);
if (IsSkiaTextBox
if (_isSkiaTextBox
&& args.GetCurrentPoint(null) is var currentPoint
&& (!currentPoint.Properties.IsRightButtonPressed || SelectionLength == 0))
{
Expand Down Expand Up @@ -1164,7 +1166,7 @@ private int EndOfLine(int i)

partial void OnTextChangedPartial()
{
if (IsSkiaTextBox)
if (_isSkiaTextBox)
{
if (_pendingSelection is { } selection)
{
Expand All @@ -1184,7 +1186,7 @@ partial void OnTextChangedPartial()

partial void OnFocusStateChangedPartial2(FocusState focusState)
{
if (IsSkiaTextBox)
if (_isSkiaTextBox)
{
// this is needed so that we UpdateScrolling after the button appears/disappears.
UpdateLayout();
Expand All @@ -1197,7 +1199,7 @@ partial void OnFocusStateChangedPartial2(FocusState focusState)

partial void PasteFromClipboardPartial(string clipboardText, int selectionStart, int selectionLength, string newText)
{
if (IsSkiaTextBox)
if (_isSkiaTextBox)
{
if (_currentlyTyping)
{
Expand All @@ -1224,7 +1226,7 @@ partial void PasteFromClipboardPartial(string clipboardText, int selectionStart,

partial void CutSelectionToClipboardPartial()
{
if (IsSkiaTextBox)
if (_isSkiaTextBox)
{
if (_currentlyTyping)
{
Expand Down Expand Up @@ -1270,7 +1272,7 @@ private void CommitAction(TextBoxAction action)

public void Undo()
{
if (!IsSkiaTextBox)
if (!_isSkiaTextBox)
{
return;
}
Expand Down Expand Up @@ -1310,7 +1312,7 @@ public void Undo()

public void Redo()
{
if (!IsSkiaTextBox)
if (!_isSkiaTextBox)
{
return;
}
Expand Down
7 changes: 4 additions & 3 deletions src/Uno.UI/UI/Xaml/Controls/TextBox/TextBoxView.skia.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ internal class TextBoxView
private readonly WeakReference<TextBox> _textBox;
private readonly bool _isPasswordBox;
private bool _isPasswordRevealed;
private readonly bool _isSkiaTextBox = !FeatureConfiguration.TextBox.UseOverlayOnSkia;

public TextBoxView(TextBox textBox)
{
Expand All @@ -25,7 +26,7 @@ public TextBoxView(TextBox textBox)

_textBox = new WeakReference<TextBox>(textBox);
_isPasswordBox = textBox is PasswordBox;
if (FeatureConfiguration.TextBox.UseOverlayOnSkia && !ApiExtensibility.CreateInstance(this, out _textBoxExtension))
if (!_isSkiaTextBox && !ApiExtensibility.CreateInstance(this, out _textBoxExtension))
{
if (this.Log().IsEnabled(LogLevel.Warning))
{
Expand Down Expand Up @@ -116,7 +117,7 @@ internal void OnSelectionHighlightColorChanged(SolidColorBrush brush)

internal void OnFocusStateChanged(FocusState focusState)
{
if (!FeatureConfiguration.TextBox.UseOverlayOnSkia)
if (_isSkiaTextBox)
{
return;
}
Expand Down Expand Up @@ -192,7 +193,7 @@ private void SetDisplayBlockText(string text)
DisplayBlock.Text = text;
}

if (!FeatureConfiguration.TextBox.UseOverlayOnSkia)
if (_isSkiaTextBox)
{
TextBox?.ContentElement?.InvalidateMeasure();
TextBox?.UpdateLayout();
Expand Down

0 comments on commit 70255bb

Please sign in to comment.