Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 6, 2025

Fixes #122054

Description

In Settings Sync sign-in dialog (and other quick picks with toggles), Tab stops at each checkbox individually before reaching action buttons, violating focus group patterns. Users must tab through all checkboxes to reach "Sign in and Turn on" button.

Changes

Modified src/vs/base/browser/ui/findinput/findInput.ts:

  • Implemented focus group pattern: only first toggle has tabIndex=0, rest have tabIndex=-1
  • Extended arrow key navigation to include additional toggles (previously only worked for common find toggles)
  • Added updateToggleTabIndexes() method to maintain tab order when toggles change

Added src/vs/base/test/browser/ui/findinput/findInput.test.ts:

  • Tests verify single tab stop for all toggle combinations
  • Covers common toggles only, additional toggles only, and mixed scenarios

Result: Tab skips toggle group entirely; Left/Right arrows navigate within group; Escape returns to input.

Original prompt

This section details on the original issue you should resolve

<issue_title>Sign in quick pick dialog: focus movement could be improved</issue_title>
<issue_description>1. Turn on Settings sync
2. Get the following dialog

Press tab - focus moves to checkbox. Press tab again, focus moves to the other checkbox.

Ideally the first tab would move focus to the "Sign in and Turn on" button. And not to the checkbox

Screenshot 2021-04-28 at 15 42 44
</issue_description>

<agent_instructions>The list of check boxes should be a focus group which can be navigated with arrows and require only a single Tab to go in or out
</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@TylerLeonhardt VoiceOver doesn't seem to be reading the description anymore both before and after this change... we may have to revert #122054... @isidorn can you give it a try on your end? @isidorn Yes I can reproduce. Well if VoiceOver was not reading the description before the change for #122054 then there is no point in reverting. Or I misunderstood something?

The issue here is that we are using aria-activedescendent to tell the screen reader what is the active element, and based on that the screen reader will read.
It would be cool if there is a way to append a label to be read by a screen reader apart from the one provided by the activedescendent. @joanmarie might you have a hint on how to do this? So A contains B, browser focus is on A, A has activedescendent pointing to B. Is there a way for A to append something to what is read by the screen reader?

@TylerLeonhardt If there is not nice way then we will have to use an aria alert to announce the placeholder</comment_new>
<comment_new>@TylerLeonhardt
I'm also concerned that if I change the focus to the Sign in and Turn on button that it might affect keybindings:

if (!visibilities.inputBox) {
// we need to move focus into the tree to detect keybindings
// properly when the input box is not visible (quick nav)
this.ui.list.domFocus();
// Focus the first element in the list if multiselect is enabled
if (this.canSelectMany) {
this.ui.list.focus(QuickInputListFocus.First);
}
}

Well if VoiceOver was not reading the description before the change for #122054 then there is no point in reverting.

You are correct.

Anyway, wanted to call that out but still interested in hearing what @joanmarie has to say :)</comment_new>
<comment_new>@TylerLeonhardt
@joanmarie

What description and what placeholder? Are these two things the same?

Sorry about that. I think @isidorn got those mixed up. Let me provide more context.

Based on your text "Select an account to sign in" I think we are talking about two different quick picks. We are talking about the one that says "Please sign in to synchronize your data across devices" that has a "Sign in & turn on" button next to it.

@isidorn said placeholder before because the description and placeholder visually appear to be in the same place... but the placeholder is shown when the input box is enabled, and the description is shown instead when the input box is disabled.

Settings Sync disables the input for this quick pick so we are purely talking about the description here. Sorry for the confusion.

When should these things be read, every time the active descendant changes? Just the first time? Something else?

I... am just getting ramped up on A11y-related changes and am quite the noob, sorry... so I'll let @isidorn answer it correctly but ideally, the description should be read out when the quick pick is shown... so it might look like this:

  • click the person icon in the bottom left of vscode
  • click on "Turn on Settings Sync.."
  • That brings me to the "Please sign in to synchronize your data across devices" quick pick and the screen reader reads out "Please sign in to synchronize your data across devices" before saying anything about the button or quick pick list.

It's possible that you see a different quick pick before the "Please sign in to synchronize your data across devices" quick pick... if so, let me know what that one is and we can talk about that one too. I want to fix all the screen reader issues for quick pick :)</comment_new>


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits November 6, 2025 23:18
Co-authored-by: dmitrivMS <9581278+dmitrivMS@users.noreply.github.com>
Co-authored-by: dmitrivMS <9581278+dmitrivMS@users.noreply.github.com>
Co-authored-by: dmitrivMS <9581278+dmitrivMS@users.noreply.github.com>
@dmitrivMS dmitrivMS added the ~copilot-video-please ⚠️WIP🏗️ Use in PRs. Runs Copilot CLI agent to record a video of "trying out the change" label Nov 6, 2025
@vs-code-engineering vs-code-engineering bot removed the ~copilot-video-please ⚠️WIP🏗️ Use in PRs. Runs Copilot CLI agent to record a video of "trying out the change" label Nov 6, 2025
Co-authored-by: dmitrivMS <9581278+dmitrivMS@users.noreply.github.com>
@vs-code-engineering
Copy link

🤖 Copilot has tried out this change! Members of the team can view the recording here or download the Playwright trace here (run)

Co-authored-by: dmitrivMS <9581278+dmitrivMS@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve focus movement in sign in quick pick dialog Implement focus group for quick pick toggles to improve keyboard navigation Nov 6, 2025
Copilot finished work on behalf of dmitrivMS November 6, 2025 23:46
Copilot AI requested a review from dmitrivMS November 6, 2025 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sign in quick pick dialog: focus movement could be improved

2 participants