Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(sidenav): trigger the focus handler for autofocus target too. #6101

Closed
wants to merge 1 commit into from

Conversation

devversion
Copy link
Member

This fixes the current issue with autofocus for other directives like autocomplete

Fixes #5665

@ThomasBurleson
Copy link
Contributor

@robertmesserle, @jelbourn - Can you review for today's build?

@ThomasBurleson ThomasBurleson added this to the 1.0-rc7 milestone Dec 7, 2015
@ThomasBurleson ThomasBurleson self-assigned this Dec 7, 2015
focusEl && focusEl.focus();
if (scope.isOpen && focusEl) {
focusEl.focus();
focusEl.triggerHandler('focus');
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this is necessary. Invoking focus on an element should always trigger any event listeners to be called. @devversion can you explain?

Copy link
Member Author

Choose a reason for hiding this comment

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

The same question I asked myself too. But as I figured out, focus() can't be applied to all type of elements. And because the sidenav is a custom angular directive it looks like, they can't be focused through the focus() function. That's why I trigger that focus handlers manually, and its working perfect. In my other PR, which already got reviewed by Elad, I'm using the same strategy to get that running (see #5943).

Quote MDN

  • The HTMLElement.focus() method sets focus on the specified element, if it can be focused.

Copy link
Member

Choose a reason for hiding this comment

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

Custom elements simply need a tabindex in order to receive focus:
http://codepen.io/jelbourn/pen/MKYpJm?editors=101

Copy link
Member Author

Choose a reason for hiding this comment

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

All right, thanks for that codepen demonstration.

devversion added a commit to devversion/material that referenced this pull request Dec 7, 2015
devversion added a commit to devversion/material that referenced this pull request Dec 7, 2015
devversion added a commit to devversion/material that referenced this pull request Dec 7, 2015
@devversion devversion closed this Dec 7, 2015
devversion added a commit to devversion/material that referenced this pull request Dec 7, 2015
jelbourn pushed a commit that referenced this pull request Dec 8, 2015
@devversion devversion deleted the fix/sidenav-autofocus branch April 21, 2016 11:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

md-autofocus does not work when md-autocomplete inside a md-sidenav
3 participants