Fix constrained tabbing failures with Safari and Firefox (#47426)

* Make the modal content container focusable when scrollable.

* Add test, improve existing a11y tests and run them with all browsers.

* Fix useConstrainedTabbing edge cases.

* Add basic focus style for the scrollable section.

* Update snapshots.

* Clean up a11y test.

* Refine test.

* Remove scrollableContentLabel prop.

* Restore conditional aria-label for the scrollable section.

* Update test after change to closeButtonLabel default.

* Add comment to clarify test.

packages/components/src/modal/index.tsx

@@ -14,6 +14,7 @@ import {
 	useRef,
 	useState,
 	forwardRef,
+	useLayoutEffect,
 } from '@wordpress/element';
 import {
 	useInstanceId,
@@ -25,6 +26,7 @@ import {
 } from '@wordpress/compose';
 import { __ } from '@wordpress/i18n';
 import { close } from '@wordpress/icons';
+import { getScrollContainer } from '@wordpress/dom';
 
 /**
  * Internal dependencies
@@ -76,8 +78,26 @@ function UnforwardedModal(
 	const constrainedTabbingRef = useConstrainedTabbing();
 	const focusReturnRef = useFocusReturn();
 	const focusOutsideProps = useFocusOutside( onRequestClose );
+	const contentRef = useRef< HTMLDivElement >( null );
+	const childrenContainerRef = useRef< HTMLDivElement >( null );
 
 	const [ hasScrolledContent, setHasScrolledContent ] = useState( false );
+	const [ hasScrollableContent, setHasScrollableContent ] = useState( false );
+
+	// Determines whether the Modal content is scrollable and updates the state.
+	const isContentScrollable = useCallback( () => {
+		if ( ! contentRef.current ) {
+			return;
+		}
+
+		const closestScrollContainer = getScrollContainer( contentRef.current );
+
+		if ( contentRef.current === closestScrollContainer ) {
+			setHasScrollableContent( true );
+		} else {
+			setHasScrollableContent( false );
+		}
+	}, [ contentRef ] );
 
 	useEffect( () => {
 		openModalCount++;
@@ -97,6 +117,22 @@ function UnforwardedModal(
 		};
 	}, [ bodyOpenClassName ] );
 
+	// Calls the isContentScrollable callback when the Modal children container resizes.
+	useLayoutEffect( () => {
+		if ( ! window.ResizeObserver || ! childrenContainerRef.current ) {
+			return;
+		}
+
+		const resizeObserver = new ResizeObserver( isContentScrollable );
+		resizeObserver.observe( childrenContainerRef.current );
+
+		isContentScrollable();
+
+		return () => {
+			resizeObserver.disconnect();
+		};
+	}, [ isContentScrollable, childrenContainerRef ] );
+
 	function handleEscapeKeyDown( event: KeyboardEvent< HTMLDivElement > ) {
 		if (
 			// Ignore keydowns from IMEs
@@ -172,10 +208,18 @@ function UnforwardedModal(
 					<div
 						className={ classnames( 'components-modal__content', {
 							'hide-header': __experimentalHideHeader,
+							'is-scrollable': hasScrollableContent,
 							'has-scrolled-content': hasScrolledContent,
 						} ) }
 						role="document"
 						onScroll={ onContentContainerScroll }
+						ref={ contentRef }
+						aria-label={
+							hasScrollableContent
+								? __( 'Scrollable section' )
+								: undefined
+						}
+						tabIndex={ hasScrollableContent ? 0 : undefined }
 					>
 						{ ! __experimentalHideHeader && (
 							<div className="components-modal__header">
@@ -208,7 +252,7 @@ function UnforwardedModal(
 								) }
 							</div>
 						) }
-						{ children }
+						<div ref={ childrenContainerRef }>{ children }</div>
 					</div>
 				</div>
 			</StyleProvider>

packages/components/src/modal/style.scss

@@ -129,4 +129,12 @@
 		margin-top: 0;
 		padding-top: $grid-unit-30;
 	}
+
+	&.is-scrollable:focus-visible {
+		box-shadow: inset 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color);
+
+		// Windows High Contrast mode will show this outline, but not the box-shadow.
+		outline: 2px solid transparent;
+		outline-offset: -2px;
+	}
 }

packages/components/src/modal/types.ts

@@ -50,6 +50,8 @@ export type ModalProps = {
 	className?: string;
 	/**
 	 * Label on the close button.
+	 *
+	 * @default `__( 'Close' )`
 	 */
 	closeButtonLabel?: string;
 	/**

packages/compose/src/hooks/use-constrained-tabbing/index.js

@@ -45,14 +45,31 @@ function useConstrainedTabbing() {
 					/** @type {HTMLElement} */ ( target )
 				) || null;
 
-			// If the element that is about to receive focus is outside the
-			// area, move focus to a div and insert it at the start or end of
-			// the area, depending on the direction. Without preventing default
-			// behaviour, the browser will then move focus to the next element.
+			// When the target element contains the element that is about to
+			// receive focus, for example when the target is a tabbable
+			// container, browsers may disagree on where to move focus next.
+			// In this case we can't rely on native browsers behavior. We need
+			// to manage focus instead.
+			// See https://github.com/WordPress/gutenberg/issues/46041.
+			if (
+				/** @type {HTMLElement} */ ( target ).contains( nextElement )
+			) {
+				event.preventDefault();
+				/** @type {HTMLElement} */ ( nextElement )?.focus();
+				return;
+			}
+
+			// If the element that is about to receive focus is inside the
+			// area, rely on native browsers behavior and let tabbing follow
+			// the native tab sequence.
 			if ( node.contains( nextElement ) ) {
 				return;
 			}
 
+			// If the element that is about to receive focus is outside the
+			// area, move focus to a div and insert it at the start or end of
+			// the area, depending on the direction. Without preventing default
+			// behaviour, the browser will then move focus to the next element.
 			const domAction = shiftKey ? 'append' : 'prepend';
 			const { ownerDocument } = node;
 			const trap = ownerDocument.createElement( 'div' );

packages/dom/README.md

@@ -135,7 +135,8 @@ _Returns_
 
 ### getScrollContainer
 
-Given a DOM node, finds the closest scrollable container node.
+Given a DOM node, finds the closest scrollable container node or the node
+itself, if scrollable.
 
 _Parameters_
 

packages/dom/src/dom/get-scroll-container.js

@@ -4,7 +4,8 @@
 import getComputedStyle from './get-computed-style';
 
 /**
- * Given a DOM node, finds the closest scrollable container node.
+ * Given a DOM node, finds the closest scrollable container node or the node
+ * itself, if scrollable.
  *
  * @param {Element | null} node Node from which to start.
  *