fix(dialog): capture previously focused element immediately #3875
Conversation
googlebot
added
the
cla: yes
| if (this._document) { | ||
| this._elementFocusedBeforeDialogWasOpened = this._document.activeElement as HTMLElement; | ||
| } | ||
| } |
There was a problem hiding this comment.
Any way to capture this in a unit test?
There was a problem hiding this comment.
It was tricky to write a test that fails when we expect it to since we trigger the change detection and animations manually.
src/lib/dialog/dialog-container.ts
Outdated
| this._focusTrap.focusFirstTabbableElementWhenReady(); | ||
| } | ||
|
|
||
| /** | ||
| * Saves a reference to the element that was focused before the dialog was opened. | ||
| * @private |
src/lib/dialog/dialog-container.ts
Outdated
| @@ -77,7 +80,8 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy { | |||
| private _ngZone: NgZone, | |||
| private _renderer: Renderer, | |||
| private _elementRef: ElementRef, | |||
| private _focusTrapFactory: FocusTrapFactory) { | |||
| private _focusTrapFactory: FocusTrapFactory, | |||
| @Optional() @Inject(DOCUMENT) private _document: any) { | |||
There was a problem hiding this comment.
We can add a type for this while working around Angular Universal's limitations by doing Document|Document
There was a problem hiding this comment.
Doing Document|Document fails in AoT.
Error: Error encountered resolving symbol values statically. Expression form not supported (position 84:54 in the original .ts file), resolving symbol MdDialogContainer
There was a problem hiding this comment.
Chatted with Chuck and this is a known issue with the metadata extractor (angular/angular#15424). Until that's fixed, we can inject it as any without the private sugar and then manually assign it to the field with type Document.
crisbeto
force-pushed
the
dialog-capture-focus-early
branch
from
0098279 to
0470d35
Compare
|
Addressed the feedback @jelbourn. |
crisbeto
force-pushed
the
dialog-capture-focus-early
branch
2 times, most recently
from
3690447 to
ebf8407
Compare
jelbourn
added
pr: lgtm
action: merge
kara
added
pr: needs rebase
and removed
action: merge
|
@crisbeto Can you rebase? |
Captures the previously-focused element immediately, instead of waiting until the animation is done. This fixes a regression from angular#3774, because if we wait for the animation to finish, the focus might have shifted, or the element could have been disabled.
crisbeto
force-pushed
the
dialog-capture-focus-early
branch
from
231f23a to
43a16fa
Compare
crisbeto
added
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. |
Captures the previously-focused element immediately, instead of waiting until the animation is done. This fixes a regression from #3774, because if we wait for the animation to finish, the focus might have shifted, or the element could have been disabled.