From 316d1414b38f7bc21dd7f70aa9499f4fc35fe139 Mon Sep 17 00:00:00 2001 From: Julien Lebosquain Date: Tue, 28 Nov 2023 05:47:29 +0100 Subject: [PATCH 01/30] Update XamlX (#13755) --- src/Markup/Avalonia.Markup.Xaml.Loader/xamlil.github | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Markup/Avalonia.Markup.Xaml.Loader/xamlil.github b/src/Markup/Avalonia.Markup.Xaml.Loader/xamlil.github index b7ed273273..320e57fa1c 160000 --- a/src/Markup/Avalonia.Markup.Xaml.Loader/xamlil.github +++ b/src/Markup/Avalonia.Markup.Xaml.Loader/xamlil.github @@ -1 +1 @@ -Subproject commit b7ed273273949a5dd9f01e682ab97f61b43697ad +Subproject commit 320e57fa1c95d10de33d9a96a323d1bc27563fa1 From b630f21b7e8c940f51fa341d575027041b92bd96 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 28 Nov 2023 22:40:37 +0100 Subject: [PATCH 02/30] Fix overlapping items in `VirtualizingStackPanel` (#13765) * Added failing test for #12744. * Inserting items makes the StartU unstable. This ports the fix in https://github.com/AvaloniaUI/Avalonia.Controls.TreeDataGrid/pull/229 to Avalonia. Fixes #12744 --- .../Utils/RealizedStackElements.cs | 1 + .../VirtualizingStackPanelTests.cs | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/Avalonia.Controls/Utils/RealizedStackElements.cs b/src/Avalonia.Controls/Utils/RealizedStackElements.cs index 11bbaa11c4..18cba5b123 100644 --- a/src/Avalonia.Controls/Utils/RealizedStackElements.cs +++ b/src/Avalonia.Controls/Utils/RealizedStackElements.cs @@ -294,6 +294,7 @@ namespace Avalonia.Controls.Utils { // The insertion point was before the first element, update the first index. _firstIndex += count; + _startUUnstable = true; } else { diff --git a/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs b/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs index 4cbf8d2142..d0e139bedf 100644 --- a/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs @@ -843,6 +843,28 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(1, panel.VisualChildren.Count); } + [Fact] + public void Inserting_Item_Before_Viewport_Preserves_FirstRealizedIndex() + { + // Issue #12744 + using var app = App(); + var (target, scroll, itemsControl) = CreateTarget(); + var items = (IList)itemsControl.ItemsSource!; + + // Scroll down 20 items. + scroll.Offset = new Vector(0, 200); + target.UpdateLayout(); + Assert.Equal(20, target.FirstRealizedIndex); + + // Insert an item at the beginning. + items.Insert(0, "New Item"); + target.UpdateLayout(); + + // The first realized index should still be 20 as the scroll should be unchanged. + Assert.Equal(20, target.FirstRealizedIndex); + Assert.Equal(new(0, 200), scroll.Offset); + } + private static IReadOnlyList GetRealizedIndexes(VirtualizingStackPanel target, ItemsControl itemsControl) { return target.GetRealizedElements() From 6d1fd9f563574178c4f1d28542ada5dc39eeeeb6 Mon Sep 17 00:00:00 2001 From: Lubomir Tetak <50887170+ltetak@users.noreply.github.com> Date: Wed, 29 Nov 2023 06:50:03 +0100 Subject: [PATCH 03/30] Data grid handled events (#13680) * DataGridCell - ignore handled events * DataGridCell - ignore handled events * revert --- src/Avalonia.Controls.DataGrid/DataGridCell.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls.DataGrid/DataGridCell.cs b/src/Avalonia.Controls.DataGrid/DataGridCell.cs index 599bea056b..616fae831a 100644 --- a/src/Avalonia.Controls.DataGrid/DataGridCell.cs +++ b/src/Avalonia.Controls.DataGrid/DataGridCell.cs @@ -169,9 +169,13 @@ namespace Avalonia.Controls return; } OwningGrid.OnCellPointerPressed(new DataGridCellPointerPressedEventArgs(this, OwningRow, OwningColumn, e)); + if (e.Handled) + { + return; + } if (e.GetCurrentPoint(this).Properties.IsLeftButtonPressed) { - if (!e.Handled && OwningGrid.IsTabStop) + if (OwningGrid.IsTabStop) { OwningGrid.Focus(); } @@ -191,7 +195,7 @@ namespace Avalonia.Controls } else if (e.GetCurrentPoint(this).Properties.IsRightButtonPressed) { - if (!e.Handled && OwningGrid.IsTabStop) + if (OwningGrid.IsTabStop) { OwningGrid.Focus(); } From b84470bf3df62157a2573e05282b3fddb57e800d Mon Sep 17 00:00:00 2001 From: IanRawley <132860927+IanRawley@users.noreply.github.com> Date: Wed, 29 Nov 2023 21:45:29 +1300 Subject: [PATCH 04/30] Fix #13474 by partially reverting #12883 (#13663) * Failing unit test for #13474 * Fix for #13474 by reverting changes to ListBoxItem from #12883 * Version 2 of fix, only marks event handled for Mouse input. * Version 3. Re-raise the event, but backup the source before doing so and restore it after. Closest in functionality to original, but preserves "correct" event source in order to allow Tapped events. --------- Co-authored-by: Jumar Macato <16554748+jmacato@users.noreply.github.com> --- src/Avalonia.Controls/ListBoxItem.cs | 4 ++ .../ListBoxTests.cs | 49 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/Avalonia.Controls/ListBoxItem.cs b/src/Avalonia.Controls/ListBoxItem.cs index aa95511524..5ee4854554 100644 --- a/src/Avalonia.Controls/ListBoxItem.cs +++ b/src/Avalonia.Controls/ListBoxItem.cs @@ -104,7 +104,11 @@ namespace Avalonia.Controls // As we only update selection from touch/pen on pointer release, we need to raise // the pointer event on the owner to trigger a commit. if (e.Pointer.Type != PointerType.Mouse) + { + var sourceBackup = e.Source; owner.RaiseEvent(e); + e.Source = sourceBackup; + } e.Handled = true; } diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index 56f20c6e8e..34ab712fe2 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -1289,6 +1289,55 @@ namespace Avalonia.Controls.UnitTests } } + [Fact] + public void ListBoxItem_Should_Not_Block_Tapped_Events() + { + + // #13474 + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + + Pointer _pointer = new Pointer(Pointer.GetNextFreeId(), PointerType.Touch, true); + ulong nextStamp = 1; + + var items = Enumerable.Range(0, 10).Select(x => $"Item {x}").ToArray(); + var target = new ListBox + { + Template = ListBoxTemplate(), + ItemsSource = items, + SelectionMode = SelectionMode.Toggle, + ItemTemplate = new FuncDataTemplate((x, _) => new TextBlock { Height = 10 }) + }; + + Prepare(target); + + var lbItems = target.GetLogicalChildren().OfType().ToArray(); + + var item = lbItems[0]; + + int tappedCount = 0; + target.Tapped += (s, e) => + { + tappedCount++; + }; + + _mouse.Click(item); + Assert.Equal(1, tappedCount); + + // Raise PointerPressed and PointerReleased events with the Left Button pressed. TouchTestHelper + // assumes no button pressed, which prevents it from generating Tapped events, or I would use that. + + item.RaiseEvent(new PointerPressedEventArgs(item, _pointer, (Visual)item, default, nextStamp++, + new PointerPointProperties(RawInputModifiers.None, PointerUpdateKind.LeftButtonPressed), KeyModifiers.None)); + + + item.RaiseEvent(new PointerReleasedEventArgs(item, _pointer, (Visual)item, default, nextStamp++, + PointerPointProperties.None, KeyModifiers.None, MouseButton.Left)); + + Assert.Equal(2, tappedCount); + } + } + private static void RaiseKeyEvent(Control target, Key key, KeyModifiers inputModifiers = 0) { target.RaiseEvent(new KeyEventArgs From 4d92e2231460397e821520a3e8cd12e651871192 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 29 Nov 2023 11:50:47 +0100 Subject: [PATCH 05/30] Fix `nth-child` styles on virtualized lists (#13770) * Don't alter state in properties. The `IValueEntry.HasValue` and `ValueFrame.IsActive` properties could alter state, which meant that when inspecting objects with these properties in a debugger, the state got altered by observing it. Make them methods. * Deleted unused file. * Add failing test for #12381. * Use the index from the event in EvaluateIsActive. Break the rules in this case. Fixes #12381 --------- Co-authored-by: Max Katz --- .../PropertyStore/BindingEntryBase.cs | 15 ++--- .../PropertyStore/IValueEntry.cs | 7 +- .../PropertyStore/ImmediateValueEntry.cs | 2 +- src/Avalonia.Base/PropertyStore/ValueFrame.cs | 2 +- src/Avalonia.Base/PropertyStore/ValueStore.cs | 6 +- .../Styling/Activators/NthChildActivator.cs | 24 +++---- .../Styling/PropertySetterTemplateInstance.cs | 2 +- src/Avalonia.Base/Styling/Setter.cs | 2 +- .../Styling/TestSelectors.cs | 17 ----- .../VirtualizingStackPanelTests.cs | 67 ++++++++++++++++++- 10 files changed, 94 insertions(+), 50 deletions(-) delete mode 100644 tests/Avalonia.Base.UnitTests/Styling/TestSelectors.cs diff --git a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs index a841803ee1..ef43720537 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs @@ -49,15 +49,6 @@ namespace Avalonia.PropertyStore _uncommon = new() { _hasDataValidation = true }; } - public bool HasValue - { - get - { - Start(produceValue: false); - return _hasValue; - } - } - public bool IsSubscribed => _subscription is not null; public AvaloniaProperty Property { get; } AvaloniaProperty IValueEntry.Property => Property; @@ -70,6 +61,12 @@ namespace Avalonia.PropertyStore BindingCompleted(); } + public bool HasValue() + { + Start(produceValue: false); + return _hasValue; + } + public TValue GetValue() { Start(produceValue: false); diff --git a/src/Avalonia.Base/PropertyStore/IValueEntry.cs b/src/Avalonia.Base/PropertyStore/IValueEntry.cs index 5898bef491..7ac7e83276 100644 --- a/src/Avalonia.Base/PropertyStore/IValueEntry.cs +++ b/src/Avalonia.Base/PropertyStore/IValueEntry.cs @@ -8,13 +8,16 @@ namespace Avalonia.PropertyStore /// internal interface IValueEntry { - bool HasValue { get; } - /// /// Gets the property that this value applies to. /// AvaloniaProperty Property { get; } + /// + /// Checks whether the entry has a value, starting the entry if necessary. + /// + bool HasValue(); + /// /// Gets the value associated with the entry. /// diff --git a/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs b/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs index 16b96eff5d..2d136fb1e5 100644 --- a/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs +++ b/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs @@ -19,13 +19,13 @@ namespace Avalonia.PropertyStore } public StyledProperty Property { get; } - public bool HasValue => true; AvaloniaProperty IValueEntry.Property => Property; public void Unsubscribe() { } public void Dispose() => _owner.OnEntryDisposed(this); + bool IValueEntry.HasValue() => true; object? IValueEntry.GetValue() => _value; T IValueEntry.GetValue() => _value; diff --git a/src/Avalonia.Base/PropertyStore/ValueFrame.cs b/src/Avalonia.Base/PropertyStore/ValueFrame.cs index 7a9d1bb13a..8ef68c651c 100644 --- a/src/Avalonia.Base/PropertyStore/ValueFrame.cs +++ b/src/Avalonia.Base/PropertyStore/ValueFrame.cs @@ -27,7 +27,7 @@ namespace Avalonia.PropertyStore } public int EntryCount => _index.Count; - public bool IsActive => GetIsActive(out _); + public bool IsActive() => GetIsActive(out _); public ValueStore? Owner => !_isShared ? _owner : throw new AvaloniaInternalException("Cannot get owner for shared ValueFrame"); public BindingPriority Priority { get; } diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index 2047f4d2d0..85adee2386 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -843,7 +843,7 @@ namespace Avalonia.PropertyStore // evaluated last as it can cause bindings to be subscribed. if (foundEntry && HasHigherPriority(entry!, priority, current, changedValueEntry) && - entry!.HasValue) + entry!.HasValue()) { if (current is not null) { @@ -911,7 +911,7 @@ namespace Avalonia.PropertyStore { var frame = _frames[i]; - if (!frame.IsActive) + if (!frame.IsActive()) continue; var priority = frame.Priority; @@ -927,7 +927,7 @@ namespace Avalonia.PropertyStore if (!HasHigherPriority(entry, priority, effectiveValue, changedValueEntry)) continue; - if (!entry.HasValue) + if (!entry.HasValue()) continue; if (effectiveValue is not null) diff --git a/src/Avalonia.Base/Styling/Activators/NthChildActivator.cs b/src/Avalonia.Base/Styling/Activators/NthChildActivator.cs index 8fe0bb2537..888a92fcdf 100644 --- a/src/Avalonia.Base/Styling/Activators/NthChildActivator.cs +++ b/src/Avalonia.Base/Styling/Activators/NthChildActivator.cs @@ -1,4 +1,5 @@ -using Avalonia.LogicalTree; +using System.Collections.Generic; +using Avalonia.LogicalTree; namespace Avalonia.Styling.Activators { @@ -12,7 +13,7 @@ namespace Avalonia.Styling.Activators private readonly int _step; private readonly int _offset; private readonly bool _reversed; - private int? _index; + private int _index = -1; public NthChildActivator( ILogical control, @@ -28,7 +29,7 @@ namespace Avalonia.Styling.Activators protected override bool EvaluateIsActive() { - var index = _index ?? _provider.GetChildIndex(_control); + var index = _index >= 0 ? _index : _provider.GetChildIndex(_control); return NthChildSelector.Evaluate(index, _provider, _step, _offset, _reversed).IsMatch; } @@ -50,26 +51,25 @@ namespace Avalonia.Styling.Activators // 3. We're a reversed (nth-last-child) selector and total count has changed switch (e.Action) { - // We're using the _index field to pass the index of the child to EvaluateIsActive - // *only* when the active state is re-evaluated via this event handler. The docs - // for EvaluateIsActive say: + // The docs for EvaluateIsActive say: // // > This method should read directly from its inputs and not rely on any // > subscriptions to fire in order to be up-to-date. // // Which is good advice in general, however in this case we need to break the rule - // and use the value from the event subscription instead of calling + // and use the value from the event subscription where possible instead of calling // IChildIndexProvider.GetChildIndex. This is because this event can be fired during - // the process of realizing an element of a virtualized list; in this case calling - // GetChildIndex may not return the correct index as the element isn't yet realized. + // the process of realizing an element of a virtualized list; in this case there may + // be more than one `nth-child` style on a the list item and when the other is + // re-evaluated calling GetChildIndex may not return the correct index as the element + // isn't yet realized. case ChildIndexChangedAction.ChildIndexChanged when e.Child == _control: - _index = e.Index; + _index = e.Index >= 0 ? e.Index : _provider.GetChildIndex(_control); ReevaluateIsActive(); - _index = null; break; case ChildIndexChangedAction.ChildIndexesReset: case ChildIndexChangedAction.TotalCountChanged when _reversed: - _index = null; + _index = _provider.GetChildIndex(_control); ReevaluateIsActive(); break; } diff --git a/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs b/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs index 7604c26244..d9a2f55da9 100644 --- a/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs +++ b/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs @@ -15,9 +15,9 @@ namespace Avalonia.Styling Property = property; } - public bool HasValue => true; public AvaloniaProperty Property { get; } + public bool HasValue() => true; public object? GetValue() => _value ??= _template.Build(); bool IValueEntry.GetDataValidationState(out BindingValueType state, out Exception? error) diff --git a/src/Avalonia.Base/Styling/Setter.cs b/src/Avalonia.Base/Styling/Setter.cs index 1ac26c79ec..f31394d6f7 100644 --- a/src/Avalonia.Base/Styling/Setter.cs +++ b/src/Avalonia.Base/Styling/Setter.cs @@ -58,7 +58,6 @@ namespace Avalonia.Styling } } - bool IValueEntry.HasValue => true; AvaloniaProperty IValueEntry.Property => EnsureProperty(); public override string ToString() => $"Setter: {Property} = {Value}"; @@ -88,6 +87,7 @@ namespace Avalonia.Styling return this; } + bool IValueEntry.HasValue() => true; object? IValueEntry.GetValue() => Value; bool IValueEntry.GetDataValidationState(out BindingValueType state, out Exception? error) diff --git a/tests/Avalonia.Base.UnitTests/Styling/TestSelectors.cs b/tests/Avalonia.Base.UnitTests/Styling/TestSelectors.cs deleted file mode 100644 index bd60275d50..0000000000 --- a/tests/Avalonia.Base.UnitTests/Styling/TestSelectors.cs +++ /dev/null @@ -1,17 +0,0 @@ -using System; -using Avalonia.Styling; - -namespace Avalonia.Base.UnitTests.Styling -{ - public static class TestSelectors - { - public static Selector SubscribeCheck(this Selector selector) - { - throw new NotImplementedException(); - //return new Selector( - // selector, - // control => new SelectorMatch(((TestControlBase)control).SubscribeCheckObservable), - // ""); - } - } -} diff --git a/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs b/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs index d0e139bedf..b61131a6ee 100644 --- a/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs @@ -843,6 +843,64 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(1, panel.VisualChildren.Count); } + [Fact] + public void Alternating_Backgrounds_Should_Be_Correct_After_Scrolling() + { + // Issue #12381. + static void AssertColors(VirtualizingStackPanel target) + { + var containers = target.GetRealizedContainers()! + .Cast() + .ToList(); + + for (var i = target.FirstRealizedIndex; i <= target.LastRealizedIndex; i++) + { + var container = Assert.IsType(target.ContainerFromIndex(i)); + var expectedBackground = i % 2 == 0 ? Colors.Green : Colors.Red; + var brush = Assert.IsAssignableFrom(container.Background); + + Assert.Equal(expectedBackground, brush.Color); + } + } + + using var app = App(); + var styles = new[] + { + new Style(x => x.OfType()) + { + Setters = { new Setter(ListBoxItem.BackgroundProperty, Brushes.White) }, + }, + new Style(x => x.OfType().NthChild(2, 1)) + { + Setters = { new Setter(ListBoxItem.BackgroundProperty, Brushes.Green) }, + }, + new Style(x => x.OfType().NthChild(2, 0)) + { + Setters = { new Setter(ListBoxItem.BackgroundProperty, Brushes.Red) }, + }, + }; + var (target, scroll, itemsControl) = CreateUnrootedTarget(); + + // We need to display an odd number of items to reproduce the issue. + var root = CreateRoot(itemsControl, clientSize: new(100, 90), styles: styles); + root.LayoutManager.ExecuteInitialLayoutPass(); + + var containers = target.GetRealizedContainers()! + .Cast() + .ToList(); + + Assert.Equal(0, target.FirstRealizedIndex); + Assert.Equal(8, target.LastRealizedIndex); + AssertColors(target); + + scroll.Offset = new Vector(0, 10); + target.UpdateLayout(); + + Assert.Equal(1, target.FirstRealizedIndex); + Assert.Equal(9, target.LastRealizedIndex); + AssertColors(target); + } + [Fact] public void Inserting_Item_Before_Viewport_Preserves_FirstRealizedIndex() { @@ -925,7 +983,7 @@ namespace Avalonia.Controls.UnitTests where T : ItemsControl, new() { var (target, scroll, itemsControl) = CreateUnrootedTarget(items, itemTemplate); - var root = CreateRoot(itemsControl, styles); + var root = CreateRoot(itemsControl, styles: styles); root.LayoutManager.ExecuteInitialLayoutPass(); @@ -964,10 +1022,13 @@ namespace Avalonia.Controls.UnitTests return (target, scroll, itemsControl); } - private static TestRoot CreateRoot(Control? child, IEnumerable - + +