feat(focus-trap): add the ability to specify a focus target #1752
Conversation
googlebot
added
the
cla: yes
| @@ -47,6 +49,12 @@ export class FocusTrap { | |||
| return root; | |||
| } | |||
|
|
|||
| let focusTarget = root.querySelector(FOCUS_TARGET_SELECTOR) as HTMLElement; | |||
There was a problem hiding this comment.
We don't want to call querySelector here- this method is used recursively, you would would potentially end up calling querySelector many times. This should happen outside of the recursive method.
| @@ -1,6 +1,8 @@ | |||
| import {Component, ViewEncapsulation, ViewChild, ElementRef} from '@angular/core'; | |||
| import {InteractivityChecker} from './interactivity-checker'; | |||
|
|
|||
| /** Selector for nodes that should have a higher priority when looking for focus targets. */ | |||
| const FOCUS_TARGET_SELECTOR = '[md-focus-target]'; | |||
There was a problem hiding this comment.
I'd still like to have separate md-focus-start and md-focus-end attributes, even if they're not directives. With only one attribute, users either have to specify it exactly zero times or more than one time- a user just trying to set where the focus should start would also inadvertently be setting where the focus should wrap backwards to.
crisbeto
force-pushed
the
1468/focus-target
branch
from
87ebbd6
to
cb8226a
Compare
|
Updated to address the comments. |
Adds the ability to specify an element that should take precedence over other focusable elements inside of a focus trap. Fixes angular#1468.
crisbeto
force-pushed
the
1468/focus-target
branch
from
cb8226a
to
e54098e
Compare
|
LGTM Could you also update the dialog README with this? |
jelbourn
added
pr: lgtm
action: merge
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Adds the ability to specify an element that should take precedence over other focusable elements inside of a focus trap.
Fixes #1468.
Note: @jelbourn we discussed that we should have separate
startandenddirectives, which we'd query via@ContentChildor@ViewChild, however I wasn't able to get it working. I was testing with the dialog demo and I think that either the element being nested in multiple other components is breaking it, or theportal(which is used to inject the dialog content) isn't working properly. I've also tried injecting the focus trap into a focus target directive, but it didn't work either.