From 8d579c18cc9b57511509f24c369a586fd7b7fb8f Mon Sep 17 00:00:00 2001
From: konnorrogers <konnor5456@gmail.com>
Date: Fri, 1 Dec 2023 18:09:11 -0500
Subject: [PATCH] account for elements with tabbable controls

---
 src/components/dialog/dialog.component.ts |  4 +--
 src/internal/modal.ts                     | 32 +++++++++++++++++++++--
 src/internal/tabbable.ts                  |  2 +-
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/components/dialog/dialog.component.ts b/src/components/dialog/dialog.component.ts
index 30afa3d5..6d31d02a 100644
--- a/src/components/dialog/dialog.component.ts
+++ b/src/components/dialog/dialog.component.ts
@@ -300,9 +300,9 @@ export default class SlDialog extends ShoelaceElement {
               `
             : ''}
           ${
-            '' /* The tabindex="-1" is here because the body is technically scrollable if overflowing. However, if there's no focusable elements inside, you won't actually be able to scroll it via keyboard. */
+          '' /* The tabindex="-1" is here because the body is technically scrollable if overflowing. However, if there's no focusable elements inside, you won't actually be able to scroll it via keyboard. Previously this was just a <slot>, but tabindex="-1" on the slot causes children to not be focusable. https://github.com/shoelace-style/shoelace/issues/1753#issuecomment-1836803277 */
           }
-          <slot part="body" class="dialog__body" tabindex="-1"></slot>
+          <div part="body" class="dialog__body" tabindex="-1"><slot></slot></div>
 
           <footer part="footer" class="dialog__footer">
             <slot name="footer"></slot>
diff --git a/src/internal/modal.ts b/src/internal/modal.ts
index c61a7276..10cca881 100644
--- a/src/internal/modal.ts
+++ b/src/internal/modal.ts
@@ -71,14 +71,26 @@ export default class Modal {
     if (event.key !== 'Tab' || this.isExternalActivated) return;
     if (!this.isActive()) return;
 
+    const elementsWithTabbableControls = [
+      "audio",
+      "video",
+      "iframe"
+    ]
+
+    const possiblyHasTabbableChildren = (element: HTMLElement) => {
+      return (
+        elementsWithTabbableControls.includes(element.tagName.toLowerCase())
+        || element.hasAttribute("controls")
+        // Should we add a data-attribute for people to set just in case they have an element where we don't know if it has possibly tabbable elements?
+      )
+    }
+
     if (event.shiftKey) {
       this.tabDirection = 'backward';
     } else {
       this.tabDirection = 'forward';
     }
 
-    event.preventDefault();
-
     const tabbableElements = getTabbableElements(this.element);
 
     // Because sometimes focus can actually be taken over from outside sources,
@@ -89,6 +101,14 @@ export default class Modal {
 
     if (currentFocusIndex === -1) {
       this.currentFocus = tabbableElements[0];
+
+      // We don't call event.preventDefault() here because it messes with tabbing to the <iframe> controls.
+      // We just wait until the current focus is no longer an element with possible hidden controls.
+      if (possiblyHasTabbableChildren(this.currentFocus)) {
+        return
+      }
+
+      event.preventDefault();
       this.currentFocus?.focus({ preventScroll: true });
       return;
     }
@@ -104,6 +124,14 @@ export default class Modal {
     }
 
     this.currentFocus = tabbableElements[currentFocusIndex];
+
+    // We don't call event.preventDefault() here because it messes with tabbing to the <iframe> controls.
+    // We just wait until the current focus is no longer an element with possible hidden controls.
+    if (possiblyHasTabbableChildren(this.currentFocus)) {
+      return
+    }
+
+    event.preventDefault()
     this.currentFocus?.focus({ preventScroll: true });
 
     setTimeout(() => this.checkFocus());
diff --git a/src/internal/tabbable.ts b/src/internal/tabbable.ts
index 4b274d97..89034eaa 100644
--- a/src/internal/tabbable.ts
+++ b/src/internal/tabbable.ts
@@ -58,7 +58,7 @@ function isTabbable(el: HTMLElement) {
   }
 
   // At this point, the following elements are considered tabbable
-  return ['button', 'input', 'select', 'textarea', 'a', 'audio', 'video', 'summary'].includes(tag);
+  return ['button', 'input', 'select', 'textarea', 'a', 'audio', 'video', 'summary', 'iframe'].includes(tag);
 }
 
 /**