-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix(sidenav): trigger the focus handler for autofocus target too. #6101
Conversation
@robertmesserle, @jelbourn - Can you review for today's build? |
focusEl && focusEl.focus(); | ||
if (scope.isOpen && focusEl) { | ||
focusEl.focus(); | ||
focusEl.triggerHandler('focus'); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
This fixes the current issue with autofocus for other directives like
autocomplete
Fixes #5665