fix(focustrap): discarding tabindex='-1' when finding element (#2888)

fix #2884
ng-bootstrap · Nov 28, 2018 · db7347b · db7347b
1 parent c021f54
commit db7347b
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 4 deletions.
diff --git a/src/modal/modal.spec.ts b/src/modal/modal.spec.ts
@@ -14,6 +14,7 @@ import {async, ComponentFixture, TestBed} from '@angular/core/testing';
 import {NgbModalConfig} from './modal-config';
 import {NgbActiveModal, NgbModal, NgbModalModule, NgbModalRef} from './modal.module';
 
+
 const NOOP = () => {};
 
 @Injectable()
@@ -747,6 +748,16 @@ describe('ngb-modal', () => {
           fixture.detectChanges();
         });
 
+        it('should skip element with tabindex=-1 when finding the first focusable element', () => {
+          fixture.detectChanges();
+          const modal = fixture.componentInstance.openCmpt(WithSkipTabindexFirstFocusableModalCmpt);
+          fixture.detectChanges();
+
+          expect(document.activeElement).toBe(document.querySelector('button.other'));
+          modal.close();
+          fixture.detectChanges();
+        });
+
         it('should focus modal window as a default fallback option', () => {
           fixture.detectChanges();
           const modal = fixture.componentInstance.open('content');
@@ -882,6 +893,16 @@ export class WithAutofocusModalCmpt {
 export class WithFirstFocusableModalCmpt {
 }
 
+@Component({
+  selector: 'modal-skip-tabindex-firstfocusable-cmpt',
+  template: `
+  <button tabindex="-1" class="firstFocusable close">Close</button>
+  <button class="other">Other button</button>
+`
+})
+export class WithSkipTabindexFirstFocusableModalCmpt {
+}
+
 @Component({
   selector: 'test-cmpt',
   template: `
@@ -949,12 +970,14 @@ class TestComponent {
 @NgModule({
   declarations: [
     TestComponent, CustomInjectorCmpt, DestroyableCmpt, WithActiveModalCmpt, WithAutofocusModalCmpt,
-    WithFirstFocusableModalCmpt
+    WithFirstFocusableModalCmpt, WithSkipTabindexFirstFocusableModalCmpt
   ],
   exports: [TestComponent, DestroyableCmpt],
   imports: [CommonModule, NgbModalModule],
-  entryComponents:
-      [CustomInjectorCmpt, DestroyableCmpt, WithActiveModalCmpt, WithAutofocusModalCmpt, WithFirstFocusableModalCmpt],
+  entryComponents: [
+    CustomInjectorCmpt, DestroyableCmpt, WithActiveModalCmpt, WithAutofocusModalCmpt, WithFirstFocusableModalCmpt,
+    WithSkipTabindexFirstFocusableModalCmpt
+  ],
   providers: [SpyService]
 })
 class NgbModalTestModule {

diff --git a/src/util/focus-trap.ts b/src/util/focus-trap.ts
@@ -3,6 +3,7 @@ import {filter, map, takeUntil, withLatestFrom} from 'rxjs/operators';
 
 import {Key} from '../util/key';
 
+
 const FOCUSABLE_ELEMENTS_SELECTOR = [
   'a[href]', 'button:not([disabled])', 'input:not([disabled]):not([type="hidden"])', 'select:not([disabled])',
   'textarea:not([disabled])', '[contenteditable]', '[tabindex]:not([tabindex="-1"])'
@@ -12,7 +13,9 @@ const FOCUSABLE_ELEMENTS_SELECTOR = [
  * Returns first and last focusable elements inside of a given element based on specific CSS selector
  */
 export function getFocusableBoundaryElements(element: HTMLElement): HTMLElement[] {
-  const list: NodeListOf<HTMLElement> = element.querySelectorAll(FOCUSABLE_ELEMENTS_SELECTOR);
+  const list: HTMLElement[] =
+      Array.from(element.querySelectorAll(FOCUSABLE_ELEMENTS_SELECTOR) as NodeListOf<HTMLElement>)
+          .filter(el => el.tabIndex !== -1);
   return [list[0], list[list.length - 1]];
 }