-
-
Notifications
You must be signed in to change notification settings - Fork 920
fix(controls): Fix ContentDialog interaction, focus, and modal behavior issues #1601
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
base: main
Are you sure you want to change the base?
Conversation
|
This solution is still not mature. If the ContentDialog does not contain focusable controls, assistive tools will keep focus on the host window, and the UI automation suppression cannot take effect. I am closing this PR again until a stable method is identified. |
|
That's really unfortunate. |
|
The issue appears to be resolved. I'm reopening this PR. |
|
Implementation is feature-complete. Cleanup tasks are deferred until the associated bug fix PR (#1615) is reviewed/merged. |
85ee1a2 to
eb44982
Compare
eb44982 to
c006d7c
Compare
|
I have consolidated the history into a single, clear commit for easier review. All changes are included. |
afb4415 to
4d67845
Compare
…or issues - Introduce ContentDialogHost to intercept host window input, implement focus preservation/restoration, and maintain backward compatibility with existing ContentPresenter - Add initial focus support to ContentDialog, aligning with WinUI and Windows App ContentDialog design guidelines - Implement ContentDialogAutomationPeer to allow UI automation/assistive tools to recognize ContentDialog as a modal window - Fix issue where opening a new ContentDialog before closing the previous one could cause ShowAsync to hang
4d67845 to
8dc25b3
Compare
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
ContentDialog is intended to behave as a modal dialog, providing consistent interaction, predictable focus handling, and proper isolation from the host window.
However, in its current implementation, several interaction, robustness, and accessibility issues can be observed under common usage scenarios—especially when keyboard navigation, access keys, dialog chaining, and automation tools are involved.
Interaction & Robustness Issues
When both ContentDialog and the host window have default/cancel buttons, incorrect key responses may be triggered
Host window access keys (e.g.,
&Execute/_Executebuttons) and shortcuts can still be activated while the dialog is openInitial focus logic does not follow Windows App design guidelines, violating user interaction habits
Keyboard operations cannot be performed when only a close button is present
Pressing Ctrl + Tab or (Up/Down/Left/Right) can move the focus out of the dialog
The focus will not return to its original position after the ContentDialog is closed
Opening a new ContentDialog before the previous one is closed causes task hanging—the
ShowAsyncmethod never returns.Typical scenario: Navigation between dialogs.
UI Automation/Accessibility Issues
Assistive tools can move keyboard focus outside the dialog boundaries
ContentDialog lacks proper modal dialog characteristics for automation
Issue Number: #1585, #1618
What is the new behavior?
This PR introduces a new
ContentDialogHostand refines the behavior, accessibility, and reliability ofContentDialog, while preserving backward compatibility.Added
ContentDialogHost. While a dialog is shown, the host layer intercepts access key events and preview command events, clears the host window’sInputBindingsandCommandBindingsto block various input interaction paths, and restores them when the dialog is closed.ContentDialogHostnow stores the focused element before the dialog is shown and attempts to restore focus after the dialog is closed.Kept support for the legacy host (
ContentPresenter). Existing code continues to work without changes, but compiling with the legacy host will emit deprecation warnings.Introduced
ContentDialogHostControllerto maintain compatibility and allow unified handling of events for both host types.Added
DisableSiblingsEnabletoContentDialogHostfor high-safety scenarios. When enabled, sibling elements of the host are disabled while the dialog is displayed. For the legacy host (ContentPresenter), this can be enabled via the attached propertyContentDialogHostBehavior.DisableSiblingsEnable="True".Refactored
ContentDialogfocus logic into partial classes. The initial focus behavior is now aligned with WinUI and avoids focusing “destructive / warning” buttons by default, in accordance with the Windows App Design Guidelines.Updated the
ContentDialogstyle to includeKeyboardNavigation.ControlTabNavigation = Cycle. Pressing Ctrl+Tab no longer allows focus to escape the dialog. With a defined initial focus, directional navigation also remains within the dialog. If developers override theContentDialogtemplate, they must explicitly setKeyboardNavigation.TabNavigation,KeyboardNavigation.DirectionalNavigation, andKeyboardNavigation.ControlTabNavigation.Added
ContentDialogAutomationPeerso that automation and accessibility tools recognizeContentDialogas a modal dialog.Fixed an issue where opening a new
ContentDialogbefore the previous one was closed could causeShowAsyncto hang.Below are some demonstration videos illustrating the behavior after the fixes have been applied.
ContentDialog-SafeInitialFocus.mp4
ContentDialog-UIAutomation.mp4
ContentDialog-AutomationContainment.mp4
Other information
Since all issues originate from
ContentDialog, submitting separate PRs for each fix would be inefficient. Additionally, the fixes are interrelated and prone to conflicts. Therefore, they are addressed comprehensively in this single PR.Fixed Close button of simple ContentDialog does not react to Enter key #1585
Fixed [Critical] ContentDialog causes memory leaks when ShowAsync task hangs #1618