Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pointer popup positioning for overlay popups #7250

Merged
merged 7 commits into from
Jan 24, 2022
Merged

Fix pointer popup positioning for overlay popups #7250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9275e59
Do not recalculate pointer popup position on parent moved
maxkatz6 Dec 23, 2021
50a97dc
Add more popup tests
maxkatz6 Dec 23, 2021
d7fb1bb
Merge branch 'master' into fixex/pointer-popup-positioning
danwalmsley Dec 30, 2021
141c805
Merge branch 'master' into fixex/pointer-popup-positioning
danwalmsley Dec 30, 2021
7eff00f
Merge branch 'master' into fixex/pointer-popup-positioning
danwalmsley Jan 2, 2022
38d69e1
Merge branch 'master' into fixex/pointer-popup-positioning
danwalmsley Jan 17, 2022
71743c5
Merge branch 'master' into fixex/pointer-popup-positioning
maxkatz6 Jan 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions src/Avalonia.Controls/Primitives/Popup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,16 +429,20 @@ public void Open()
(x, handler) => x.LostFocus += handler,
(x, handler) => x.LostFocus -= handler).DisposeWith(handlerCleanup);

SubscribeToEventHandler<IWindowImpl, Action<PixelPoint>>(window.PlatformImpl, WindowPositionChanged,
(x, handler) => x.PositionChanged += handler,
(x, handler) => x.PositionChanged -= handler).DisposeWith(handlerCleanup);

if (placementTarget is Layoutable layoutTarget)
// Recalculate popup position on parent moved/resized, but not if placement was on pointer
if (PlacementMode != PlacementMode.Pointer)
{
// If the placement target is moved, update the popup position
SubscribeToEventHandler<Layoutable, EventHandler>(layoutTarget, PlacementTargetLayoutUpdated,
(x, handler) => x.LayoutUpdated += handler,
(x, handler) => x.LayoutUpdated -= handler).DisposeWith(handlerCleanup);
SubscribeToEventHandler<IWindowImpl, Action<PixelPoint>>(window.PlatformImpl, WindowPositionChanged,
(x, handler) => x.PositionChanged += handler,
(x, handler) => x.PositionChanged -= handler).DisposeWith(handlerCleanup);

if (placementTarget is Layoutable layoutTarget)
{
// If the placement target is moved, update the popup position
SubscribeToEventHandler<Layoutable, EventHandler>(layoutTarget, PlacementTargetLayoutUpdated,
(x, handler) => x.LayoutUpdated += handler,
(x, handler) => x.LayoutUpdated -= handler).DisposeWith(handlerCleanup);
}
}
}
else if (topLevel is PopupRoot parentPopupRoot)
Expand Down
237 changes: 224 additions & 13 deletions tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
using Xunit;
using Avalonia.Input;
using Avalonia.Rendering;
using System.Threading.Tasks;
using Avalonia.Threading;

namespace Avalonia.Controls.UnitTests.Primitives
{
Expand Down Expand Up @@ -597,27 +599,44 @@ public void Prog_Close_Popup_NoLightDismiss_Doesnt_Move_Focus_To_PlacementTarget
}

[Fact]
public void Popup_Should_Follow_Placement_Target_On_Window_Move()
public void Popup_Should_Not_Follow_Placement_Target_On_Window_Move_If_Pointer()
{
using (CreateServices())
{
var popup = new Popup { Width = 400, Height = 200 };
var popup = new Popup
{
Width = 400,
Height = 200,
PlacementMode = PlacementMode.Pointer
};
var window = PreparedWindow(popup);
window.Show();
popup.Open();
Dispatcher.UIThread.RunJobs(DispatcherPriority.Layout);

var raised = false;
if (popup.Host is PopupRoot popupRoot)
{
// Moving the window must move the popup (screen coordinates have changed)
var raised = false;
popupRoot.PositionChanged += (_, args) =>
{
Assert.Equal(new PixelPoint(10, 10), args.Point);
raised = true;
};

window.Position = new PixelPoint(10, 10);
Assert.True(raised);
}
else if (popup.Host is OverlayPopupHost overlayPopupHost)
{
overlayPopupHost.PropertyChanged += (_, args) =>
{
if (args.Property == Canvas.TopProperty
|| args.Property == Canvas.LeftProperty)
{
raised = true;
}
};
}
window.Position = new PixelPoint(10, 10);
Dispatcher.UIThread.RunJobs(DispatcherPriority.Layout);
Assert.False(raised);
}
}

Expand All @@ -634,30 +653,222 @@ public void Popup_Should_Follow_Placement_Target_On_Window_Resize()
HorizontalAlignment = HorizontalAlignment.Center,
VerticalAlignment = VerticalAlignment.Center
};
var popup = new Popup() { PlacementTarget = placementTarget, Width = 10, Height = 10 };
var popup = new Popup()
{
PlacementTarget = placementTarget,
PlacementMode = PlacementMode.Bottom,
Width = 10,
Height = 10
};
((ISetLogicalParent)popup).SetParent(popup.PlacementTarget);

var window = PreparedWindow(placementTarget);
window.Show();
popup.Open();
Dispatcher.UIThread.RunJobs(DispatcherPriority.Layout);

// The target's initial placement is (395,295) which is a 10x10 panel centered in a 800x600 window
Assert.Equal(placementTarget.Bounds, new Rect(395D, 295D, 10, 10));

