diff --git a/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs index 8fc2a7b77c..3bf6842cd6 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.ComponentModel; using System.Reflection; using Avalonia.Utilities; @@ -11,8 +12,11 @@ namespace Avalonia.Data.Core.Plugins /// public class InpcPropertyAccessorPlugin : IPropertyAccessorPlugin { + private readonly Dictionary<(Type, string), PropertyInfo> _propertyLookup = + new Dictionary<(Type, string), PropertyInfo>(); + /// - public bool Match(object obj, string propertyName) => GetPropertyWithName(obj.GetType(), propertyName) != null; + public bool Match(object obj, string propertyName) => GetFirstPropertyWithName(obj.GetType(), propertyName) != null; /// /// Starts monitoring the value of a property on an object. @@ -30,7 +34,7 @@ namespace Avalonia.Data.Core.Plugins reference.TryGetTarget(out object instance); - var p = GetPropertyWithName(instance.GetType(), propertyName); + var p = GetFirstPropertyWithName(instance.GetType(), propertyName); if (p != null) { @@ -44,12 +48,40 @@ namespace Avalonia.Data.Core.Plugins } } - private static PropertyInfo GetPropertyWithName(Type type, string propertyName) + private PropertyInfo GetFirstPropertyWithName(Type type, string propertyName) { - const BindingFlags bindingFlags = BindingFlags.NonPublic | BindingFlags.Public | - BindingFlags.Static | BindingFlags.Instance; + var key = (type, propertyName); + + if (!_propertyLookup.TryGetValue(key, out PropertyInfo propertyInfo)) + { + propertyInfo = TryFindAndCacheProperty(type, propertyName); + } + + return propertyInfo; + } + + private PropertyInfo TryFindAndCacheProperty(Type type, string propertyName) + { + PropertyInfo found = null; + + const BindingFlags bindingFlags = + BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance; + + var properties = type.GetProperties(bindingFlags); + + foreach (PropertyInfo propertyInfo in properties) + { + if (propertyInfo.Name == propertyName) + { + found = propertyInfo; + + break; + } + } + + _propertyLookup.Add((type, propertyName), found); - return type.GetProperty(propertyName, bindingFlags); + return found; } private class Accessor : PropertyAccessorBase, IWeakSubscriber diff --git a/src/Avalonia.Controls/Primitives/Popup.cs b/src/Avalonia.Controls/Primitives/Popup.cs index 1e5e80d144..becb489557 100644 --- a/src/Avalonia.Controls/Primitives/Popup.cs +++ b/src/Avalonia.Controls/Primitives/Popup.cs @@ -358,7 +358,7 @@ namespace Avalonia.Controls.Primitives return; } - var placementTarget = PlacementTarget ?? this.GetLogicalAncestors().OfType().FirstOrDefault(); + var placementTarget = PlacementTarget ?? this.FindLogicalAncestorOfType(); if (placementTarget == null) { @@ -586,6 +586,26 @@ namespace Avalonia.Controls.Primitives } Closed?.Invoke(this, EventArgs.Empty); + + var focusCheck = FocusManager.Instance?.Current; + + // Focus is set to null as part of popup closing, so we only want to + // set focus to PlacementTarget if this is the case + if (focusCheck == null) + { + if (PlacementTarget != null) + { + FocusManager.Instance?.Focus(PlacementTarget); + } + else + { + var anc = this.FindLogicalAncestorOfType(); + if (anc != null) + { + FocusManager.Instance?.Focus(anc); + } + } + } } private void ListenForNonClientClick(RawInputEventArgs e) diff --git a/src/Avalonia.Controls/TreeView.cs b/src/Avalonia.Controls/TreeView.cs index b4c30e0149..b2bd5ab2e5 100644 --- a/src/Avalonia.Controls/TreeView.cs +++ b/src/Avalonia.Controls/TreeView.cs @@ -117,10 +117,8 @@ namespace Avalonia.Controls if (value != null) { if (selectedItems.Count != 1 || selectedItems[0] != value) - { - _syncingSelectedItems = true; - SelectSingleItem(value); - _syncingSelectedItems = false; + { + SelectSingleItem(value); } } else if (SelectedItems.Count > 0) @@ -219,8 +217,12 @@ namespace Avalonia.Controls private void SelectSingleItem(object item) { - SelectedItems.Clear(); + _syncingSelectedItems = true; + SelectedItems.Clear(); SelectedItems.Add(item); + _syncingSelectedItems = false; + + SetAndRaise(SelectedItemProperty, ref _selectedItem, item); } /// diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_Property.cs b/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_Property.cs index 784046ac0b..32cdd21e04 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_Property.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_Property.cs @@ -600,6 +600,24 @@ namespace Avalonia.Base.UnitTests.Data.Core result); } + [Fact] + public void Should_Not_Throw_Exception_On_Duplicate_Properties() + { + // Repro of https://github.com/AvaloniaUI/Avalonia/issues/4733. + var source = new MyViewModel(); + var target = new PropertyAccessorNode("Name", false); + + target.Target = new WeakReference(source); + + var result = new List(); + + target.Subscribe(x => result.Add(x)); + } + + public class MyViewModelBase { public object Name => "Name"; } + + public class MyViewModel : MyViewModelBase { public new string Name => "NewName"; } + private interface INext { int PropertyChangedSubscriptionCount { get; } diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs index f032186bcd..9223b08c81 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs @@ -485,6 +485,85 @@ namespace Avalonia.Controls.UnitTests.Primitives } } + [Fact] + public void Closing_Popup_Sets_Focus_On_PlacementTarget() + { + using (CreateServicesWithFocus()) + { + var window = PreparedWindow(); + + var tb = new TextBox(); + var p = new Popup + { + PlacementTarget = window, + Child = tb + }; + ((ISetLogicalParent)p).SetParent(p.PlacementTarget); + window.Show(); + + p.Open(); + + if (p.Host is OverlayPopupHost host) + { + //Need to measure/arrange for visual children to show up + //in OverlayPopupHost + host.Measure(Size.Infinity); + host.Arrange(new Rect(host.DesiredSize)); + } + + tb.Focus(); + + p.Close(); + + var focus = FocusManager.Instance?.Current; + Assert.True(focus == window); + } + } + + [Fact] + public void Prog_Close_Popup_NoLightDismiss_Doesnt_Move_Focus_To_PlacementTarget() + { + using (CreateServicesWithFocus()) + { + var window = PreparedWindow(); + + var windowTB = new TextBox(); + window.Content = windowTB; + + var popupTB = new TextBox(); + var p = new Popup + { + PlacementTarget = window, + IsLightDismissEnabled = false, + Child = popupTB + }; + ((ISetLogicalParent)p).SetParent(p.PlacementTarget); + window.Show(); + + p.Open(); + + if (p.Host is OverlayPopupHost host) + { + //Need to measure/arrange for visual children to show up + //in OverlayPopupHost + host.Measure(Size.Infinity); + host.Arrange(new Rect(host.DesiredSize)); + } + + popupTB.Focus(); + + windowTB.Focus(); + + var focus = FocusManager.Instance?.Current; + + Assert.True(focus == windowTB); + + p.Close(); + + Assert.True(focus == windowTB); + } + } + private IDisposable CreateServices() { return UnitTestApplication.Start(TestServices.StyledWindow.With(windowingPlatform: diff --git a/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs b/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs index 7022fbf4c1..9253d8a07f 100644 --- a/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Linq; using Avalonia.Collections; using Avalonia.Controls.Presenters; @@ -13,6 +14,7 @@ using Avalonia.Interactivity; using Avalonia.LogicalTree; using Avalonia.Styling; using Avalonia.UnitTests; +using ReactiveUI; using Xunit; namespace Avalonia.Controls.UnitTests @@ -454,6 +456,41 @@ namespace Avalonia.Controls.UnitTests } } + [Fact] + public void Bound_SelectedItem_Should_Not_Be_Cleared_when_Changing_Selection() + { + using (Application()) + { + var dataContext = new TestDataContext(); + + var target = new TreeView + { + Template = CreateTreeViewTemplate(), + DataContext = dataContext + }; + + target.Bind(TreeView.ItemsProperty, new Binding("Items")); + target.Bind(TreeView.SelectedItemProperty, new Binding("SelectedItem")); + + var visualRoot = new TestRoot(); + visualRoot.Child = target; + + CreateNodeDataTemplate(target); + ApplyTemplates(target); + + var selectedValues = new List(); + + dataContext.WhenAnyValue(x => x.SelectedItem) + .Subscribe(x => selectedValues.Add(x)); + + _mouse.Click((Interactive)target.Presenter.Panel.Children[0], MouseButton.Left); + _mouse.Click((Interactive)target.Presenter.Panel.Children[2], MouseButton.Left); + + Assert.Equal(3, selectedValues.Count); + Assert.Equal(new[] { null, "Item 0", "Item 2" }, selectedValues.ToArray()); + } + } + [Fact] public void LogicalChildren_Should_Be_Set() { @@ -1288,5 +1325,26 @@ namespace Avalonia.Controls.UnitTests private class DerivedTreeView : TreeView { } + + private class TestDataContext : ReactiveObject + { + private string _selectedItem; + + public TestDataContext() + { + Items = new ObservableCollection(Enumerable.Range(0, 5).Select(i => $"Item {i}")); + } + + public ObservableCollection Items { get; } + + public string SelectedItem + { + get { return _selectedItem; } + set + { + this.RaiseAndSetIfChanged(ref _selectedItem, value); + } + } + } } }