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

Support more meaningful options for dialog autoFocus #22678

Closed
jelbourn opened this issue May 12, 2021 · 2 comments
Closed

Support more meaningful options for dialog autoFocus #22678

jelbourn opened this issue May 12, 2021 · 2 comments
Assignees
Labels
Accessibility This issue is related to accessibility (a11y) feature This issue represents a new feature or feature request rather than a bug or bug fix P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@jelbourn
Copy link
Member

Currently, the autoFocus option of MatDialogConfig is a boolean, with the values meaning:

  • false - default value, focused the role="dialog" element
  • true - focuses the first tabbable element inside the dialog

This API is kind of confusing, since the dialog automatically focuses something regardless of what you do, so autoFocus: false doesn't really make sense.

Instead, we should change autoFocus to accept these values:

  • dialog - focus the role="dialog" element
  • first-tabbable - focus the first tabbable element
  • first-header - focus the first header (h1 - h6) element in the dialog. It's still an open question whether this should force tabIndex = -1.
  • A CSS selector- arbitrary CSS selector, the dialog would focus the first matching element (vis querySelector). Again an open question if this should force tabIndex = -1.
@jelbourn jelbourn added feature This issue represents a new feature or feature request rather than a bug or bug fix P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent Accessibility This issue is related to accessibility (a11y) labels May 12, 2021
@zelliott
Copy link
Collaborator

I might call it first-heading instead of first-header (https://html.spec.whatwg.org/multipage/sections.html#the-h1,-h2,-h3,-h4,-h5,-and-h6-elements) (wouldn't want it to be confused with <header>).

I'll throw a vote in for forcing the element to be focusable. I feel like forgetting to add tabindex will otherwise be a super common gotcha and result in bug reports that these values aren't working as expected.

The logic could be something like:

  1. If tabindex is specified, do nothing.
  2. Else, set tabindex to -1, and add self-removing event listeners to remove the tabindex once the element is blurred or mousedown-ed. You don't want to keep the element focusable because it will mess with a user setting the sequential focus starting point via "click to move focus" (https://googlechrome.github.io/samples/focus-navigation-start-point/).

The logic to "force-focus" an element and revert it back to normal afterward could be pulled out into a common helper function.

amysorto added a commit to amysorto/components that referenced this issue Jun 2, 2021
Before this PR, autoFocus was a boolean that allowed users to specify whether the container element or the first tabbable element is focused on dialog open. Now you can also specify focusing the first header element or use a CSS selector and focus the first element that matches that. If these elements can't be focused, then the container element is focused by default.

This applies to other components that are similar to dialog and also have a autoFocus field.

Fixes angular#22678
amysorto added a commit to amysorto/components that referenced this issue Jun 3, 2021
Before this PR, autoFocus was a boolean that allowed users to specify whether the container element or the first tabbable element is focused on dialog open. Now you can also specify focusing the first header element or use a CSS selector and focus the first element that matches that. If these elements can't be focused, then the container element is focused by default.

This applies to other components that are similar to dialog and also have a autoFocus field.

Fixes angular#22678
amysorto added a commit to amysorto/components that referenced this issue Jun 3, 2021
Before this PR, autoFocus was a boolean that allowed users to specify whether the container element or the first tabbable element is focused on dialog open. Now you can also specify focusing the first header element or use a CSS selector and focus the first element that matches that. If these elements can't be focused, then the container element is focused by default.

This applies to other components that are similar to dialog and also have a autoFocus field.

Fixes angular#22678
amysorto added a commit to amysorto/components that referenced this issue Jun 3, 2021
Before this PR, autoFocus was a boolean that allowed users to specify whether the container element or the first tabbable element is focused on dialog open. Now you can also specify focusing the first header element or use a CSS selector and focus the first element that matches that. If these elements can't be focused, then the container element is focused by default.

This applies to other components that are similar to dialog and also have a autoFocus field.

Fixes angular#22678
amysorto added a commit to amysorto/components that referenced this issue Jun 4, 2021
Before this PR, autoFocus was a boolean that allowed users to specify whether the container element or the first tabbable element is focused on dialog open. Now you can also specify focusing the first header element or use a CSS selector and focus the first element that matches that. If these elements can't be focused, then the container element is focused by default.

This applies to other components that are similar to dialog and also have a autoFocus field.

Fixes angular#22678
amysorto added a commit to amysorto/components that referenced this issue Jun 4, 2021
Before this PR, autoFocus was a boolean that allowed users to specify whether the container element or the first tabbable element is focused on dialog open. Now you can also specify focusing the first header element or use a CSS selector and focus the first element that matches that. If these elements can't be focused, then the container element is focused by default.

This applies to other components that are similar to dialog and also have a autoFocus field.

Fixes angular#22678
amysorto added a commit to amysorto/components that referenced this issue Jun 8, 2021
Before this PR, autoFocus was a boolean that allowed users to specify whether the container element or the first tabbable element is focused on dialog open. Now you can also specify focusing the first header element or use a CSS selector and focus the first element that matches that. If these elements can't be focused, then the container element is focused by default.

This applies to other components that are similar to dialog and also have a autoFocus field.

Fixes angular#22678
amysorto added a commit to amysorto/components that referenced this issue Jul 26, 2021
Before this PR, autoFocus was a boolean that allowed users to specify whether the container element or the first tabbable element is focused on dialog open. Now you can also specify focusing the first header element or use a CSS selector and focus the first element that matches that. If these elements can't be focused, then the container element is focused by default.

This applies to other components that are similar to dialog and also have a autoFocus field.

Fixes angular#22678
mmalerba pushed a commit to amysorto/components that referenced this issue Jul 27, 2021
Before this PR, autoFocus was a boolean that allowed users to specify whether the container element or the first tabbable element is focused on dialog open. Now you can also specify focusing the first header element or use a CSS selector and focus the first element that matches that. If these elements can't be focused, then the container element is focused by default.

This applies to other components that are similar to dialog and also have a autoFocus field.

Fixes angular#22678
amysorto added a commit to amysorto/components that referenced this issue Jul 28, 2021
Before this PR, autoFocus was a boolean that allowed users to specify whether the container element or the first tabbable element is focused on dialog open. Now you can also specify focusing the first header element or use a CSS selector and focus the first element that matches that. If these elements can't be focused, then the container element is focused by default.

This applies to other components that are similar to dialog and also have a autoFocus field.

Fixes angular#22678
@zarend zarend closed this as completed in 769996e Jul 30, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) feature This issue represents a new feature or feature request rather than a bug or bug fix P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

3 participants