From 86aad32bee708db2b93bcfdac87f1af620c089ca Mon Sep 17 00:00:00 2001 From: Luis von der Eltz Date: Thu, 20 Aug 2020 18:23:19 +0200 Subject: [PATCH 1/5] Adding support for regex filter of devtools properties --- .../ViewModels/ControlDetailsViewModel.cs | 31 ++++++------- .../ViewModels/TreePageViewModel.cs | 34 +++++++++++--- .../Diagnostics/Views/ControlDetailsView.xaml | 45 +++++++++++-------- 3 files changed, 67 insertions(+), 43 deletions(-) diff --git a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs index 4e7129da41..c8f618580b 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs +++ b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.ComponentModel; using System.Linq; +using System.Text.RegularExpressions; using Avalonia.Collections; using Avalonia.VisualTree; @@ -12,12 +13,13 @@ namespace Avalonia.Diagnostics.ViewModels private readonly IVisual _control; private readonly IDictionary> _propertyIndex; private AvaloniaPropertyViewModel _selectedProperty; - private string _propertyFilter; - public ControlDetailsViewModel(IVisual control, string propertyFilter) + public ControlDetailsViewModel(TreePageViewModel treePage, IVisual control) { _control = control; + TreePage = treePage; + var properties = GetAvaloniaProperties(control) .Concat(GetClrProperties(control)) .OrderBy(x => x, PropertyComparer.Instance) @@ -25,7 +27,6 @@ namespace Avalonia.Diagnostics.ViewModels .ToList(); _propertyIndex = properties.GroupBy(x => x.Key).ToDictionary(x => x.Key, x => x.ToList()); - _propertyFilter = propertyFilter; var view = new DataGridCollectionView(properties); view.GroupDescriptions.Add(new DataGridPathGroupDescription(nameof(AvaloniaPropertyViewModel.Group))); @@ -43,19 +44,9 @@ namespace Avalonia.Diagnostics.ViewModels } } - public DataGridCollectionView PropertiesView { get; } + public TreePageViewModel TreePage { get; } - public string PropertyFilter - { - get => _propertyFilter; - set - { - if (RaiseAndSetIfChanged(ref _propertyFilter, value)) - { - PropertiesView.Refresh(); - } - } - } + public DataGridCollectionView PropertiesView { get; } public AvaloniaPropertyViewModel SelectedProperty { @@ -137,9 +128,15 @@ namespace Avalonia.Diagnostics.ViewModels private bool FilterProperty(object arg) { - if (!string.IsNullOrWhiteSpace(PropertyFilter) && arg is PropertyViewModel property) + if (!string.IsNullOrWhiteSpace(TreePage.PropertyFilter) && arg is PropertyViewModel property) { - return property.Name.IndexOf(PropertyFilter, StringComparison.OrdinalIgnoreCase) != -1; + if (TreePage.UseRegexFilter) + { + var regex = new Regex(TreePage.PropertyFilter); + return regex.IsMatch(property.Name); + } + + return property.Name.IndexOf(TreePage.PropertyFilter, StringComparison.OrdinalIgnoreCase) != -1; } return true; diff --git a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs index ec48cff399..77594570ed 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs +++ b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs @@ -8,7 +8,8 @@ namespace Avalonia.Diagnostics.ViewModels { private TreeNode _selectedNode; private ControlDetailsViewModel _details; - private string _propertyFilter; + private string _propertyFilter = string.Empty; + private bool _useRegexFilter; public TreePageViewModel(TreeNode[] nodes) { @@ -34,15 +35,10 @@ namespace Avalonia.Diagnostics.ViewModels get => _selectedNode; private set { - if (Details != null) - { - _propertyFilter = Details.PropertyFilter; - } - if (RaiseAndSetIfChanged(ref _selectedNode, value)) { Details = value != null ? - new ControlDetailsViewModel(value.Visual, _propertyFilter) : + new ControlDetailsViewModel(this, value.Visual) : null; } } @@ -62,6 +58,30 @@ namespace Avalonia.Diagnostics.ViewModels } } + public string PropertyFilter + { + get => _propertyFilter; + set + { + if (RaiseAndSetIfChanged(ref _propertyFilter, value)) + { + Details.PropertiesView.Refresh(); + } + } + } + + public bool UseRegexFilter + { + get => _useRegexFilter; + set + { + if (RaiseAndSetIfChanged(ref _useRegexFilter, value)) + { + Details.PropertiesView.Refresh(); + } + } + } + public void Dispose() { foreach (var node in Nodes) diff --git a/src/Avalonia.Diagnostics/Diagnostics/Views/ControlDetailsView.xaml b/src/Avalonia.Diagnostics/Diagnostics/Views/ControlDetailsView.xaml index 29ce26fcdf..b2d3a8bddb 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/Views/ControlDetailsView.xaml +++ b/src/Avalonia.Diagnostics/Diagnostics/Views/ControlDetailsView.xaml @@ -2,24 +2,31 @@ xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:conv="clr-namespace:Avalonia.Diagnostics.Converters" x:Class="Avalonia.Diagnostics.Views.ControlDetailsView"> - - - - - - - - - - - - + + + + + + + + + + + + + + + From 2ab0f80459101628a00f310d86d10efc0f818c0c Mon Sep 17 00:00:00 2001 From: Luis von der Eltz Date: Thu, 20 Aug 2020 18:41:48 +0200 Subject: [PATCH 2/5] Reduce overhead of compiling regex Use INotifyDataErrorInfo to signal error --- .../ViewModels/ControlDetailsViewModel.cs | 3 +- .../ViewModels/TreePageViewModel.cs | 60 ++++++++++++++++--- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs index c8f618580b..b4243c6088 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs +++ b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs @@ -132,8 +132,7 @@ namespace Avalonia.Diagnostics.ViewModels { if (TreePage.UseRegexFilter) { - var regex = new Regex(TreePage.PropertyFilter); - return regex.IsMatch(property.Name); + return TreePage.FilterRegex?.IsMatch(property.Name) ?? true; } return property.Name.IndexOf(TreePage.PropertyFilter, StringComparison.OrdinalIgnoreCase) != -1; diff --git a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs index 77594570ed..da2fd25c0c 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs +++ b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs @@ -1,11 +1,16 @@ using System; +using System.Collections; +using System.Collections.Generic; +using System.ComponentModel; +using System.Text.RegularExpressions; using Avalonia.Controls; using Avalonia.VisualTree; namespace Avalonia.Diagnostics.ViewModels { - internal class TreePageViewModel : ViewModelBase, IDisposable + internal class TreePageViewModel : ViewModelBase, IDisposable, INotifyDataErrorInfo { + private readonly Dictionary _errors = new Dictionary(); private TreeNode _selectedNode; private ControlDetailsViewModel _details; private string _propertyFilter = string.Empty; @@ -14,17 +19,13 @@ namespace Avalonia.Diagnostics.ViewModels public TreePageViewModel(TreeNode[] nodes) { Nodes = nodes; - Selection = new SelectionModel - { - SingleSelect = true, - Source = Nodes - }; + Selection = new SelectionModel { SingleSelect = true, Source = Nodes }; Selection.SelectionChanged += (s, e) => { SelectedNode = (TreeNode)Selection.SelectedItem; }; - } + } public TreeNode[] Nodes { get; protected set; } @@ -58,6 +59,37 @@ namespace Avalonia.Diagnostics.ViewModels } } + public Regex FilterRegex { get; set; } + + private void UpdateFilterRegex() + { + void ClearError() + { + if (_errors.Remove(nameof(PropertyFilter))) + { + ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(nameof(PropertyFilter))); + } + } + + if (UseRegexFilter) + { + try + { + FilterRegex = new Regex(PropertyFilter); + ClearError(); + } + catch (Exception exception) + { + _errors[nameof(PropertyFilter)] = exception.Message; + ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(nameof(PropertyFilter))); + } + } + else + { + ClearError(); + } + } + public string PropertyFilter { get => _propertyFilter; @@ -65,6 +97,7 @@ namespace Avalonia.Diagnostics.ViewModels { if (RaiseAndSetIfChanged(ref _propertyFilter, value)) { + UpdateFilterRegex(); Details.PropertiesView.Refresh(); } } @@ -77,6 +110,7 @@ namespace Avalonia.Diagnostics.ViewModels { if (RaiseAndSetIfChanged(ref _useRegexFilter, value)) { + UpdateFilterRegex(); Details.PropertiesView.Refresh(); } } @@ -158,5 +192,17 @@ namespace Avalonia.Diagnostics.ViewModels return null; } + + public IEnumerable GetErrors(string propertyName) + { + if (_errors.TryGetValue(propertyName, out var error)) + { + yield return error; + } + } + + public bool HasErrors => _errors.Count > 0; + + public event EventHandler ErrorsChanged; } } From ba8e6f029ec3befb5514ee860627844e91cab984 Mon Sep 17 00:00:00 2001 From: Luis von der Eltz Date: Fri, 28 Aug 2020 11:43:11 +0200 Subject: [PATCH 3/5] Compile regex --- .../Diagnostics/ViewModels/ControlDetailsViewModel.cs | 1 - .../Diagnostics/ViewModels/TreePageViewModel.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs index b4243c6088..764d699658 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs +++ b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs @@ -2,7 +2,6 @@ using System; using System.Collections.Generic; using System.ComponentModel; using System.Linq; -using System.Text.RegularExpressions; using Avalonia.Collections; using Avalonia.VisualTree; diff --git a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs index 53abba8906..bd65a3b06b 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs +++ b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/TreePageViewModel.cs @@ -70,7 +70,7 @@ namespace Avalonia.Diagnostics.ViewModels { try { - FilterRegex = new Regex(PropertyFilter); + FilterRegex = new Regex(PropertyFilter, RegexOptions.Compiled); ClearError(); } catch (Exception exception) From 525dbbb2e7e16afb2e86b4f1a64bc209168ed3e6 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 1 Sep 2020 16:54:17 +0200 Subject: [PATCH 4/5] Addeding failing selection tests. --- .../Primitives/SelectingItemsControlTests.cs | 63 +++++++++++++++++++ .../Selection/SelectionModelTests_Multiple.cs | 32 ++++++++++ .../Selection/SelectionModelTests_Single.cs | 31 +++++++++ 3 files changed, 126 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs index 33744949c3..00d148093a 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -1335,6 +1335,47 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.False(model.SingleSelect); } + [Fact] + public void Does_The_Best_It_Can_With_AutoSelecting_ViewModel() + { + // Tests the following scenario: + // + // - Items changes from empty to having 1 item + // - ViewModel auto-selects item 0 in CollectionChanged + // - SelectionModel receives CollectionChanged + // - And so adjusts the selected item from 0 to 1, which is past the end of the items. + // + // There's not much we can do about this situation because the order in which + // CollectionChanged handlers are called can't be known (the problem also exists with + // WPF). The best we can do is not select an invalid index. + var vm = new SelectionViewModel(); + + vm.Items.CollectionChanged += (s, e) => + { + if (vm.SelectedIndex == -1 && vm.Items.Count > 0) + { + vm.SelectedIndex = 0; + } + }; + + var target = new ListBox + { + [!ListBox.ItemsProperty] = new Binding("Items"), + [!ListBox.SelectedIndexProperty] = new Binding("SelectedIndex"), + DataContext = vm, + }; + + Prepare(target); + + vm.Items.Add("foo"); + vm.Items.Add("bar"); + + Assert.Equal(0, target.SelectedIndex); + Assert.Equal(new[] { 0 }, target.Selection.SelectedIndexes); + Assert.Equal("foo", target.SelectedItem); + Assert.Equal(new[] { "foo" }, target.SelectedItems); + } + private static void Prepare(SelectingItemsControl target) { var root = new TestRoot @@ -1397,6 +1438,28 @@ namespace Avalonia.Controls.UnitTests.Primitives public int SelectedIndex { get; set; } } + private class SelectionViewModel : NotifyingBase + { + private int _selectedIndex = -1; + + public SelectionViewModel() + { + Items = new ObservableCollection(); + } + + public int SelectedIndex + { + get => _selectedIndex; + set + { + _selectedIndex = value; + RaisePropertyChanged(); + } + } + + public ObservableCollection Items { get; } + } + private class RootWithItems : TestRoot { public List Items { get; set; } = new List() { "a", "b", "c", "d", "e" }; diff --git a/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Multiple.cs b/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Multiple.cs index 3640faf7cb..3eddd35465 100644 --- a/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Multiple.cs +++ b/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Multiple.cs @@ -1230,6 +1230,38 @@ namespace Avalonia.Controls.UnitTests.Selection Assert.Equal(1, resetRaised); Assert.Equal(1, selectedIndexRaised); } + + [Fact] + public void Handles_Selection_Made_In_CollectionChanged() + { + // Tests the following scenario: + // + // - Items changes from empty to having 2 items + // - ViewModel auto-selects range 0..1 in CollectionChanged + // - SelectionModel receives CollectionChanged + // - And so adjusts the selected item from 0..1 to 2..4, which is past the end of + // the items. + // + // There's not much we can do about this situation because the order in which + // CollectionChanged handlers are called can't be known (the problem also exists with + // WPF). The best we can do is not select an invalid index. + var target = CreateTarget(createData: false); + var data = new AvaloniaList(); + + data.CollectionChanged += (s, e) => + { + target.SelectRange(0, 1); + }; + + target.Source = data; + data.AddRange(new[] { "foo", "bar" }); + + Assert.Equal(0, target.SelectedIndex); + Assert.Equal(new[] { 0, 1 }, target.SelectedIndexes); + Assert.Equal("foo", target.SelectedItem); + Assert.Equal(new[] { "foo", "bar" }, target.SelectedItems); + Assert.Equal(0, target.AnchorIndex); + } } public class BatchUpdate diff --git a/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Single.cs b/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Single.cs index 345518e729..1b37730797 100644 --- a/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Single.cs +++ b/tests/Avalonia.Controls.UnitTests/Selection/SelectionModelTests_Single.cs @@ -1052,6 +1052,37 @@ namespace Avalonia.Controls.UnitTests.Selection Assert.Equal(1, resetRaised); Assert.Equal(1, selectedIndexRaised); } + + [Fact] + public void Handles_Selection_Made_In_CollectionChanged() + { + // Tests the following scenario: + // + // - Items changes from empty to having 1 item + // - ViewModel auto-selects item 0 in CollectionChanged + // - SelectionModel receives CollectionChanged + // - And so adjusts the selected item from 0 to 1, which is past the end of the items. + // + // There's not much we can do about this situation because the order in which + // CollectionChanged handlers are called can't be known (the problem also exists with + // WPF). The best we can do is not select an invalid index. + var target = CreateTarget(createData: false); + var data = new AvaloniaList(); + + data.CollectionChanged += (s, e) => + { + target.Select(0); + }; + + target.Source = data; + data.Add("foo"); + + Assert.Equal(0, target.SelectedIndex); + Assert.Equal(new[] { 0 }, target.SelectedIndexes); + Assert.Equal("foo", target.SelectedItem); + Assert.Equal(new[] { "foo" }, target.SelectedItems); + Assert.Equal(0, target.AnchorIndex); + } } public class BatchUpdate From 8dfc65d17be00b7f7c96c294dabe7616916951b2 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 1 Sep 2020 18:16:55 +0200 Subject: [PATCH 5/5] Try to handle selections made in CollectionChanged. There's not much we can do about this except not select invalid indexes. --- .../Selection/SelectionModel.cs | 23 +++++++++++++ .../Selection/SelectionNodeBase.cs | 34 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/Avalonia.Controls/Selection/SelectionModel.cs b/src/Avalonia.Controls/Selection/SelectionModel.cs index 7ce2624d02..2556bd4c4c 100644 --- a/src/Avalonia.Controls/Selection/SelectionModel.cs +++ b/src/Avalonia.Controls/Selection/SelectionModel.cs @@ -436,6 +436,29 @@ namespace Avalonia.Controls.Selection } } + private protected override bool IsValidCollectionChange(NotifyCollectionChangedEventArgs e) + { + if (!base.IsValidCollectionChange(e)) + { + return false; + } + + if (ItemsView is object && e.Action == NotifyCollectionChangedAction.Add) + { + if (e.NewStartingIndex <= _selectedIndex) + { + return _selectedIndex + e.NewItems.Count < ItemsView.Count; + } + + if (e.NewStartingIndex <= _anchorIndex) + { + return _anchorIndex + e.NewItems.Count < ItemsView.Count; + } + } + + return true; + } + protected override void OnSourceCollectionChangeFinished() { if (_operation is object) diff --git a/src/Avalonia.Controls/Selection/SelectionNodeBase.cs b/src/Avalonia.Controls/Selection/SelectionNodeBase.cs index ff3b8f43a8..230575074a 100644 --- a/src/Avalonia.Controls/Selection/SelectionNodeBase.cs +++ b/src/Avalonia.Controls/Selection/SelectionNodeBase.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; +using System.Linq; using Avalonia.Controls.Utils; #nullable enable @@ -234,6 +235,11 @@ namespace Avalonia.Controls.Selection var shiftIndex = -1; List? removed = null; + if (!IsValidCollectionChange(e)) + { + return; + } + switch (e.Action) { case NotifyCollectionChangedAction.Add: @@ -276,6 +282,34 @@ namespace Avalonia.Controls.Selection } } + private protected virtual bool IsValidCollectionChange(NotifyCollectionChangedEventArgs e) + { + // If the selection is modified in a CollectionChanged handler before the selection + // model's CollectionChanged handler has had chance to run then we can end up with + // a selected index that refers to the *new* state of the Source intermixed with + // indexes that reference an old state of the source. + // + // There's not much we can do in this situation, so detect whether shifting the + // current selected indexes would result in an invalid index in the source, and if + // so bail. + // + // See unit test Handles_Selection_Made_In_CollectionChanged for more details. + if (ItemsView is object && + RangesEnabled && + Ranges.Count > 0 && + e.Action == NotifyCollectionChangedAction.Add) + { + var lastIndex = Ranges.Last().End; + + if (e.NewStartingIndex <= lastIndex) + { + return lastIndex + e.NewItems.Count < ItemsView.Count; + } + } + + return true; + } + private protected struct CollectionChangeState { public int ShiftIndex;