feat(focus-trap): allow setting initially focused element #4577
Conversation
googlebot
added
the
cla: yes
src/lib/core/a11y/focus-trap.ts
Outdated
| @@ -97,27 +101,47 @@ export class FocusTrap { | |||
| this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusLastTabbableElement()); | |||
| } | |||
|
|
|||
| private _getRegionMarker(marker: 'start' | 'end'): HTMLElement | null { | |||
There was a problem hiding this comment.
Add function description? Not immediately clear what a "region marker" is.
There was a problem hiding this comment.
Instead of "marker", maybe "boundary element"?
src/lib/list/list.ts
Outdated
| @@ -151,6 +149,7 @@ export class MdListItem implements AfterContentInit { | |||
|
|
|||
| constructor(private _renderer: Renderer2, | |||
| private _element: ElementRef, | |||
| private _cdr: ChangeDetectorRef, | |||
There was a problem hiding this comment.
nowhere, switched to adding class via renderer
src/lib/core/a11y/focus-trap.ts
Outdated
|
|
||
| /** Focuses the element that should be focused when the focus trap is initialized. */ | ||
| focusInitialElement() { | ||
| let redirectToElement = this._element.querySelector('[cdk-focus-init]') as HTMLElement; |
There was a problem hiding this comment.
Write out initial (to disambiguate from "initialize")?
src/lib/core/a11y/focus-trap.spec.ts
Outdated
| <button id="first" cdk-focus-start>SAVE</button> | ||
| <button id="last" cdk-focus-region-end></button> | ||
| <button id="middle" cdk-focus-init></button> | ||
| <button id="first" cdk-focus-region-start>SAVE</button> |
There was a problem hiding this comment.
Why does start come after end here?
There was a problem hiding this comment.
The test was already like this, I think the reasoning is to make sure that it's really grabbing the element with the attribute, not just the first or last one. I changed it to something less confusing.
src/lib/core/a11y/focus-trap.ts
Outdated
| private _getRegionMarker(marker: 'start' | 'end'): HTMLElement | null { | ||
| let markers = [ | ||
| ...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-region-${marker}]`)), | ||
| ...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-${marker}]`)), |
There was a problem hiding this comment.
Add comment explaining what's for backwards compatibility
|
comments addressed, PTAL |
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. |
also fix
md-nav-listfocused item style.fixes #2224