From efca784c016f30b8d6c6972fe33ea49ccf45390a Mon Sep 17 00:00:00 2001
From: Conor Wright <Conor.Wright@blackbaud.com>
Date: Wed, 14 Nov 2018 11:39:43 -0500
Subject: [PATCH 1/4] fixed initial focus to ignore hidden elements

---
 .../modal/fixtures/modal-with-focus-content.fixture.html | 4 +++-
 .../modules/modal/modal-component-adapter.service.ts     | 9 +++++++--
 src/app/public/modules/modal/modal.component.spec.ts     | 2 +-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html b/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html
index 3a198c9..4f40a6d 100644
--- a/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html
+++ b/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html
@@ -1,6 +1,8 @@
 <sky-modal>
   <sky-modal-header>Test</sky-modal-header>
   <sky-modal-content>
-    <button>Content</button>
+    <button style="visibility: hidden">Hidden</button>
+    <button style="display: none">No display</button>
+    <button id="visible-btn">Content</button>
   </sky-modal-content>
 </sky-modal>
diff --git a/src/app/public/modules/modal/modal-component-adapter.service.ts b/src/app/public/modules/modal/modal-component-adapter.service.ts
index 28afadd..a597590 100644
--- a/src/app/public/modules/modal/modal-component-adapter.service.ts
+++ b/src/app/public/modules/modal/modal-component-adapter.service.ts
@@ -113,8 +113,13 @@ export class SkyModalComponentAdapterService {
   }
 
   private isVisible(element: HTMLElement) {
-    return !!(element.offsetWidth ||
+    const style = window.getComputedStyle(element);
+    const isHidden = style.display === 'none' || style.visibility === 'hidden';
+    const hasBounds = !!(
+      element.offsetWidth ||
       element.offsetHeight ||
-      element.getClientRects().length);
+      element.getClientRects().length
+    );
+    return !isHidden && hasBounds;
   }
 }
diff --git a/src/app/public/modules/modal/modal.component.spec.ts b/src/app/public/modules/modal/modal.component.spec.ts
index 0456114..f350d59 100644
--- a/src/app/public/modules/modal/modal.component.spec.ts
+++ b/src/app/public/modules/modal/modal.component.spec.ts
@@ -113,7 +113,7 @@ describe('Modal component', () => {
 
   it('should focus the first focusable element when no autofocus is inside of content', fakeAsync(() => {
     let modalInstance1 = openModal(ModalWithFocusContentTestComponent);
-    expect(document.activeElement).toEqual(document.querySelector('.sky-modal-content button'));
+    expect(document.activeElement).toEqual(document.querySelector('#visible-btn'));
     closeModal(modalInstance1);
   }));
 

From 55de8884201bb11269fe1091e624ca9ae23812a3 Mon Sep 17 00:00:00 2001
From: Conor Wright <Conor.Wright@blackbaud.com>
Date: Fri, 16 Nov 2018 10:57:50 -0500
Subject: [PATCH 2/4] added bound-less button to nav tests

---
 .../modal/fixtures/modal-with-focus-content.fixture.html     | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html b/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html
index 4f40a6d..d7bbf52 100644
--- a/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html
+++ b/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html
@@ -1,8 +1,9 @@
 <sky-modal>
   <sky-modal-header>Test</sky-modal-header>
   <sky-modal-content>
-    <button style="visibility: hidden">Hidden</button>
-    <button style="display: none">No display</button>
+    <button style="visibility: hidden;">Hidden</button>
+    <button style="display: none;">No display</button>
+    <button style="display: contents;"></button>
     <button id="visible-btn">Content</button>
   </sky-modal-content>
 </sky-modal>

From 11706d6967b4c30078eab26d222c940cbcf89c46 Mon Sep 17 00:00:00 2001
From: Conor Wright <Conor.Wright@blackbaud.com>
Date: Fri, 16 Nov 2018 15:42:54 -0500
Subject: [PATCH 3/4] removed contents display check

---
 .../modal-with-focus-content.fixture.html     | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html b/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html
index d7bbf52..2cb701a 100644
--- a/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html
+++ b/src/app/public/modules/modal/fixtures/modal-with-focus-content.fixture.html
@@ -1,9 +1,22 @@
 <sky-modal>
-  <sky-modal-header>Test</sky-modal-header>
+  <sky-modal-header>
+    Test
+  </sky-modal-header>
   <sky-modal-content>
-    <button style="visibility: hidden;">Hidden</button>
-    <button style="display: none;">No display</button>
-    <button style="display: contents;"></button>
-    <button id="visible-btn">Content</button>
+    <button
+      style="visibility: hidden;"
+    >
+      Hidden
+    </button>
+    <button
+      style="display: none;"
+    >
+      No display
+    </button>
+    <button
+      id="visible-btn"
+    >
+      Content
+    </button>
   </sky-modal-content>
 </sky-modal>

From a29db0414e0230f9fd7b563547c17c6ae24c1f65 Mon Sep 17 00:00:00 2001
From: Conor Wright <conor.wright@blackbaud.com>
Date: Tue, 27 Nov 2018 14:34:11 -0500
Subject: [PATCH 4/4] Added early return to avoid unnecessary calls

---
 .../public/modules/modal/modal-component-adapter.service.ts | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/app/public/modules/modal/modal-component-adapter.service.ts b/src/app/public/modules/modal/modal-component-adapter.service.ts
index a597590..1a1f526 100644
--- a/src/app/public/modules/modal/modal-component-adapter.service.ts
+++ b/src/app/public/modules/modal/modal-component-adapter.service.ts
@@ -115,11 +115,15 @@ export class SkyModalComponentAdapterService {
   private isVisible(element: HTMLElement) {
     const style = window.getComputedStyle(element);
     const isHidden = style.display === 'none' || style.visibility === 'hidden';
+    if (isHidden) {
+      return false;
+    }
+
     const hasBounds = !!(
       element.offsetWidth ||
       element.offsetHeight ||
       element.getClientRects().length
     );
-    return !isHidden && hasBounds;
+    return hasBounds;
   }
 }