From 07ed194c9d95183489c85250f437f9200a5b385d Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Tue, 6 Aug 2019 19:58:08 +0300 Subject: [PATCH] Review comments --- src/Avalonia.Controls/Primitives/IPopupHost.cs | 4 ++-- .../{PopupHost.cs => OverlayPopupHost.cs} | 14 +++++++------- src/Avalonia.Controls/Primitives/Popup.cs | 10 +++++----- .../PopupPositioning/ManagedPopupPositioner.cs | 6 +++--- src/Avalonia.Controls/Primitives/PopupRoot.cs | 6 +++--- .../Primitives/VisualLayerManager.cs | 18 +++++++----------- src/Avalonia.Controls/ToolTip.cs | 6 +++--- .../Avalonia.Themes.Default.csproj | 8 ++++---- src/Avalonia.Themes.Default/ComboBox.xaml | 18 ++++++++---------- src/Avalonia.Themes.Default/DefaultTheme.xaml | 2 +- .../OverlayPopupHost.xaml | 14 ++++++++++++++ src/Avalonia.Themes.Default/PopupHost.xaml | 12 ------------ src/Avalonia.Themes.Default/PopupRoot.xaml | 14 ++++++++------ src/Avalonia.Visuals/Media/PixelPoint.cs | 2 -- src/Avalonia.Visuals/Media/PixelRect.cs | 1 - src/Avalonia.Visuals/Media/PixelVector.cs | 2 -- .../Primitives/PopupRootTests.cs | 6 +++++- .../Primitives/PopupTests.cs | 4 +++- 18 files changed, 73 insertions(+), 74 deletions(-) rename src/Avalonia.Controls/Primitives/{PopupHost.cs => OverlayPopupHost.cs} (93%) create mode 100644 src/Avalonia.Themes.Default/OverlayPopupHost.xaml delete mode 100644 src/Avalonia.Themes.Default/PopupHost.xaml diff --git a/src/Avalonia.Controls/Primitives/IPopupHost.cs b/src/Avalonia.Controls/Primitives/IPopupHost.cs index 912879af03..74a3ca8818 100644 --- a/src/Avalonia.Controls/Primitives/IPopupHost.cs +++ b/src/Avalonia.Controls/Primitives/IPopupHost.cs @@ -7,9 +7,9 @@ namespace Avalonia.Controls.Primitives { public interface IPopupHost : IDisposable { - void SetContent(IControl control); + void SetChild(IControl control); IContentPresenter Presenter { get; } - IVisual VisualRoot { get; } + IVisual HostedVisualTreeRoot { get; } event EventHandler TemplateApplied; diff --git a/src/Avalonia.Controls/Primitives/PopupHost.cs b/src/Avalonia.Controls/Primitives/OverlayPopupHost.cs similarity index 93% rename from src/Avalonia.Controls/Primitives/PopupHost.cs rename to src/Avalonia.Controls/Primitives/OverlayPopupHost.cs index 2e18ccfa64..3dc9d302db 100644 --- a/src/Avalonia.Controls/Primitives/PopupHost.cs +++ b/src/Avalonia.Controls/Primitives/OverlayPopupHost.cs @@ -9,25 +9,26 @@ using Avalonia.VisualTree; namespace Avalonia.Controls.Primitives { - public class PopupHost : ContentControl, IPopupHost, IInteractive, IManagedPopupPositionerPopup + public class OverlayPopupHost : ContentControl, IPopupHost, IInteractive, IManagedPopupPositionerPopup { private readonly OverlayLayer _overlayLayer; private PopupPositionerParameters _positionerParameters = new PopupPositionerParameters(); private ManagedPopupPositioner _positioner; + private Point _lastRequestedPosition; private bool _shown; - public PopupHost(OverlayLayer overlayLayer) + public OverlayPopupHost(OverlayLayer overlayLayer) { _overlayLayer = overlayLayer; _positioner = new ManagedPopupPositioner(this); } - public void SetContent(IControl control) + public void SetChild(IControl control) { Content = control; } - public IVisual VisualRoot => null; + public IVisual HostedVisualTreeRoot => null; /// IInteractive IInteractive.InteractiveParent => Parent; @@ -88,7 +89,7 @@ namespace Avalonia.Controls.Primitives } - void UpdatePosition() + private void UpdatePosition() { // Don't bother the positioner with layout system artifacts if (_positionerParameters.Size.Width == 0 || _positionerParameters.Size.Height == 0) @@ -111,7 +112,6 @@ namespace Avalonia.Controls.Primitives Rect IManagedPopupPositionerPopup.ParentClientAreaScreenGeometry => new Rect(default, _overlayLayer.Bounds.Size); - private Point _lastRequestedPosition; void IManagedPopupPositionerPopup.MoveAndResize(Point devicePoint, Size virtualSize) { _lastRequestedPosition = devicePoint; @@ -138,7 +138,7 @@ namespace Avalonia.Controls.Primitives "Unable to create IPopupImpl and no overlay layer is found for the target control"); - return new PopupHost(overlayLayer); + return new OverlayPopupHost(overlayLayer); } public override void Render(DrawingContext context) diff --git a/src/Avalonia.Controls/Primitives/Popup.cs b/src/Avalonia.Controls/Primitives/Popup.cs index b4c9fafda1..5ddbed5944 100644 --- a/src/Avalonia.Controls/Primitives/Popup.cs +++ b/src/Avalonia.Controls/Primitives/Popup.cs @@ -211,7 +211,7 @@ namespace Avalonia.Controls.Primitives /// /// Gets the root of the popup window. /// - IVisual IVisualTreeHost.Root => _popupHost?.VisualRoot; + IVisual IVisualTreeHost.Root => _popupHost?.HostedVisualTreeRoot; /// /// Opens the popup. @@ -234,12 +234,12 @@ namespace Avalonia.Controls.Primitives "Attempted to open a popup not attached to a TopLevel"); } - _popupHost = PopupHost.CreatePopupHost(placementTarget, DependencyResolver); + _popupHost = OverlayPopupHost.CreatePopupHost(placementTarget, DependencyResolver); _bindings.Add(_popupHost.BindConstraints(this, WidthProperty, MinWidthProperty, MaxWidthProperty, HeightProperty, MinHeightProperty, MaxHeightProperty, TopmostProperty)); - _popupHost.SetContent(Child); + _popupHost.SetChild(Child); ((ISetLogicalParent)_popupHost).SetParent(this); _popupHost.ConfigurePosition(placementTarget, PlacementMode, new Point(HorizontalOffset, VerticalOffset)); @@ -319,7 +319,7 @@ namespace Avalonia.Controls.Primitives foreach(var b in _bindings) b.Dispose(); _bindings.Clear(); - _popupHost.SetContent(null); + _popupHost.SetChild(null); _popupHost.Hide(); ((ISetLogicalParent)_popupHost).SetParent(null); _popupHost.Dispose(); @@ -455,7 +455,7 @@ namespace Avalonia.Controls.Primitives public bool IsInsidePopup(IVisual visual) { - return _popupHost != null && ((IVisual)_popupHost)?.FindCommonVisualAncestor(visual) == _popupHost; + return _popupHost != null && ((IVisual)_popupHost)?.IsVisualAncestorOf(visual) == true; } public bool IsPointerOverPopup => ((IInputElement)_popupHost).IsPointerOver; diff --git a/src/Avalonia.Controls/Primitives/PopupPositioning/ManagedPopupPositioner.cs b/src/Avalonia.Controls/Primitives/PopupPositioning/ManagedPopupPositioner.cs index 02f482d38d..d428952bb9 100644 --- a/src/Avalonia.Controls/Primitives/PopupPositioning/ManagedPopupPositioner.cs +++ b/src/Avalonia.Controls/Primitives/PopupPositioning/ManagedPopupPositioner.cs @@ -35,7 +35,7 @@ namespace Avalonia.Controls.Primitives.PopupPositioning } - static Point GetAnchorPoint(Rect anchorRect, PopupPositioningEdge edge) + private static Point GetAnchorPoint(Rect anchorRect, PopupPositioningEdge edge) { double x, y; if ((edge & PopupPositioningEdge.Left) != 0) @@ -54,7 +54,7 @@ namespace Avalonia.Controls.Primitives.PopupPositioning return new Point(x, y); } - static Point Gravitate(Point anchorPoint, Size size, PopupPositioningEdge gravity) + private static Point Gravitate(Point anchorPoint, Size size, PopupPositioningEdge gravity) { double x, y; if ((gravity & PopupPositioningEdge.Left) != 0) @@ -84,7 +84,7 @@ namespace Avalonia.Controls.Primitives.PopupPositioning } - void Update(Size translatedSize, Size originalSize, + private void Update(Size translatedSize, Size originalSize, Rect anchorRect, PopupPositioningEdge anchor, PopupPositioningEdge gravity, PopupPositionerConstraintAdjustment constraintAdjustment, Point offset) { diff --git a/src/Avalonia.Controls/Primitives/PopupRoot.cs b/src/Avalonia.Controls/Primitives/PopupRoot.cs index 8c4fbc7370..b7f0c8f47d 100644 --- a/src/Avalonia.Controls/Primitives/PopupRoot.cs +++ b/src/Avalonia.Controls/Primitives/PopupRoot.cs @@ -77,7 +77,7 @@ namespace Avalonia.Controls.Primitives /// public void Dispose() => PlatformImpl?.Dispose(); - void UpdatePosition() + private void UpdatePosition() { PlatformImpl?.PopupPositioner.Update(_positionerParameters); } @@ -93,9 +93,9 @@ namespace Avalonia.Controls.Primitives UpdatePosition(); } - public void SetContent(IControl control) => Content = control; + public void SetChild(IControl control) => Content = control; - IVisual IPopupHost.VisualRoot => this; + IVisual IPopupHost.HostedVisualTreeRoot => this; public IDisposable BindConstraints(AvaloniaObject popup, StyledProperty widthProperty, StyledProperty minWidthProperty, StyledProperty maxWidthProperty, StyledProperty heightProperty, StyledProperty minHeightProperty, diff --git a/src/Avalonia.Controls/Primitives/VisualLayerManager.cs b/src/Avalonia.Controls/Primitives/VisualLayerManager.cs index 7354f2788f..b7229eb121 100644 --- a/src/Avalonia.Controls/Primitives/VisualLayerManager.cs +++ b/src/Avalonia.Controls/Primitives/VisualLayerManager.cs @@ -8,14 +8,12 @@ namespace Avalonia.Controls.Primitives { private const int AdornerZIndex = int.MaxValue - 100; private const int OverlayZIndex = int.MaxValue - 99; + private IStyleHost _styleRoot; + private readonly List _layers = new List(); + - private bool _isAttachedToLogicalTree; - private IStyleHost _styleHost; public bool IsPopup { get; set; } - List _layers = new List(); - - public AdornerLayer AdornerLayer { get @@ -54,8 +52,8 @@ namespace Avalonia.Controls.Primitives ((ISetLogicalParent)layer).SetParent(this); layer.ZIndex = zindex; VisualChildren.Add(layer); - if (_isAttachedToLogicalTree) - ((ILogical)layer).NotifyAttachedToLogicalTree(new LogicalTreeAttachmentEventArgs(_styleHost)); + if (((ILogical)this).IsAttachedToLogicalTree) + ((ILogical)layer).NotifyAttachedToLogicalTree(new LogicalTreeAttachmentEventArgs(_styleRoot)); InvalidateArrange(); } @@ -63,8 +61,7 @@ namespace Avalonia.Controls.Primitives protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e) { base.OnAttachedToLogicalTree(e); - _isAttachedToLogicalTree = true; - _styleHost = e.Root; + _styleRoot = e.Root; foreach (var l in _layers) ((ILogical)l).NotifyAttachedToLogicalTree(e); @@ -72,8 +69,7 @@ namespace Avalonia.Controls.Primitives protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs e) { - _styleHost = null; - _isAttachedToLogicalTree = false; + _styleRoot = null; base.OnDetachedFromLogicalTree(e); foreach (var l in _layers) ((ILogical)l).NotifyDetachedFromLogicalTree(e); diff --git a/src/Avalonia.Controls/ToolTip.cs b/src/Avalonia.Controls/ToolTip.cs index 682f7cdd70..5fe1e3804b 100644 --- a/src/Avalonia.Controls/ToolTip.cs +++ b/src/Avalonia.Controls/ToolTip.cs @@ -235,8 +235,8 @@ namespace Avalonia.Controls { Close(); - _popup = PopupHost.CreatePopupHost(control, null); - _popup.SetContent(this); + _popup = OverlayPopupHost.CreatePopupHost(control, null); + _popup.SetChild(this); ((ISetLogicalParent)_popup).SetParent(control); _popup.ConfigurePosition(control, GetPlacement(control), @@ -248,7 +248,7 @@ namespace Avalonia.Controls { if (_popup != null) { - _popup.SetContent(null); + _popup.SetChild(null); _popup.Hide(); _popup = null; } diff --git a/src/Avalonia.Themes.Default/Avalonia.Themes.Default.csproj b/src/Avalonia.Themes.Default/Avalonia.Themes.Default.csproj index 3d525955b4..c44cc358e8 100644 --- a/src/Avalonia.Themes.Default/Avalonia.Themes.Default.csproj +++ b/src/Avalonia.Themes.Default/Avalonia.Themes.Default.csproj @@ -12,11 +12,11 @@ - - + + - + - + diff --git a/src/Avalonia.Themes.Default/ComboBox.xaml b/src/Avalonia.Themes.Default/ComboBox.xaml index ffc96d5a2c..49f1ad1024 100644 --- a/src/Avalonia.Themes.Default/ComboBox.xaml +++ b/src/Avalonia.Themes.Default/ComboBox.xaml @@ -39,16 +39,14 @@ StaysOpen="False"> - - - - - + + + diff --git a/src/Avalonia.Themes.Default/DefaultTheme.xaml b/src/Avalonia.Themes.Default/DefaultTheme.xaml index 75dc5edd94..114979fba2 100644 --- a/src/Avalonia.Themes.Default/DefaultTheme.xaml +++ b/src/Avalonia.Themes.Default/DefaultTheme.xaml @@ -19,7 +19,7 @@ - + diff --git a/src/Avalonia.Themes.Default/OverlayPopupHost.xaml b/src/Avalonia.Themes.Default/OverlayPopupHost.xaml new file mode 100644 index 0000000000..35d3a8cff4 --- /dev/null +++ b/src/Avalonia.Themes.Default/OverlayPopupHost.xaml @@ -0,0 +1,14 @@ + diff --git a/src/Avalonia.Themes.Default/PopupHost.xaml b/src/Avalonia.Themes.Default/PopupHost.xaml deleted file mode 100644 index 21d99e5305..0000000000 --- a/src/Avalonia.Themes.Default/PopupHost.xaml +++ /dev/null @@ -1,12 +0,0 @@ - diff --git a/src/Avalonia.Themes.Default/PopupRoot.xaml b/src/Avalonia.Themes.Default/PopupRoot.xaml index cc23367ac0..71042f2a98 100644 --- a/src/Avalonia.Themes.Default/PopupRoot.xaml +++ b/src/Avalonia.Themes.Default/PopupRoot.xaml @@ -2,11 +2,13 @@ - + + + - \ No newline at end of file + diff --git a/src/Avalonia.Visuals/Media/PixelPoint.cs b/src/Avalonia.Visuals/Media/PixelPoint.cs index 1fc102045e..d62c2a2e55 100644 --- a/src/Avalonia.Visuals/Media/PixelPoint.cs +++ b/src/Avalonia.Visuals/Media/PixelPoint.cs @@ -160,8 +160,6 @@ namespace Avalonia } } - - /// /// Returns a new with the same Y co-ordinate and the specified X co-ordinate. /// diff --git a/src/Avalonia.Visuals/Media/PixelRect.cs b/src/Avalonia.Visuals/Media/PixelRect.cs index 75987681ff..0e2094da07 100644 --- a/src/Avalonia.Visuals/Media/PixelRect.cs +++ b/src/Avalonia.Visuals/Media/PixelRect.cs @@ -272,7 +272,6 @@ namespace Avalonia return new PixelRect(Position + offset, Size); } - /// /// Gets the union of two rectangles. /// diff --git a/src/Avalonia.Visuals/Media/PixelVector.cs b/src/Avalonia.Visuals/Media/PixelVector.cs index b959b462c2..4a623e3bc2 100644 --- a/src/Avalonia.Visuals/Media/PixelVector.cs +++ b/src/Avalonia.Visuals/Media/PixelVector.cs @@ -130,9 +130,7 @@ namespace Avalonia /// public bool Equals(PixelVector other) { - // ReSharper disable CompareOfFloatsByEqualityOperator return _x == other._x && _y == other._y; - // ReSharper restore CompareOfFloatsByEqualityOperator } /// diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/PopupRootTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/PopupRootTests.cs index e840e7b530..0ebe6833d3 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/PopupRootTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/PopupRootTests.cs @@ -75,10 +75,14 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Single(((Visual)target.Host).GetVisualChildren()); var templatedChild = ((Visual)target.Host).GetVisualChildren().Single(); - Assert.IsType(templatedChild); + + Assert.IsType(templatedChild); + var contentPresenter = templatedChild.VisualChildren.Single(); + Assert.IsType(contentPresenter); Assert.Equal((PopupRoot)target.Host, ((IControl)templatedChild).TemplatedParent); + Assert.Equal((PopupRoot)target.Host, ((IControl)contentPresenter).TemplatedParent); } } diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs index f5d68be02a..7cb9fccee8 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs @@ -252,6 +252,7 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal( new[] { + "VisualLayerManager", "ContentPresenter", "ContentPresenter", "Border", @@ -265,6 +266,7 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal( new object[] { + popupRoot, popupRoot, target, null, @@ -320,7 +322,7 @@ namespace Avalonia.Controls.UnitTests.Primitives target.Open(); if (UsePopupHost) - Assert.IsType(target.Host); + Assert.IsType(target.Host); else Assert.IsType(target.Host); }