Skip to content

Commit

Permalink
fix: Align Popup light-dismiss with WinUI
Browse files Browse the repository at this point in the history
Handle pointer events exclusively on PopupPanel (not quite PopupRoot, but close). Rather than bounds-checking and adding handlers to the child content, just check the OriginalSource of the event, as WinUI does, to exclude events that are 'blocked' by the Popup content. This aligns Uno with WinUI in terms of hit-transparent areas of the content.
  • Loading branch information
davidjohnoliver committed Dec 14, 2021
1 parent 3078e4e commit 040acc4
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 91 deletions.
17 changes: 0 additions & 17 deletions src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,23 +122,6 @@ partial void OnChildChangedPartial(UIElement oldChild, UIElement newChild)
UpdateDataContext(null);
UpdateTemplatedParent();
PropagateFocusProperties();

if (oldChild is FrameworkElement ocfe)
{
ocfe.PointerPressed -= HandlePointerEvent;
ocfe.PointerReleased -= HandlePointerEvent;
}

if (newChild is FrameworkElement ncfe)
{
ncfe.PointerPressed += HandlePointerEvent;
ncfe.PointerReleased += HandlePointerEvent;
}
}

private void HandlePointerEvent(object sender, PointerRoutedEventArgs e)
{
e.Handled = true;
}

protected internal override void OnDataContextChanged(DependencyPropertyChangedEventArgs e)
Expand Down
74 changes: 0 additions & 74 deletions src/Uno.UI/UI/Xaml/Controls/Popup/Popup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,36 +86,6 @@ internal PopupPanel PopupPanel

private void OnPopupPanelChanged(PopupPanel oldHost, PopupPanel newHost)
{
// Note: On UWP if the popup is light dismissable, the popup is closed as soon as we get a PointerPressed
// Controls under the popup does not receive any Pointer event while the popup is open,
// and when light dismissed they receive the PointerReleased but not the pressed

if (oldHost != null)
{
oldHost.PointerPressed -= _handleIfOpenedAndTryDismiss; // Cf. comment about child pressed below

oldHost.PointerEntered -= _handleIfOpened;
oldHost.PointerExited -= _handleIfOpened;
oldHost.PointerMoved -= _handleIfOpened;
oldHost.PointerReleased -= _handleIfOpened;
oldHost.PointerCanceled -= _handleIfOpened;
oldHost.PointerCaptureLost -= _handleIfOpened;
oldHost.PointerWheelChanged -= _handleIfOpened;
}

if (newHost != null)
{
newHost.PointerPressed += _handleIfOpenedAndTryDismiss; // Cf. comment about child pressed below

newHost.PointerEntered += _handleIfOpened;
newHost.PointerExited += _handleIfOpened;
newHost.PointerMoved += _handleIfOpened;
newHost.PointerReleased += _handleIfOpened;
newHost.PointerCanceled += _handleIfOpened;
newHost.PointerCaptureLost += _handleIfOpened;
newHost.PointerWheelChanged += _handleIfOpened;
}

OnPopupPanelChangedPartial(oldHost, newHost);

ApplyLightDismissOverlayMode();
Expand All @@ -129,50 +99,6 @@ partial void OnVerticalOffsetChangedPartial(double oldVerticalOffset, double new
partial void OnHorizontalOffsetChangedPartial(double oldHorizontalOffset, double newHorizontalOffset)
=> PopupPanel?.InvalidateMeasure();

private static readonly PointerEventHandler _handleIfOpened = (snd, e) =>
{
var popup = ((PopupPanel)snd).Popup;
if (!popup.IsOpen || !popup.IsLightDismissEnabled)
{
return;
}

e.Handled = true;
};
private static readonly PointerEventHandler _handleIfOpenedAndTryDismiss = (snd, e) =>
{
var popup = ((PopupPanel)snd).Popup;
if (!popup.IsOpen || !popup.IsLightDismissEnabled)
{
return;
}

e.Handled = true;

if (popup.Child is FrameworkElement content)
{
var position = e.GetCurrentPoint(content).Position;
if (
position.X < 0 || position.X > content.ActualWidth
|| position.Y < 0 || position.Y > content.ActualHeight)
{
popup.IsOpen = false;
}
}
else if (popup.Child == null)
{
popup.Log().Warn($"Dismiss is not supported if the 'Child' is null.");
}
else
{
popup.Log().Warn($"Dismiss is not supported if the 'Child' ({popup.Child?.GetType()}) of the 'Popup' is not a 'FrameworkElement'.");
}

// Note: This is not the right way: we should instead listen for the PointerPressed directly on the Child
// so we should be able to still dismiss the Popup if the background if the Child is `null`.
// But this would require us to refactor more deeply the Popup which is not the purpose of the current work.
};

internal override void UpdateThemeBindings(Data.ResourceUpdateReason updateReason)
{
base.UpdateThemeBindings(updateReason);
Expand Down
15 changes: 15 additions & 0 deletions src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Windows.UI.Xaml.Media;
using Uno.UI.DataBinding;
using Uno.Foundation.Logging;
using Windows.UI.Xaml.Input;

#if XAMARIN_IOS
using UIKit;
Expand Down Expand Up @@ -38,6 +39,7 @@ public PopupPanel(Popup popup)
{
Popup = popup ?? throw new ArgumentNullException(nameof(popup));
Visibility = Visibility.Collapsed;
PointerPressed += OnPointerPressed;
}

protected Size _lastMeasuredSize;
Expand Down Expand Up @@ -188,5 +190,18 @@ private protected override void OnUnloaded()
base.OnUnloaded();
this.SetLogicalParent(null);
}

// TODO: pointer handling should really go on PopupRoot. For now it's easier to put here because PopupRoot doesn't track open popups, and also we
// need to support native popups on Android that don't use PopupRoot.
private void OnPointerPressed(object sender, PointerRoutedEventArgs args)
{
// Make sure we are the original source. We do not want to handle PointerPressed on the Popup itself.
if (args.OriginalSource == this && Popup is { } popup)
{
popup.IsOpen = false;
}
}

internal override bool IsViewHit() => Popup?.IsLightDismissEnabled ?? false;
}
}

0 comments on commit 040acc4

Please sign in to comment.