Fix Button.Flyout not toggling

[pr8x]

• Fixes Button.Flyout not toggling correctly (see Button.Flyout should toggle on/off #8379) by manually setting OverlayInputPassThroughElement to placementTarget inside FlyoutBase (as suggested by @maxkatz6). Not sure if this can have side effects for other code using flyouts.
• Exposing OverlayDismissEventPassThrough on FlyoutBase to allow for customizing event pass-through behavior.

Fixes #8379

[grokys]

@pr8x I'll close my PR and we'll use this one as it also contains fixes.

What I assume should happen is that if IsSet(OverlayInputPassThroughElementProperty) then the control provided there should be used (or it could be set to null to disable the behavior), otherwise the placement target should be used. Does that sound right?

cc: @maxkatz6

[maxkatz6]

@grokys it's a direct property, so does IsSet work there properly?

[grokys]

Ah yeah IsSet doesn't work with direct properties. I guess we have 2 choices:

1. Make OverlayInputPassThroughElementProperty a styled property on FlyoutBase
2. Leave it a direct property and make null mean unset: this means there will be no way to disable making the PlacementTarget the passthough element, but not sure if that's actually required.

[pr8x]

I wonder if we could just explicitly set the OverlayInputPassThroughElement for Button.Flyout and in other cases leave the user to decide?

[maxkatz6]

@pr8x yes, it should be better

[grokys]

I wonder if we could just explicitly set the OverlayInputPassThroughElement for Button.Flyout and in other cases leave the user to decide?

Is this what UWP does?

[pr8x]

Is this what UWP does?

No idea. Couldn't find the source code for FlyoutBase

[grokys]

I think it should be easy enough to determine by adding a flyout to a button and seeing if it behaves differently to flyouts on other controls?

[robloo]

Tagging @amwx who did the implementation for FlyoutBase.

[avaloniaui-team]

You can test this PR using the following package version. 0.10.999-cibuild0021861-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[grokys]

Taken a look at UWP, and it looks like there, no passthrough is being set. Here's my test code:

  <StackPanel>
    <Button x:Name="b">
      Hello
      <Button.Flyout>
        <MenuFlyout>
          <MenuFlyoutItem>Foo</MenuFlyoutItem>
        </MenuFlyout>
      </Button.Flyout>
    </Button>

    <CheckBox IsChecked="{Binding ElementName=b, Path=IsPointerOver}">PointerOver</CheckBox>
  </StackPanel>

You can see that hovering over the button with the flyout open does not set PointerOver on the button meaning that there's no passthrough.

[grokys]

From the related issue:

Instead, it seems like PointerPressed triggers light dismiss layer to close the popup. And same click PointerReleased triggers button to open the popup. Default ClickMode is Release.

This actually sounds like the root of the problem. Even when ClickMode is Release the button shouldn't be triggering a click without an associated pointer down event.

[pr8x]

This actually sounds like the root of the problem. Even when ClickMode is Release the button shouldn't be triggering a click without an associated pointer down event.

I think this is due to OverlayDismissEventPassThrough (which is set for all Flyouts by default). It will trigger a pointer down on the button which will make it handle the release event later. The right solution probably is to not use OverlayInputPassThroughElement and in case OverlayDismissEventPassThrough is set somehow ignore the button the flyout was opened from? Not sure how to do this though - API wise.

[avaloniaui-team]

You can test this PR using the following package version. 0.10.999-cibuild0022327-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[maxkatz6]

@pr8x @grokys
Let's do next:

1. Current changes in Button.cs look good.
2. Feature wise, we need Flyout.OverlayInputPassThroughElement anyway, so it should be added. As it was in Add OverlayInputPassThroughElement to FlyoutBase. #8446.
3. Let's not set Flyout.OverlayInputPassThroughElement by default (keep it null). But set OverlayDismissEventPassThroughProperty to false. With this combination I can't reproduce original issue.

@grokys do you remember why OverlayDismissEventPassThroughProperty was set to true on the ContextMenu (and later copied to the FlyoutBase)?

I am actually not sure if we even need Flyout.OverlayDismissEventPassThroughProperty property, we can just disable it on the inner popup without giving customization. Flyout.OverlayInputPassThroughElement should be enough. Unless @grokys is against it.

[pr8x]

Current changes in Button.cs look good.

Actually when we disable OverlayDismissEventPassThroughProperty we don't need any extra logic inside Button. The toggling should work out-of-the-box.

[maxkatz6]

The toggling should work out-of-the-box.

True. I just think it makes sense to toggle flyout OnClick instead of just opening.

[timunie]

For Context Menu I can imagine the user wants to open it at a different position once clicked on a different position of the same control. Like when you have a collection of images. Have to double check if that's the same behavior in normal Windows or just in a particular App I have in mind.

[maxkatz6]

@timunie oh, it's actually a good point

[timunie]

@timunie oh, it's actually a good point

Okay, So I double-checked this and it's not consistent over the programs I use:

Close ContextMenu on second right click:

• Gimp
• Inkscape

Open ContextMenu at another position on second right click:

• Win 10 Desktop
• Word
• Excel
• CATIA
• DoubleCommander

List could be extended if needed.

[grokys]

Yeah, seems like a platform-specific thing. It's probably something we could change per-platform once we have a platform settings API, but for the moment we should probably just stick with the existing behavior for ContentMenu. Looks like OverlayDismissEventPassThrough can be removed for Flyout though as this behavior doesn't apply.

[pr8x]

To make a context menu open again at different mouse position we only need OverlayInputPassThroughElement actually (and set it to the control that opened the context menu). Not sure if we even need OverlayDismissEventPassThrough tbh

[pr8x]

@maxkatz6 Fixed it.

[avaloniaui-team]

You can test this PR using the following package version. 0.10.999-cibuild0022630-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[avaloniaui-team]

You can test this PR using the following package version. 0.10.999-cibuild0022728-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]