Fix focus trap on the last element in a modal

.changeset/moody-poems-speak.md

@@ -0,0 +1,7 @@
+---
+"apostrophe": minor
+---
+
+Fixed an issue where using the Tab key to navigate within modals could incorrectly jump focus to a wrong element instead of the next input field.
+
+Fixed Tab navigation escaping out of modals when the form contained hidden sections or elements that became disabled after editing.

packages/apostrophe/modules/@apostrophecms/modal/ui/apos/components/AposModal.vue

@@ -16,7 +16,7 @@
       :data-apos-graph-key="props.graphKey || undefined"
       tabindex="0"
       @focus.capture="captureFocus"
-      @keyup.tab="onKeyup"
+      @keydown.tab="onKeydownTab"
       @keyup.esc="onKeyup"
     >
       <transition :name="transitionType">
@@ -216,6 +216,22 @@ const nonDraggableElements = [
   '.apos-input-array-inline-table'
 ];
 
+// Selector for focusable elements inside the modal. Used both at trap setup
+// and on every Tab keydown to refresh the cycle list, so that elements that
+// became disabled/hidden (e.g. Save when validation fails) or newly visible
+// are reflected.
+const focusableSelector = [
+  '[tabindex]',
+  '[href]',
+  'input',
+  'select',
+  'textarea',
+  'button',
+  '[data-apos-focus-priority]'
+]
+  .map(s => `${s}:not([tabindex="-1"]):not([disabled]):not([type="hidden"]):not([aria-hidden]):not(.apos-sr-only)`)
+  .join(', ');
+
 const resizeSides = [
   {
     edge: 'top',
@@ -476,13 +492,30 @@ onUnmounted(() => {
   }
 });
 
-function onKeyup(event) {
+// Handle Tab on keydown — before the browser moves focus.
+// Handling Tab on keyup is too late: the browser has already moved focus,
+// so the cycling logic sees the wrong activeElement.
+//
+// We also recompute the focusable list here on every Tab, scoped to
+// modalEl, instead of relying on the snapshot taken at trapFocus() time.
+function onKeydownTab(event) {
+  if (!shouldTrapFocus.value) {
+    return;
+  }
   if (!store.isOnTop(modalEl.value)) {
     return;
   }
+  if (event.target?.nodeName?.toLowerCase() === 'textarea') {
+    return;
+  }
+  const elements = getFocusableElements(modalEl.value);
+  // Keep the store snapshot consistent for other consumers.
+  store.updateModalData(props.modalData.id, { elementsToFocus: elements });
+  cycleElementsToFocus(event, elements);
+}
 
-  if (event.key === 'Tab') {
-    cycleElementsToFocus(event, props.modalData.elementsToFocus);
+function onKeyup(event) {
+  if (!store.isOnTop(modalEl.value)) {
     return;
   }
 
@@ -517,23 +550,20 @@ function captureFocus(e) {
   store.updateModalData(props.modalData.id, { focusedElement: e.target });
 }
 
+function getFocusableElements(rootEl) {
+  if (!rootEl) {
+    return [];
+  }
+  return [ ...rootEl.querySelectorAll(focusableSelector) ]
+    // a cheap "visible and in the DOM" condition,
+    // false positive expected for position: fixed and
+    // visually hidden elements (visible: hidden, opacity: 0, etc.)
+    .filter(el => el.offsetParent !== null);
+}
+
 async function trapFocus() {
   if (modalEl?.value) {
-    const elementSelectors = [
-      '[tabindex]',
-      '[href]',
-      'input',
-      'select',
-      'textarea',
-      'button',
-      '[data-apos-focus-priority]'
-    ];
-
-    const selector = elementSelectors
-      .map(addExcludingAttributes)
-      .join(', ');
-
-    const elementsToFocus = [ ...modalEl.value.querySelectorAll(selector) ];
+    const elementsToFocus = getFocusableElements(modalEl.value);
 
     store.updateModalData(props.modalData.id, { elementsToFocus });
 
@@ -556,10 +586,6 @@ async function trapFocus() {
     await nextTick();
     focusElement(props.modalData.focusedElement, firstElementToFocus);
   }
-
-  function addExcludingAttributes(element) {
-    return `${element}:not([tabindex="-1"]):not([disabled]):not([type="hidden"]):not([aria-hidden]):not(.apos-sr-only)`;
-  }
 }
 
 function close() {