[PM-19923] Update autofocus directive to be more aggressive in focusing (#14368)

* Update autofocus directive to be more aggressive in focusing

* Handle checkVisibility not existing (safari < 17.4)

* Tweak phrasing

* Change approach

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;
   }
 }

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) {

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;
 }