Update popups and flyouts to properly support OverlayDismissEventPassThrough #15517
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o possibly closing the popup when a pointer is pressed. Added the PopupFlyoutBase.OverlayDismissEventPassThrough property and updated logic in Button.
…l. This was problematic when the popup was open with OverlayDismissEventPassThrough and clicking onto another control. Focus would not move to the clicked control.
|
|
@cla-avalonia agree |
|
@billhenn thanks! Would it be possible to add some tests for Flyout control at least? https://github.com/AvaloniaUI/Avalonia/blob/master/tests/Avalonia.Controls.UnitTests/FlyoutTests.cs Also, Clicking_On_Control_PseudoClass test is failing now with this PR, for some reason pseudoclasses are not applied. |
maxkatz6
added
the
backport-candidate-11.1.x
…nize pseudo-class behavior change.
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
maxkatz6
approved these changes
May 9, 2024
grokys
pushed a commit
that referenced
this pull request
Aug 2, 2024
…Through (#15517) * Updated Popup to raise the pass-through overlay dismiss event prior to possibly closing the popup when a pointer is pressed. Added the PopupFlyoutBase.OverlayDismissEventPassThrough property and updated logic in Button. * Updated SplitButton logic to handle OverlayDismissEventPassThrough scenarios. * Updated CalendarDatePicker logic to handle OverlayDismissEventPassThrough scenarios. * Updated ComboBox logic to handle OverlayDismissEventPassThrough scenarios. * Removed unncessary ComboBox.PopupClosed logic that focused the control. This was problematic when the popup was open with OverlayDismissEventPassThrough and clicking onto another control. Focus would not move to the clicked control. * Fixed the Clicking_On_Control_PseudoClass unit test to properly recognize pseudo-class behavior change. * Added a couple unit tests to FlyoutTests.
grokys
added
backported-11.1.x
and removed
backport-candidate-11.1.x
grokys
pushed a commit
that referenced
this pull request
Aug 2, 2024
…Through (#15517) * Updated Popup to raise the pass-through overlay dismiss event prior to possibly closing the popup when a pointer is pressed. Added the PopupFlyoutBase.OverlayDismissEventPassThrough property and updated logic in Button. * Updated SplitButton logic to handle OverlayDismissEventPassThrough scenarios. * Updated CalendarDatePicker logic to handle OverlayDismissEventPassThrough scenarios. * Updated ComboBox logic to handle OverlayDismissEventPassThrough scenarios. * Removed unncessary ComboBox.PopupClosed logic that focused the control. This was problematic when the popup was open with OverlayDismissEventPassThrough and clicking onto another control. Focus would not move to the clicked control. * Fixed the Clicking_On_Control_PseudoClass unit test to properly recognize pseudo-class behavior change. * Added a couple unit tests to FlyoutTests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have found that clicking outside of popups/flyouts takes an extra click, since the first click dismisses the popup, and a second click is required to focus/activate whatever target control was clicked. This is a productivity annoyance to desktop users who are used to single clicking on a control outside of a popup to focus/activate it.
There is a
Popup.OverlayDismissEventPassThroughproperty that can be set totrue, which is meant to help with this. However, flyouts don't support it at all, and the pass-through event should be raised prior to the dismiss overlay closing the popup. This is in case the pass-through event closed the popup already, which should occur inButton,ComboBox, and several other native controls.Related Issues/PRs
A related issue was opened recently by another user, requesting that
OverlayDismissEventPassThroughbe added toPopupFlyoutBase.Another past PR worked to prevent
Buttonwith a flyout open from closing and instantly reopening the flyout when it was clicked. It accomplished this by forcing the flyout popup'sOverlayDismissEventPassThroughproperty tofalse. While it achieved that goal, it prevented flyouts from ever supporting event pass-through, which is not ideal.Goals
The intended goal of the code changes in this new PR are:
OverlayDismissEventPassThroughis supported on bothPopupandPopupFlyoutBase.OverlayDismissEventPassThroughistrue, any clicks to another control outside of the popup will work as expected, thereby preventing a second click from being necessary to dismiss the popup.OverlayDismissEventPassThroughistrue.OverlayDismissEventPassThroughisfalse, the same run-time behavior should be achieved as before this PR.OverlayDismissEventPassThroughscenario, so they can close their own popup.Implementation Details
PopupPointerPressedDismissOverlaylogic so the pass-through event fires first, then the popup is closed if the pass-through event didn't already close it.PopupFlyoutBaseOverlayDismissEventPassThroughproperty and assign it to thePopupused by the flyout.CreatePopupmethod to remove thePopup.OverlayDismissEventPassThroughfrom always beingfalse.ButtonOnPointerPressedto watch forOverlayDismissEventPassThroughscenarios when a flyout is open and close the flyout without marking the button as pressed.SplitButtonPreviewPointerPressedhandler to watch forOverlayDismissEventPassThroughscenarios when a flyout is open and close the flyout.CalendarDatePickerDropDownButton_PointerPressedto watch forOverlayDismissEventPassThroughscenarios when a flyout is open and close the flyout.ComboBoxOnPointerPressedto watch forOverlayDismissEventPassThroughscenarios when a drop-down is open and close the drop-down. If no popup is open, theComboBoxis flagged as pressed.OnPointerReleasedto only open the drop-down if theComboBoxis flagged as pressed.PopupClosedlogic to remove code that blindly focuses itself when the drop-down closes. This was bad because whenOverlayDismissEventPassThroughwastrueon theComboBox's popup, clicking on an external control while the drop-down was open would not let the other control get focus. With the focus code removed, theComboBoxis still properly focused when the popup is closed by clicking on theComboBox, pressingEsc, etc. Therefore, the focus code was removed since it was unnecessary and caused problems.Contact me if you have any questions on the code changes.