Skip to content
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

Fix focus transition issues related to dialogs and UAC in Virtualization #3640

Merged
merged 3 commits into from
Aug 26, 2024

Conversation

nieubank
Copy link
Contributor

Summary of the pull request

This change ensures the narrator reads the committing changes and restart dialog without manual reapplication of focus.

The progress modal dialog is disrupted by a race with the UAC prompt focusing the narrator. Additionally, having one content dialog that shares two presentations make this difficult to orchestrate with focus and accessibility, to simplify things, the modify features dialog is split:

  • One dialog hosts the committing changes and is shown immediately after starting to apply features, removing the race condition with the UAC dialog.
  • Another dialog for the restart action is shown after the process completes.

References and relevant issues

#3639

Detailed description of the pull request / Additional comments

Validation steps performed

Apply a feature with narrator enabled

PR checklist

nieubank and others added 2 commits August 19, 2024 11:24
Co-authored-by: Kristen Schau <47155823+krschau@users.noreply.github.com>
Co-authored-by: Kristen Schau <47155823+krschau@users.noreply.github.com>
@nieubank nieubank added the Needs-Second Pull request that needs another approval label Aug 20, 2024
<comment>Message instructing the user to restart their machine to apply changes</comment>
</data>
<data name="RestartDialog.PrimaryButtonText" xml:space="preserve">
<value>Restart now</value>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a random thought. Is it clear to the user that this is to restart the machine and not just DevHome?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a good question. I'll follow up with our content writer to see if more clarity is needed there.

@nieubank nieubank removed the Needs-Second Pull request that needs another approval label Aug 26, 2024
@nieubank nieubank merged commit 2128607 into main Aug 26, 2024
4 checks passed
@nieubank nieubank deleted the user/nieubank/custacc branch August 26, 2024 21:18
@krschau krschau removed this from the Dev Home v0.18 milestone Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants