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

Breadcrumbs: focus needs love #54745

Closed
isidorn opened this issue Jul 20, 2018 · 17 comments
Closed

Breadcrumbs: focus needs love #54745

isidorn opened this issue Jul 20, 2018 · 17 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues breadcrumbs feature-request Request for new features or functionality info-needed Issue requires more information from poster

Comments

@isidorn
Copy link
Contributor

isidorn commented Jul 20, 2018

I feel like the focus moving needs more work in the breadcrumbs. Here are my suggestion:

  1. When focus on first tree element UP will move focus to the actual breadcrumb which is the parent of the tree
  2. It is possible to use Tab to focus the whole breadcrumb thing (once the focus is there use arrow keys to move focus). They would optimaly get focus after the tab area
  3. Tab inside the tree should move focus out of the breadcrumbs (and not navigate to that item like now, enter is for that)
  4. Focused element in the breadcrumb has a stronger visual feedback (especially hard to spot for the ...)
  5. Once we have that all figured out I would expect that we announce what keys we use for navigation once breadcrumbs is focused

Shift + left / right is nice but people will not discover it imho. Also why did you use shift, and not alt since that is the alternative modifier which we use across the workbench.

@isidorn isidorn added feature-request Request for new features or functionality accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues breadcrumbs labels Jul 20, 2018
@jrieken jrieken added this to the July 2018 milestone Jul 24, 2018
@jrieken
Copy link
Member

jrieken commented Jul 25, 2018

When focus on first tree element UP will move focus to the actual breadcrumb which is the parent of the tree

That is: #53950

@jrieken
Copy link
Member

jrieken commented Jul 25, 2018

Tab inside the tree should move focus out of the breadcrumbs (and not navigate to that item like now, enter is for that)

Not sure about that. I added tab on request of @Tyriar. It's like the tab behaviour in IntelliSense

@Tyriar
Copy link
Member

Tyriar commented Jul 25, 2018

@jrieken here's my expectation vs current:

  • Breadcrumb titles are focused
    • tab
      • Expectation: Moves to the next breadcrumb, if the last breadcrumb is focused then focus the editor
      • Current: Same
    • shift+tab:
      • Expectation: Moves focus to the previous breadcrumb, if the first breadcrumb is focused then focus the last editor group action
      • Current: Focuses the last editor group action
  • Breadcrumb menus are focused
    • tab
      • Expectation: Focus the next breadcrumb's menu (shift+right), if the last breadcrumb is focused then focus the editor
      • Current: Nothing ⚠️
    • shift+tab
      • Expectation: Focus the previous breadcrumb's menu (shift+left), if the first breadcrumb is focused then focus the last editor group action
      • Current: Focuses the end of the workbench ⚠️

@jrieken
Copy link
Member

jrieken commented Jul 25, 2018

@Tyriar So, you are suggesting to use tab/shift+tab (instead of arrows) for the in-widget navigation, right?

@isidorn
Copy link
Contributor Author

isidorn commented Jul 25, 2018

Not a big fan of that since in all other trees we use arrows. We would not be consistent.

@Tyriar
Copy link
Member

Tyriar commented Jul 25, 2018

@isidorn in the tree we do use arrows, I want tab to navigate the top-level breadcrumbs while the menu is focused, or at the very least focus the editor/editor group actions.

@jrieken
Copy link
Member

jrieken commented Jul 25, 2018

I want tab to navigate the top-level breadcrumbs while the menu is focused,

Does "menu" mean the breadcrumb picker? So, you want tab to move the picker or to close the picker and to focus the breadcrumbs?

@Tyriar
Copy link
Member

Tyriar commented Jul 25, 2018

@jrieken by menu I mean the thing that pops up when you click a breadcrumb.

jrieken added a commit that referenced this issue Jul 26, 2018
@jrieken jrieken closed this as completed Jul 27, 2018
@sbatten sbatten added the verification-needed Verification of issue is requested label Aug 1, 2018
@JacksonKearl JacksonKearl added verified Verification succeeded verification-found Issue verification failed and removed verification-needed Verification of issue is requested verified Verification succeeded labels Aug 1, 2018
@JacksonKearl
Copy link
Contributor

Verified for

  • Better emphasis (underline)
  • Ctrl instead of shift
  • Tab leaves menu
  • Tab focuses menu, then arrows move in menu

Not verified

  • We don't "announce the keys".
  • Up from top element doesn't close dropdown

Not mentioned here, but related: space opens the menu for the right of the hilighted item, not the item itself

@JacksonKearl JacksonKearl reopened this Aug 1, 2018
@isidorn
Copy link
Contributor Author

isidorn commented Aug 2, 2018

Thanks for verifiying, the remaining issue can be looked in august

@isidorn isidorn modified the milestones: July 2018, August 2018 Aug 2, 2018
@jrieken
Copy link
Member

jrieken commented Aug 6, 2018

Up from top element doesn't close dropdown

That is: #53950

We don't "announce the keys".

@JacksonKearl What does that mean?

@jrieken jrieken added info-needed Issue requires more information from poster and removed verification-found Issue verification failed labels Aug 6, 2018
@JacksonKearl
Copy link
Contributor

@jrieken I’m not sure, that’s just the wording from the original issue.

@jrieken
Copy link
Member

jrieken commented Aug 6, 2018

Well, why did you reopen this issue?

@JacksonKearl
Copy link
Contributor

I added verification-found and reopened because part of the original issue was unaddressed. Is that not correct?

@jrieken
Copy link
Member

jrieken commented Aug 6, 2018

Yeah, I think it is not correct. According to your comment (that's my understanding) you reopened this because of two things: 'We don't "announce the keys".' and 'Up from top element doesn't close dropdown'. Is that correct? Iff so, the latter is a dupe and with the former you are not sure what it means.

@JacksonKearl
Copy link
Contributor

@isidorn could you clarify what you meant about announcing the keys?

@jrieken
Copy link
Member

jrieken commented Aug 6, 2018

Once we have that all figured out I would expect that we announce what keys we use for navigation once breadcrumbs is focused

This means that we decide, after everything else is done, on the keybindings we use for navigating breadcrumbs.

@jrieken jrieken removed this from the August 2018 milestone Aug 27, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues breadcrumbs feature-request Request for new features or functionality info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

5 participants