diff --git a/libs/components/src/input/autofocus.directive.ts b/libs/components/src/input/autofocus.directive.ts index 27abccdd45f..46eb1b15b16 100644 --- a/libs/components/src/input/autofocus.directive.ts +++ b/libs/components/src/input/autofocus.directive.ts @@ -1,6 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { Directive, ElementRef, Input, NgZone, OnInit, Optional } from "@angular/core"; +import { AfterContentChecked, Directive, ElementRef, Input, NgZone, Optional } from "@angular/core"; import { take } from "rxjs/operators"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -12,40 +12,72 @@ import { FocusableElement } from "../shared/focusable-element"; * * @remarks * + * Will focus the element once, when it becomes visible. + * * If the component provides the `FocusableElement` interface, the `focus` * method will be called. Otherwise, the native element will be focused. */ @Directive({ selector: "[appAutofocus], [bitAutofocus]", }) -export class AutofocusDirective implements OnInit { +export class AutofocusDirective implements AfterContentChecked { @Input() set appAutofocus(condition: boolean | string) { this.autofocus = condition === "" || condition === true; } private autofocus: boolean; + // Track if we have already focused the element. + private focused = false; + constructor( private el: ElementRef, private ngZone: NgZone, @Optional() private focusableElement: FocusableElement, ) {} - ngOnInit() { - if (!Utils.isMobileBrowser && this.autofocus) { - if (this.ngZone.isStable) { - this.focus(); - } else { - this.ngZone.onStable.pipe(take(1)).subscribe(this.focus.bind(this)); - } + /** + * Using AfterContentChecked is a hack to ensure we only focus once. This is because + * the element may not be in the DOM, or not be focusable when the directive is + * created, and we want to wait until it is. + * + * Note: This might break in the future since it relies on Angular change detection + * to trigger after the element becomes visible. + */ + ngAfterContentChecked() { + // We only want to focus the element on initial render and it's not a mobile browser + if (this.focused || !this.autofocus || Utils.isMobileBrowser) { + return; + } + + const el = this.getElement(); + if (el == null) { + return; + } + + if (this.ngZone.isStable) { + this.focus(); + } else { + this.ngZone.onStable.pipe(take(1)).subscribe(this.focus.bind(this)); } } + /** + * Attempt to focus the element. If successful we set focused to true to prevent further focus + * attempts. + */ private focus() { + const el = this.getElement(); + + el.focus(); + this.focused = el === document.activeElement; + } + + private getElement() { if (this.focusableElement) { - this.focusableElement.getFocusTarget().focus(); - } else { - this.el.nativeElement.focus(); + return this.focusableElement.getFocusTarget(); } + + return this.el.nativeElement; } } diff --git a/libs/components/src/search/search.component.ts b/libs/components/src/search/search.component.ts index 7f1bd781e9d..7edf3b1d60a 100644 --- a/libs/components/src/search/search.component.ts +++ b/libs/components/src/search/search.component.ts @@ -49,7 +49,7 @@ export class SearchComponent implements ControlValueAccessor, FocusableElement { @Input() autocomplete: string; getFocusTarget() { - return this.input.nativeElement; + return this.input?.nativeElement; } onChange(searchText: string) { diff --git a/libs/components/src/shared/focusable-element.ts b/libs/components/src/shared/focusable-element.ts index 7b063f4ddc9..99340d5a7bf 100644 --- a/libs/components/src/shared/focusable-element.ts +++ b/libs/components/src/shared/focusable-element.ts @@ -6,5 +6,5 @@ * Used by the `AutofocusDirective` and `A11yGridDirective`. */ export abstract class FocusableElement { - getFocusTarget: () => HTMLElement; + getFocusTarget: () => HTMLElement | undefined; }