Skip to content

Commit fb5b1f8

Browse files
author
Tim Sinaeve
committed
fix: prevent focus stealing in modal dialogs by optimizing focus trap logic
1 parent 4180438 commit fb5b1f8

1 file changed

Lines changed: 31 additions & 20 deletions

File tree

components/Modal.tsx

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ const Modal: React.FC<ModalProps> = ({ onClose, children, title, initialFocusRef
4040
};
4141
}, [onClose]);
4242

43-
// Effect for focus trapping
43+
// Effect for setting initial focus - RUNS ONLY ONCE
4444
useEffect(() => {
4545
const focusTimer = setTimeout(() => {
4646
const modalNode = modalRef.current;
@@ -58,23 +58,29 @@ const Modal: React.FC<ModalProps> = ({ onClose, children, title, initialFocusRef
5858
focusableElements[0].focus();
5959
}
6060
}
61-
}, 0); // Use a timeout to ensure the DOM is ready for focus
61+
}, 50); // Small delay to ensure render
6262

63-
// Focus trapping logic for Tab key
64-
const modalNode = modalRef.current;
65-
if (!modalNode) return () => clearTimeout(focusTimer);
66-
67-
const focusableElements = modalNode.querySelectorAll<HTMLElement>(
68-
'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
69-
);
70-
if (focusableElements.length === 0) return () => clearTimeout(focusTimer);
71-
72-
const firstElement = focusableElements[0];
73-
const lastElement = focusableElements[focusableElements.length - 1];
63+
return () => clearTimeout(focusTimer);
64+
}, []); // Empty dependency array ensures this runs only on mount
7465

66+
// Effect for focus trapping (Tab key)
67+
useEffect(() => {
7568
const handleTabKey = (e: KeyboardEvent) => {
7669
if (e.key !== 'Tab') return;
7770

71+
const modalNode = modalRef.current;
72+
if (!modalNode) return;
73+
74+
// Re-query focusable elements every time Tab is pressed to handle dynamic content changes
75+
const focusableElements = modalNode.querySelectorAll<HTMLElement>(
76+
'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
77+
);
78+
79+
if (focusableElements.length === 0) return;
80+
81+
const firstElement = focusableElements[0];
82+
const lastElement = focusableElements[focusableElements.length - 1];
83+
7884
if (e.shiftKey) { // Shift + Tab
7985
if (document.activeElement === firstElement) {
8086
e.preventDefault();
@@ -87,16 +93,21 @@ const Modal: React.FC<ModalProps> = ({ onClose, children, title, initialFocusRef
8793
}
8894
}
8995
};
90-
91-
modalNode.addEventListener('keydown', handleTabKey);
96+
97+
// Attach listener to the specific modal node if possible, or window/document if needed for trapping.
98+
// Attaching to modalNode is better for containment, but we need to ensure the modal has focus.
99+
// For a robust trap, listening on the modal node is good IF the focus is inside.
100+
const modalNode = modalRef.current;
101+
if (modalNode) {
102+
modalNode.addEventListener('keydown', handleTabKey);
103+
}
92104

93105
return () => {
94-
clearTimeout(focusTimer);
95-
if (modalNode) {
96-
modalNode.removeEventListener('keydown', handleTabKey);
97-
}
106+
if (modalNode) {
107+
modalNode.removeEventListener('keydown', handleTabKey);
108+
}
98109
};
99-
}, [onClose, initialFocusRef]);
110+
}, []); // Run once to attach handlers
100111

101112
const modalContent = (
102113
<div

0 commit comments

Comments
 (0)