Fixed modal default focus issue

e2e/modal.e2e-spec.ts

@@ -13,41 +13,65 @@ describe('Modal', () => {
     SkyHostBrowser.get('visual/modal');
   });
 
-  afterEach((done) => {
-    expect('.sky-modal').toMatchBaselineScreenshot(done);
-  });
-
   describe('(lg screens)', () => {
     beforeEach(() => {
       SkyHostBrowser.setWindowBreakpoint('lg');
     });
 
-    it('should match previous modal screenshot', () => {
+    it('should match previous modal screenshot', (done) => {
       element(by.css('.sky-btn-primary')).click();
+      expect('.sky-modal').toMatchBaselineScreenshot(done, {
+        screenshotName: 'modal-lg-screenshot'
+      });
     });
 
-    it('should match previous modal screenshot with help button in header', () => {
+    it('should match previous modal screenshot with help button in header', (done) => {
       element(by.css('.sky-modal-with-help')).click();
+      expect('.sky-modal').toMatchBaselineScreenshot(done, {
+        screenshotName: 'modal-lg-with-help-screenshot'
+      });
     });
 
-    it('should match previous screenshot of modal without header or footer', () => {
+    it('should match previous screenshot of modal without header or footer', (done) => {
       element(by.css('.sky-test-content-only')).click();
+      expect('.sky-modal').toMatchBaselineScreenshot(done, {
+        screenshotName: 'modal-lg-content-screenshot'
+      });
     });
 
-    it('should match previous small size modal screenshot', () => {
+    it('should match previous small size modal screenshot', (done) => {
       element(by.css('.sky-test-small-size-modal')).click();
+      expect('.sky-modal').toMatchBaselineScreenshot(done, {
+        screenshotName: 'modal-lg-small-size-screenshot'
+      });
     });
 
-    it('should match previous medium size modal screenshot', () => {
+    it('should match previous medium size modal screenshot', (done) => {
       element(by.css('.sky-test-medium-size-modal')).click();
+      expect('.sky-modal').toMatchBaselineScreenshot(done, {
+        screenshotName: 'modal-lg-medium-size-screenshot'
+      });
     });
 
-    it('should match previous large modal screenshot', () => {
+    it('should match previous large modal screenshot', (done) => {
       element(by.css('.sky-test-large-modal')).click();
+      expect('.sky-modal').toMatchBaselineScreenshot(done, {
+        screenshotName: 'modal-lg-large-screenshot'
+      });
     });
 
-    it('should match previous large size modal screenshot', () => {
+    it('should match previous large size modal screenshot', (done) => {
       element(by.css('.sky-test-large-size-modal')).click();
+      expect('.sky-modal').toMatchBaselineScreenshot(done, {
+        screenshotName: 'modal-lg-large-size-screenshot'
+      });
+    });
+
+    it('should match previous autofocus screenshot', (done) => {
+      element(by.css('.sky-test-large-modal-autofocus')).click();
+      expect('.sky-modal').toMatchBaselineScreenshot(done, {
+        screenshotName: 'modal-lg-autofocus-screenshot'
+      });
     });
   });
 
@@ -56,12 +80,18 @@ describe('Modal', () => {
       SkyHostBrowser.setWindowBreakpoint('md');
     });
 
-    it('should match previous large size modal screenshot on intermediate screens', () => {
+    it('should match previous large size modal screenshot on intermediate screens', (done) => {
       element(by.css('.sky-test-large-size-modal')).click();
+      expect('.sky-modal').toMatchBaselineScreenshot(done, {
+        screenshotName: 'modal-md-large-size-screenshot'
+      });
     });
 
-    it('should match previous tiled modal screenshot', () => {
+    it('should match previous tiled modal screenshot', (done) => {
       element(by.css('.sky-test-tiled-modal')).click();
+      expect('.sky-modal').toMatchBaselineScreenshot(done, {
+        screenshotName: 'modal-md-tiled-screenshot'
+      });
     });
   });
 

src/app/app-extras.module.ts

@@ -12,11 +12,24 @@ import {
   SkyTilesModule
 } from '@skyux/tiles';
 
-import { ModalDemoComponent } from './visual/modal/modal-demo.component';
-import { ModalContentDemoComponent } from './visual/modal/modal-content-demo.component';
-import { ModalFullPageDemoComponent } from './visual/modal/modal-fullpage-demo.component';
-import { ModalLargeDemoComponent } from './visual/modal/modal-large-demo.component';
-import { ModalTiledDemoComponent } from './visual/modal/modal-tiled-demo.component';
+import {
+  ModalDemoComponent
+} from './visual/modal/modal-demo.component';
+import {
+  ModalContentDemoComponent
+} from './visual/modal/modal-content-demo.component';
+import {
+  ModalFullPageDemoComponent
+} from './visual/modal/modal-fullpage-demo.component';
+import {
+  ModalLargeDemoComponent
+} from './visual/modal/modal-large-demo.component';
+import {
+  ModalTiledDemoComponent
+} from './visual/modal/modal-tiled-demo.component';
+import {
+  ModalContentAutofocusComponent
+} from './visual/modal/modal-content-autofocus.component';
 
 @NgModule({
   imports: [
@@ -37,7 +50,8 @@ import { ModalTiledDemoComponent } from './visual/modal/modal-tiled-demo.compone
     ModalContentDemoComponent,
     ModalFullPageDemoComponent,
     ModalLargeDemoComponent,
-    ModalTiledDemoComponent
+    ModalTiledDemoComponent,
+    ModalContentAutofocusComponent
   ]
 })
 export class AppExtrasModule { }

src/app/public/modules/modal/fixtures/modal-fixtures.module.ts

@@ -1,19 +1,41 @@
-import { NgModule } from '@angular/core';
-import { CommonModule } from '@angular/common';
-
+import {
+  NgModule
+} from '@angular/core';
+import {
+  CommonModule
+} from '@angular/common';
 import {
   RouterTestingModule
 } from '@angular/router/testing';
 
-import { SkyWindowRefService } from '@skyux/core';
+import {
+  SkyWindowRefService
+} from '@skyux/core';
 
-import { SkyModalModule } from '../modal.module';
-import { ModalTestComponent } from './modal.component.fixture';
-import { ModalWithValuesTestComponent } from './modal-with-values.component.fixture';
-import { ModalAutofocusTestComponent } from './modal-autofocus.component.fixture';
-import { ModalFooterTestComponent } from './modal-footer.component.fixture';
-import { ModalNoHeaderTestComponent } from './modal-no-header.component.fixture';
-import { ModalTiledBodyTestComponent } from './modal-tiled-body.component.fixture';
+import {
+  SkyModalModule
+} from '../modal.module';
+import {
+  ModalTestComponent
+} from './modal.component.fixture';
+import {
+  ModalWithValuesTestComponent
+} from './modal-with-values.component.fixture';
+import {
+  ModalAutofocusTestComponent
+} from './modal-autofocus.component.fixture';
+import {
+  ModalFooterTestComponent
+} from './modal-footer.component.fixture';
+import {
+  ModalNoHeaderTestComponent
+} from './modal-no-header.component.fixture';
+import {
+  ModalTiledBodyTestComponent
+} from './modal-tiled-body.component.fixture';
+import {
+  ModalWithFocusContentTestComponent
+} from './modal-with-focus-content.fixture';
 
 @NgModule({
   declarations: [
@@ -22,7 +44,8 @@ import { ModalTiledBodyTestComponent } from './modal-tiled-body.component.fixtur
     ModalAutofocusTestComponent,
     ModalFooterTestComponent,
     ModalNoHeaderTestComponent,
-    ModalTiledBodyTestComponent
+    ModalTiledBodyTestComponent,
+    ModalWithFocusContentTestComponent
   ],
   imports: [
     CommonModule,
@@ -38,7 +61,8 @@ import { ModalTiledBodyTestComponent } from './modal-tiled-body.component.fixtur
     ModalAutofocusTestComponent,
     ModalFooterTestComponent,
     ModalNoHeaderTestComponent,
-    ModalTiledBodyTestComponent
+    ModalTiledBodyTestComponent,
+    ModalWithFocusContentTestComponent
   ]
 })
 export class SkyModalFixturesModule { }

src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html

@@ -0,0 +1,6 @@
+<sky-modal>
+  <sky-modal-header>Test</sky-modal-header>
+  <sky-modal-content>
+    <button>Content</button>
+  </sky-modal-content>
+</sky-modal>

src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.ts

@@ -0,0 +1,9 @@
+import {
+  Component
+} from '@angular/core';
+
+@Component({
+  selector: 'sky-test-cmp-with-focus',
+  templateUrl: './modal-with-focus-content.fixture.html'
+})
+export class ModalWithFocusContentTestComponent { }

src/app/public/modules/modal/modal-component-adapter.service.ts

@@ -38,12 +38,7 @@ export class SkyModalComponentAdapterService {
   }
 
   public loadFocusElementList(modalEl: ElementRef): Array<HTMLElement> {
-    let elements: Array<HTMLElement>
-      = Array.prototype.slice.call(modalEl.nativeElement.querySelectorAll(tabbableSelector));
-
-    return elements.filter((element) => {
-      return this.isVisible(element);
-    });
+    return this.loadFocussableChildren(modalEl.nativeElement);
   }
 
   public isFocusInFirstItem(event: KeyboardEvent, list: Array<HTMLElement>): boolean {
@@ -97,13 +92,26 @@ export class SkyModalComponentAdapterService {
         inputWithAutofocus.focus();
       } else {
         let focusEl: HTMLElement = modalEl.nativeElement.querySelector('.sky-modal-content');
-        focusEl.focus();
+        let focussableChildren = this.loadFocussableChildren(focusEl);
 
+        // Focus first focussable child if available. Otherwise focus content pane.
+        if (!this.focusFirstElement(focussableChildren)) {
+          focusEl.focus();
+        }
       }
       window.scrollTo(currentScrollX, currentScrollY);
     }
   }
 
+  private loadFocussableChildren(elem: HTMLElement) {
+    let elements: Array<HTMLElement>
+      = Array.prototype.slice.call(elem.querySelectorAll(tabbableSelector));
+
+    return elements.filter((element) => {
+      return this.isVisible(element);
+    });
+  }
+
   private isVisible(element: HTMLElement) {
     return !!(element.offsetWidth ||
       element.offsetHeight ||

src/app/public/modules/modal/modal.component.spec.ts

@@ -1,11 +1,12 @@
-import { ApplicationRef } from '@angular/core';
+import {
+  ApplicationRef
+} from '@angular/core';
 import {
   fakeAsync,
   inject,
   tick,
   TestBed
 } from '@angular/core/testing';
-
 import {
   Router
 } from '@angular/router';
@@ -15,18 +16,37 @@ import {
   SkyAppTestUtility
 } from '@skyux-sdk/testing';
 
-import { SkyModalInstance } from './modal-instance';
-import { SkyModalService } from './modal.service';
-
-import { SkyModalFixturesModule } from './fixtures/modal-fixtures.module';
-import { ModalTestComponent } from './fixtures/modal.component.fixture';
-import { ModalAutofocusTestComponent } from './fixtures/modal-autofocus.component.fixture';
-import { ModalFooterTestComponent } from './fixtures/modal-footer.component.fixture';
-
-import { ModalNoHeaderTestComponent } from './fixtures/modal-no-header.component.fixture';
-import { ModalTiledBodyTestComponent  } from './fixtures/modal-tiled-body.component.fixture';
+import {
+  SkyModalInstance
+} from './modal-instance';
+import {
+  SkyModalService
+} from './modal.service';
+import {
+  SkyModalComponentAdapterService
+} from './modal-component-adapter.service';
 
-import { SkyModalComponentAdapterService } from './modal-component-adapter.service';
+import {
+  SkyModalFixturesModule
+} from './fixtures/modal-fixtures.module';
+import {
+  ModalTestComponent
+} from './fixtures/modal.component.fixture';
+import {
+  ModalAutofocusTestComponent
+} from './fixtures/modal-autofocus.component.fixture';
+import {
+  ModalFooterTestComponent
+} from './fixtures/modal-footer.component.fixture';
+import {
+  ModalNoHeaderTestComponent
+} from './fixtures/modal-no-header.component.fixture';
+import {
+  ModalTiledBodyTestComponent
+} from './fixtures/modal-tiled-body.component.fixture';
+import {
+  ModalWithFocusContentTestComponent
+} from './fixtures/modal-with-focus-content.fixture';
 
 describe('Modal component', () => {
   let applicationRef: ApplicationRef;
@@ -91,7 +111,13 @@ describe('Modal component', () => {
     closeModal(modalInstance1);
   }));
 
-  it('should focus the dialog when no autofocus is inside of content', fakeAsync(() => {
+  it('should focus the first focussable element when no autofocus is inside of content', fakeAsync(() => {
+    let modalInstance1 = openModal(ModalWithFocusContentTestComponent);
+    expect(document.activeElement).toEqual(document.querySelector('.sky-modal-content button'));
+    closeModal(modalInstance1);
+  }));
+
+  it('should focus the dialog when no autofocus or focus element is inside of content', fakeAsync(() => {
     let modalInstance1 = openModal(ModalTestComponent);
     expect(document.activeElement).toEqual(document.querySelector('.sky-modal-content'));
     closeModal(modalInstance1);

src/app/visual/modal/modal-content-autofocus.component.html

@@ -0,0 +1,7 @@
+<sky-modal>
+  <sky-modal-content>
+    Content
+    <button type="button" class="sky-btn sky-btn-primary sky-test-close" (click)="instance.cancel()">Close</button>
+    <button type="button" class="sky-btn sky-btn-link" autofocus="autofocus" (click)="instance.cancel()">Focused button</button>
+  </sky-modal-content>
+</sky-modal>

src/app/visual/modal/modal-content-autofocus.component.ts

@@ -0,0 +1,12 @@
+import { Component } from '@angular/core';
+
+import { SkyModalInstance } from '../../public';
+
+@Component({
+  selector: 'sky-test-cmp-modal-autofocus',
+  templateUrl: './modal-content-autofocus.component.html'
+})
+export class ModalContentAutofocusComponent {
+
+  constructor(public instance: SkyModalInstance) {}
+}

src/app/visual/modal/modal-visual.component.html

@@ -58,4 +58,11 @@
   >
   Open tiled modal
   </button>
+  <button
+    type="button"
+    class="sky-btn sky-btn-primary sky-test-large-modal-autofocus"
+    (click)="openAutofocusModal()"
+  >
+  Open autofocus modal
+  </button>
 </div>