var raised = false;
// Resizing the window to 700x500 must move the popup to (345,255) as this is the new
// location of the placement target
if (popup.Host is PopupRoot popupRoot)
{
// Resizing the window to 700x500 must move the popup to (345,255) as this is the new
// location of the placement target
var raised = false;
popupRoot.PositionChanged += (_, args) =>
{
Assert.Equal(new PixelPoint(345, 255), args.Point);
raised = true;
};

window.PlatformImpl?.Resize(new Size(700D, 500D), PlatformResizeReason.Unspecified);
Assert.True(raised);
}
else if (popup.Host is OverlayPopupHost overlayPopupHost)
{
overlayPopupHost.PropertyChanged += (_, args) =>
{
if ((args.Property == Canvas.TopProperty
|| args.Property == Canvas.LeftProperty)
&& Canvas.GetLeft(overlayPopupHost) == 345
&& Canvas.GetTop(overlayPopupHost) == 255)
{
raised = true;
}
};
}
window.PlatformImpl?.Resize(new Size(700D, 500D), PlatformResizeReason.Unspecified);
Dispatcher.UIThread.RunJobs(DispatcherPriority.Layout);
Assert.True(raised);
}
}

[Fact]
public void Popup_Should_Not_Follow_Placement_Target_On_Window_Resize_If_Pointer_If_Pointer()
{
using (CreateServices())
{

var placementTarget = new Panel()
{
Width = 10,
Height = 10,
HorizontalAlignment = HorizontalAlignment.Center,
VerticalAlignment = VerticalAlignment.Center
};
var popup = new Popup()
{
PlacementTarget = placementTarget,
PlacementMode = PlacementMode.Pointer,
Width = 10,
Height = 10
};
((ISetLogicalParent)popup).SetParent(popup.PlacementTarget);

var window = PreparedWindow(placementTarget);
window.Show();
popup.Open();
Dispatcher.UIThread.RunJobs(DispatcherPriority.Layout);

// The target's initial placement is (395,295) which is a 10x10 panel centered in a 800x600 window
Assert.Equal(placementTarget.Bounds, new Rect(395D, 295D, 10, 10));

var raised = false;
if (popup.Host is PopupRoot popupRoot)
{
popupRoot.PositionChanged += (_, args) =>
{
raised = true;
};

}
else if (popup.Host is OverlayPopupHost overlayPopupHost)
{
overlayPopupHost.PropertyChanged += (_, args) =>
{
if (args.Property == Canvas.TopProperty
|| args.Property == Canvas.LeftProperty)
{
raised = true;
}
};
}
window.PlatformImpl?.Resize(new Size(700D, 500D), PlatformResizeReason.Unspecified);
Dispatcher.UIThread.RunJobs(DispatcherPriority.Layout);
Assert.False(raised);
}
}

[Fact]
public void Popup_Should_Follow_Placement_Target_On_Target_Moved()
{
using (CreateServices())
{
var placementTarget = new Panel()
{
Width = 10,
Height = 10,
HorizontalAlignment = HorizontalAlignment.Center,
VerticalAlignment = VerticalAlignment.Center
};
var popup = new Popup()
{
PlacementTarget = placementTarget,
PlacementMode = PlacementMode.Bottom,
Width = 10,
Height = 10
};
((ISetLogicalParent)popup).SetParent(popup.PlacementTarget);

var window = PreparedWindow(placementTarget);
window.Show();
popup.Open();
Dispatcher.UIThread.RunJobs(DispatcherPriority.Layout);

// The target's initial placement is (395,295) which is a 10x10 panel centered in a 800x600 window
Assert.Equal(placementTarget.Bounds, new Rect(395D, 295D, 10, 10));

var raised = false;
// Margin will move placement target
if (popup.Host is PopupRoot popupRoot)
{
popupRoot.PositionChanged += (_, args) =>
{
Assert.Equal(new PixelPoint(400, 305), args.Point);
raised = true;
};

}
else if (popup.Host is OverlayPopupHost overlayPopupHost)
{
overlayPopupHost.PropertyChanged += (_, args) =>
{
if ((args.Property == Canvas.TopProperty
|| args.Property == Canvas.LeftProperty)
&& Canvas.GetLeft(overlayPopupHost) == 400
&& Canvas.GetTop(overlayPopupHost) == 305)
{
raised = true;
}
};
}
placementTarget.Margin = new Thickness(10, 0, 0, 0);
Dispatcher.UIThread.RunJobs(DispatcherPriority.Layout);
Assert.True(raised);
}
}

[Fact]
public void Popup_Should_Not_Follow_Placement_Target_On_Target_Moved_If_Pointer()
{
using (CreateServices())
{

var placementTarget = new Panel()
{
Width = 10,
Height = 10,
HorizontalAlignment = HorizontalAlignment.Center,
VerticalAlignment = VerticalAlignment.Center
};
var popup = new Popup()
{
PlacementTarget = placementTarget,
PlacementMode = PlacementMode.Pointer,
Width = 10,
Height = 10
};
((ISetLogicalParent)popup).SetParent(popup.PlacementTarget);

var window = PreparedWindow(placementTarget);
window.Show();
popup.Open();
Dispatcher.UIThread.RunJobs(DispatcherPriority.Layout);

// The target's initial placement is (395,295) which is a 10x10 panel centered in a 800x600 window
Assert.Equal(placementTarget.Bounds, new Rect(395D, 295D, 10, 10));

var raised = false;
if (popup.Host is PopupRoot popupRoot)
{
popupRoot.PositionChanged += (_, args) =>
{
raised = true;
};

}
else if (popup.Host is OverlayPopupHost overlayPopupHost)
{
overlayPopupHost.PropertyChanged += (_, args) =>
{
if (args.Property == Canvas.TopProperty
|| args.Property == Canvas.LeftProperty)
{
raised = true;
}
};
}
placementTarget.Margin = new Thickness(10, 0, 0, 0);
Dispatcher.UIThread.RunJobs(DispatcherPriority.Layout);
Assert.False(raised);
}
}

Expand Down