fix(components/modals): modal host sets initial modal focus to avoid …

…race condition (#2944)
blackbaud · Dec 13, 2024 · dfdd548 · dfdd548
1 parent 665f518
commit dfdd548
Show file tree

Hide file tree

Showing 8 changed files with 115 additions and 95 deletions.
diff --git a/apps/integration/src/app/integrations/lookup-in-modal/lookup-in-modal.component.spec.ts b/apps/integration/src/app/integrations/lookup-in-modal/lookup-in-modal.component.spec.ts
@@ -1,7 +1,11 @@
+import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
-import { RouterTestingModule } from '@angular/router/testing';
+import { provideRouter } from '@angular/router';
 import { expectAsync } from '@skyux-sdk/testing';
+import { SkyInputBoxHarness } from '@skyux/forms/testing';
+import { SkyLookupHarness } from '@skyux/lookup/testing';
+import { SkyModalHarness } from '@skyux/modals/testing';
 import { SkyThemeService } from '@skyux/theme';
 
 import { LookupInModalComponent } from './lookup-in-modal.component';
@@ -13,12 +17,8 @@ describe('LookupInModalComponent', () => {
 
   beforeEach(() => {
     TestBed.configureTestingModule({
-      imports: [
-        LookupInModalModule,
-        NoopAnimationsModule,
-        RouterTestingModule.withRoutes([]),
-      ],
-      providers: [SkyThemeService],
+      imports: [LookupInModalModule, NoopAnimationsModule],
+      providers: [SkyThemeService, provideRouter([])],
     });
 
     fixture = TestBed.createComponent(LookupInModalComponent);
@@ -36,6 +36,25 @@ describe('LookupInModalComponent', () => {
     ).toContain('margin-top: 50px');
   });
 
+  it('should show lookup in modal', async () => {
+    await fixture.whenStable();
+    fixture.detectChanges();
+    await fixture.whenStable();
+    const loader = TestbedHarnessEnvironment.documentRootLoader(fixture);
+    const modalHarness = await loader.getHarness(
+      SkyModalHarness.with({
+        dataSkyId: 'modal-lookup',
+      }),
+    );
+    expect(await modalHarness.getSize()).toBe('small');
+    const lookupHarness = await (
+      await loader.getHarness(
+        SkyInputBoxHarness.with({ dataSkyId: 'favorite-names-field' }),
+      )
+    ).queryHarness(SkyLookupHarness);
+    expect(await (await lookupHarness.getControl()).isFocused()).toBeTrue();
+  });
+
   it('should be accessible', async () => {
     await fixture.whenStable();
     await expectAsync(fixture.nativeElement).toBeAccessible();

diff --git a/apps/integration/src/app/integrations/lookup-in-modal/modal-lookup.component.spec.ts b/apps/integration/src/app/integrations/lookup-in-modal/modal-lookup.component.spec.ts
@@ -1,7 +1,7 @@
 import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
-import { RouterTestingModule } from '@angular/router/testing';
+import { provideRouter } from '@angular/router';
 import { expectAsync } from '@skyux-sdk/testing';
 import { SkyInputBoxHarness } from '@skyux/forms/testing';
 import { SkyLookupHarness } from '@skyux/lookup/testing';
@@ -16,12 +16,8 @@ describe('ModalLookupComponent', () => {
 
   beforeEach(() => {
     TestBed.configureTestingModule({
-      imports: [
-        LookupInModalModule,
-        NoopAnimationsModule,
-        RouterTestingModule.withRoutes([]),
-      ],
-      providers: [SkyThemeService],
+      imports: [LookupInModalModule, NoopAnimationsModule],
+      providers: [SkyThemeService, provideRouter([])],
     });
 
     fixture = TestBed.createComponent(ModalLookupComponent);
@@ -43,7 +39,7 @@ describe('ModalLookupComponent', () => {
         SkyInputBoxHarness.with({ dataSkyId: 'favorite-names-field' }),
       )
     ).queryHarness(SkyLookupHarness);
-    expect(await lookupHarness.isFocused()).toBeTruthy();
+    expect(await (await lookupHarness.getControl()).isDisabled()).toBeFalse();
     fixture.componentInstance.onSubmit();
     expect(consoleLog).toHaveBeenCalledWith({
       submitted: { favoriteNames: [] },

diff --git a/libs/components/modals/src/lib/modules/modal/modal-adapter.service.ts b/libs/components/modals/src/lib/modules/modal/modal-adapter.service.ts
@@ -1,5 +1,5 @@
-import { ElementRef, Injectable } from '@angular/core';
-import { SkyAppWindowRef } from '@skyux/core';
+import { ElementRef, Injectable, inject } from '@angular/core';
+import { SkyAppWindowRef, SkyCoreAdapterService } from '@skyux/core';
 
 /**
  * @internal
@@ -12,6 +12,7 @@ export class SkyModalAdapterService {
   #docRef: any;
   #bodyEl: HTMLElement;
 
+  #coreAdapter = inject(SkyCoreAdapterService);
   #windowRef: SkyAppWindowRef;
   #hostSiblingAriaHiddenCache = new Map<Element, string | null>();
 
@@ -59,22 +60,55 @@ export class SkyModalAdapterService {
    */
   public hideHostSiblingsFromScreenReaders(hostElRef: ElementRef): void {
     const hostElement = hostElRef.nativeElement;
-    const hostSiblings = hostElement.parentElement.children;
-
-    for (const element of hostSiblings) {
-      if (
-        element !== hostElement &&
-        !element.hasAttribute('aria-live') &&
-        element.nodeName.toLowerCase() !== 'script' &&
-        element.nodeName.toLowerCase() !== 'style'
-      ) {
-        // preserve previous aria-hidden status of elements outside of modal host
-        this.#hostSiblingAriaHiddenCache.set(
-          element,
-          element.getAttribute('aria-hidden'),
+    const hostSiblings = hostElement.parentElement?.children;
+
+    if (hostSiblings) {
+      for (const element of hostSiblings) {
+        if (element.contains(document.activeElement)) {
+          document.body.focus();
+        }
+        if (
+          element !== hostElement &&
+          !element.hasAttribute('aria-live') &&
+          element.nodeName.toLowerCase() !== 'script' &&
+          element.nodeName.toLowerCase() !== 'style'
+        ) {
+          // preserve previous aria-hidden status of elements outside of modal host
+          this.#hostSiblingAriaHiddenCache.set(
+            element,
+            element.getAttribute('aria-hidden'),
+          );
+          element.setAttribute('aria-hidden', 'true');
+        }
+      }
+    }
+  }
+
+  public focusFirstElement(modalEl: ElementRef): void {
+    /* istanbul ignore else */
+    /* handle the case where somehow there is a focused element already in the modal */
+    if (
+      !(
+        document.activeElement &&
+        modalEl.nativeElement.contains(document.activeElement)
+      )
+    ) {
+      const currentScrollX = window.pageXOffset;
+      const currentScrollY = window.pageYOffset;
+
+      const inputWithAutofocus =
+        modalEl.nativeElement.querySelector('[autofocus]');
+
+      if (inputWithAutofocus) {
+        inputWithAutofocus.focus();
+      } else {
+        this.#coreAdapter.getFocusableChildrenAndApplyFocus(
+          modalEl,
+          '.sky-modal-content',
+          true,
         );
-        element.setAttribute('aria-hidden', 'true');
       }
+      window.scrollTo(currentScrollX, currentScrollY);
     }
   }
 

diff --git a/libs/components/modals/src/lib/modules/modal/modal-component-adapter.service.ts b/libs/components/modals/src/lib/modules/modal/modal-component-adapter.service.ts
@@ -1,17 +1,10 @@
 import { ElementRef, Injectable } from '@angular/core';
-import { SkyCoreAdapterService } from '@skyux/core';
 
 /**
  * @internal
  */
 @Injectable()
 export class SkyModalComponentAdapterService {
-  #coreAdapter: SkyCoreAdapterService;
-
-  constructor(coreAdapter: SkyCoreAdapterService) {
-    this.#coreAdapter = coreAdapter;
-  }
-
   public handleWindowChange(modalEl: ElementRef): void {
     const boundedHeightEl = modalEl.nativeElement.querySelector('.sky-modal');
     const fullPageModalEl = modalEl.nativeElement.querySelector(
@@ -88,34 +81,6 @@ export class SkyModalComponentAdapterService {
     );
   }
 
-  public modalOpened(modalEl: ElementRef): void {
-    /* istanbul ignore else */
-    /* handle the case where somehow there is a focused element already in the modal */
-    if (
-      !(
-        document.activeElement &&
-        modalEl.nativeElement.contains(document.activeElement)
-      )
-    ) {
-      const currentScrollX = window.pageXOffset;
-      const currentScrollY = window.pageYOffset;
-
-      const inputWithAutofocus =
-        modalEl.nativeElement.querySelector('[autofocus]');
-
-      if (inputWithAutofocus) {
-        inputWithAutofocus.focus();
-      } else {
-        this.#coreAdapter.getFocusableChildrenAndApplyFocus(
-          modalEl,
-          '.sky-modal-content',
-          true,
-        );
-      }
-      window.scrollTo(currentScrollX, currentScrollY);
-    }
-  }
-
   #setFullPageHeight(fullPageModalEl: HTMLElement): void {
     const windowHeight = window.innerHeight;
     const fullPageModalStyle = getComputedStyle(fullPageModalEl);

diff --git a/libs/components/modals/src/lib/modules/modal/modal-host.component.ts b/libs/components/modals/src/lib/modules/modal/modal-host.component.ts
@@ -11,7 +11,11 @@ import {
   inject,
 } from '@angular/core';
 import { NavigationStart, Router, RouterModule } from '@angular/router';
-import { SKY_STACKING_CONTEXT, SkyDynamicComponentService } from '@skyux/core';
+import {
+  SKY_STACKING_CONTEXT,
+  SkyAppWindowRef,
+  SkyDynamicComponentService,
+} from '@skyux/core';
 
 import { BehaviorSubject } from 'rxjs';
 import { takeUntil, takeWhile } from 'rxjs/operators';
@@ -60,6 +64,7 @@ export class SkyModalHostComponent implements OnDestroy {
   readonly #environmentInjector = inject(EnvironmentInjector);
   readonly #modalHostContext = inject(SkyModalHostContext);
   readonly #router = inject(Router, { optional: true });
+  readonly #windowRef = inject(SkyAppWindowRef);
 
   public ngOnDestroy(): void {
     // Close all modal instances before disposing of the host container.
@@ -133,15 +138,21 @@ export class SkyModalHostComponent implements OnDestroy {
 
     this.#registerModalInstance(modalInstance);
 
-    // hiding all elements at the modal-host level from screen readers when the a modal is opened
-    this.#adapter.hideHostSiblingsFromScreenReaders(this.#elRef);
-    if (
-      SkyModalHostService.openModalCount > 1 &&
-      SkyModalHostService.topModal === hostService
-    ) {
-      // hiding the lower modals when more than one modal is opened
-      this.#adapter.hidePreviousModalFromScreenReaders(modalElement);
-    }
+    // Adding a timeout to avoid ExpressionChangedAfterItHasBeenCheckedError.
+    // https://stackoverflow.com/questions/40562845
+    this.#windowRef.nativeWindow.setTimeout(() => {
+      this.#adapter.focusFirstElement(modalElement);
+
+      // hiding all elements at the modal-host level from screen readers when the a modal is opened
+      this.#adapter.hideHostSiblingsFromScreenReaders(this.#elRef);
+      if (
+        SkyModalHostService.openModalCount > 1 &&
+        SkyModalHostService.topModal === hostService
+      ) {
+        // hiding the lower modals when more than one modal is opened
+        this.#adapter.hidePreviousModalFromScreenReaders(modalElement);
+      }
+    });
 
     const closeModal = (): void => {
       // unhide siblings if last modal is closing

diff --git a/libs/components/modals/src/lib/modules/modal/modal.component.spec.ts b/libs/components/modals/src/lib/modules/modal/modal.component.spec.ts
@@ -3,7 +3,6 @@ import { TestBed, fakeAsync, tick } from '@angular/core/testing';
 import { Router } from '@angular/router';
 import { SkyAppTestUtility, expect, expectAsync } from '@skyux-sdk/testing';
 import {
-  SkyCoreAdapterService,
   SkyDockLocation,
   SkyDockService,
   SkyLiveAnnouncerService,
@@ -561,9 +560,7 @@ describe('Modal component', () => {
   }));
 
   it('should handle empty list for focus first and last element functions', fakeAsync(() => {
-    const adapterService = new SkyModalComponentAdapterService(
-      TestBed.inject(SkyCoreAdapterService),
-    );
+    const adapterService = new SkyModalComponentAdapterService();
     const firstResult = adapterService.focusFirstElement([]);
     expect(firstResult).toBe(false);
 

diff --git a/libs/components/modals/src/lib/modules/modal/modal.component.ts b/libs/components/modals/src/lib/modules/modal/modal.component.ts
@@ -14,7 +14,6 @@ import {
   inject,
 } from '@angular/core';
 import {
-  SkyAppWindowRef,
   SkyCoreAdapterService,
   SkyDockLocation,
   SkyDockService,
@@ -188,7 +187,6 @@ export class SkyModalComponent implements AfterViewInit, OnDestroy, OnInit {
   readonly #errorsSvc = inject(SkyModalErrorsService);
   readonly #hostService = inject(SkyModalHostService);
   readonly #liveAnnouncerSvc = inject(SkyLiveAnnouncerService);
-  readonly #windowRef = inject(SkyAppWindowRef);
 
   /**
    * This provider is optional to account for situations where a modal component
@@ -297,12 +295,6 @@ export class SkyModalComponent implements AfterViewInit, OnDestroy, OnInit {
   public ngAfterViewInit(): void {
     this.#componentAdapter.handleWindowChange(this.#elRef);
 
-    // Adding a timeout to avoid ExpressionChangedAfterItHasBeenCheckedError.
-    // https://stackoverflow.com/questions/40562845
-    this.#windowRef.nativeWindow.setTimeout(() => {
-      this.#componentAdapter.modalOpened(this.#elRef);
-    });
-
     this.#dockService.setDockOptions({
       location: SkyDockLocation.ElementBottom,
       referenceEl: this.modalContentWrapperElement!.nativeElement,

diff --git a/package-lock.json b/package-lock.json