-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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.
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
Update Flyout and Popup APIs to make gap between them smaller #10492
Conversation
src/Avalonia.Controls/Primitives/PopupPositioning/IPopupPositioner.cs
Outdated
Show resolved
Hide resolved
You can test this PR using the following package version. |
An additional thought since you're in this code (though could be done separately): it might be good to add |
public static readonly DirectProperty<PopupFlyoutBase, IInputElement?> OverlayInputPassThroughElementProperty = | ||
Popup.OverlayInputPassThroughElementProperty.AddOwner<PopupFlyoutBase>( | ||
o => o._overlayInputPassThroughElement, | ||
(o, v) => o._overlayInputPassThroughElement = v); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably could too? (obviously requires the change in Popup)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this property. Doesn't feel like OverlayInputPassThroughElementProperty is even needs to be styled, as it tied too specific.
Related, maybe it can be fixed in this PR: From Or should I open a new issue? |
Good idea, added.
Couldn't reproduce it on control catalog for some reason. But I see possible reason, that _lastPointer was reset on pointer leaving the screen. If you can verify, please take a look. Committed in this branch. |
I want to rewrite tooltips to use Popup internally instead of low level components. Probably in another PR... |
# Conflicts: # src/Avalonia.Controls/Flyouts/FlyoutBase.cs
Would be nice to add support for |
You can test this PR using the following package version. |
Now after moving the window it shows at the old location the first time, then at the "correct" location (not pointer, but center of control), so it's much better, but not completely fixed. Isn't it possible to update the pointer position before processing pointer events here: https://github.com/AvaloniaUI/Avalonia/pull/10492/files#diff-1c13e366d9f7efa03e332d42d84e61e1901fdd6d05a616c223edb87bcc02330fR151-R170? Or would that break other code? |
@jp2masa yes, it should work. |
You can test this PR using the following package version. |
@maxkatz6 It's fixed, thanks! |
@maxkatz6 Flyout has Avalonia/src/Avalonia.Controls/Primitives/Popup.cs Lines 237 to 244 in e9cd5d5
Avalonia/src/Avalonia.Controls/Flyouts/PopupFlyoutBase.cs Lines 67 to 74 in e9cd5d5
|
Default behavior was changed after AvaloniaUI#10492
@robloo historical differences I suppose. Same with Tooltip. |
I think Placement is a bit better of a name personally. Microsoft did too since that's what they changed it to in UWP and WinUI. The issue is it isn't a mode of placement, it is actually placement. If you don't want to break existing apps ToolTip though, that is understandable. |
@robloo problem with "Placement" is that we have "PlacementGravity", "PlacementAnchor", "PlacementRect", "PlacementTarget"... "PlacementMode" fits here without any confusion as one of equally configurable properties. |
@maxkatz6 Well, I don't have a strong preference here either so perhaps compatibility with existing code wins. "Mode" just seems like a little misnomer to me based on what the property does. It still has the Placement prefix to associate with all other related members as well. I realize you are referring to "Placement" as more of a grouping term though so naming a memer the same as the member group prefix name may be strange. |
@maxkatz6 ToolTip has a Therefore |
@kekekeks @amwx
What does the pull request do?
Breaking changes
Behavior change.
Popup PlacementMode=Top|Bottom|Left|Right behavior was changed. Now it is centered and matches old flyout placement mode values. For example, previously
Popup PlacementMode="Top"
was the same as TopEdgeAlignedLeft currently is.Also, FlyoutPlacementMode enum was removed, but should be a pretty small breaking change.
Fixed issues
Fixes #9583
Fixes #9582 (provides an alternative solution)