diff --git a/packages/ui-modal/src/Modal/v1/ModalBody/index.tsx b/packages/ui-modal/src/Modal/v1/ModalBody/index.tsx index 974db84537..71b9efd834 100644 --- a/packages/ui-modal/src/Modal/v1/ModalBody/index.tsx +++ b/packages/ui-modal/src/Modal/v1/ModalBody/index.tsx @@ -33,7 +33,7 @@ import generateStyle from './styles.js' import generateComponentTheme from './theme.js' import { allowedProps } from './props.js' -import type { ModalBodyProps } from './props' +import type { ModalBodyProps, ModalBodyState } from './props' import { UIElement } from '@instructure/shared-types' import ModalContext from '../ModalContext.js' @@ -44,7 +44,7 @@ id: Modal.Body --- **/ @withStyle(generateStyle, generateComponentTheme) -class ModalBody extends Component { +class ModalBody extends Component { static displayName = 'ModalBody' static readonly componentId = 'Modal.Body' @@ -55,6 +55,11 @@ class ModalBody extends Component { variant: 'default' } + state: ModalBodyState = { isFirefox: false, needsTabIndex: false } + // mirrors state.needsTabIndex, but readable synchronously in the observer + // callbacks below, where a pending setState would make this.state stale + private lastNeedsTabIndex = false + ref: UIElement | null = null private resizeObserver?: ResizeObserver private mutationObserver?: MutationObserver @@ -92,9 +97,10 @@ class ModalBody extends Component { const finalRef = this.getFinalRef(this.ref) if (finalRef && typeof ResizeObserver !== 'undefined') { - this.resizeObserver = new ResizeObserver(() => this.forceUpdate()) + this.syncTabIndex() + this.resizeObserver = new ResizeObserver(this.syncTabIndex) this.resizeObserver.observe(finalRef) - this.mutationObserver = new MutationObserver(() => this.forceUpdate()) + this.mutationObserver = new MutationObserver(this.syncTabIndex) this.mutationObserver.observe(finalRef, { childList: true, subtree: true, @@ -122,6 +128,32 @@ class ModalBody extends Component { this.mutationObserver?.disconnect() } + // The body is a tab stop only while it can be scrolled but holds nothing + // focusable. Both inputs come from the DOM, so the observers recompute them + // on resize and on subtree changes — which is most changes inside the body, + // the vast majority of them leaving the result identical. Comparing against + // the last computed value before calling setState keeps those callbacks from + // scheduling an update at all, rather than scheduling one React later + // discards: `setState` warns about updates outside `act()` in tests as soon + // as it schedules, so bailing out inside the updater would be too late. + syncTabIndex = () => { + const finalRef = this.getFinalRef(this.ref) + const hasScrollbar = + !!finalRef && + Math.abs( + (finalRef.scrollHeight ?? 0) - + (finalRef.getBoundingClientRect()?.height ?? 0) + ) > 1 + const needsTabIndex = hasScrollbar && findTabbable(finalRef).length === 0 + + if (needsTabIndex === this.lastNeedsTabIndex) return + this.lastNeedsTabIndex = needsTabIndex + this.setState({ needsTabIndex }) + } + + // this recursive function is needed because `ref` can be a React component. + // TODO rethink, the 'as' prop, likely its not a good idea to allow React + // components. See INSTUI-4674 getFinalRef(el: UIElement): Element | undefined { if (!el) { return undefined @@ -144,18 +176,7 @@ class ModalBody extends Component { ModalBody ) const isFit = overflow === 'fit' - // this recursive function is needed because `ref` can be a React component. - // TODO rethink, the 'as' prop, likely its not a good idea to allow React - // components. See INSTUI-4674 - const finalRef = this.getFinalRef(this.ref) - const hasScrollbar = - finalRef && - Math.abs( - (finalRef.scrollHeight ?? 0) - - (finalRef.getBoundingClientRect()?.height ?? 0) - ) > 1 - const hasTabbableChildren = !!finalRef && findTabbable(finalRef).length > 0 - const needsTabIndex = hasScrollbar && !hasTabbableChildren + const { needsTabIndex } = this.state return ( {(value) => ( diff --git a/packages/ui-modal/src/Modal/v1/ModalBody/props.ts b/packages/ui-modal/src/Modal/v1/ModalBody/props.ts index 2e22407ad8..e91e921751 100644 --- a/packages/ui-modal/src/Modal/v1/ModalBody/props.ts +++ b/packages/ui-modal/src/Modal/v1/ModalBody/props.ts @@ -56,6 +56,7 @@ type ModalBodyStyle = ComponentStyle<'modalBody'> type ModalBodyState = { isFirefox: boolean + needsTabIndex: boolean } const allowedProps: AllowedPropKeys = [ 'children', diff --git a/packages/ui-modal/src/Modal/v2/ModalBody/index.tsx b/packages/ui-modal/src/Modal/v2/ModalBody/index.tsx index 9e4ffb5fc4..4e2971fb14 100644 --- a/packages/ui-modal/src/Modal/v2/ModalBody/index.tsx +++ b/packages/ui-modal/src/Modal/v2/ModalBody/index.tsx @@ -32,7 +32,7 @@ import { withStyleNew } from '@instructure/emotion' import generateStyle from './styles.js' import { allowedProps } from './props.js' -import type { ModalBodyProps } from './props' +import type { ModalBodyProps, ModalBodyState } from './props' import { UIElement } from '@instructure/shared-types' import ModalContext from '../ModalContext.js' @@ -43,7 +43,7 @@ id: Modal.Body --- **/ @withStyleNew(generateStyle, 'ModalBody') -class ModalBody extends Component { +class ModalBody extends Component { static displayName = 'ModalBody' static readonly componentId = 'Modal.Body' static readonly themeId = 'ModalBody' @@ -55,6 +55,11 @@ class ModalBody extends Component { variant: 'default' } + state: ModalBodyState = { isFirefox: false, needsTabIndex: false } + // mirrors state.needsTabIndex, but readable synchronously in the observer + // callbacks below, where a pending setState would make this.state stale + private lastNeedsTabIndex = false + ref: UIElement | null = null private resizeObserver?: ResizeObserver private mutationObserver?: MutationObserver @@ -92,9 +97,10 @@ class ModalBody extends Component { const finalRef = this.getFinalRef(this.ref) if (finalRef && typeof ResizeObserver !== 'undefined') { - this.resizeObserver = new ResizeObserver(() => this.forceUpdate()) + this.syncTabIndex() + this.resizeObserver = new ResizeObserver(this.syncTabIndex) this.resizeObserver.observe(finalRef) - this.mutationObserver = new MutationObserver(() => this.forceUpdate()) + this.mutationObserver = new MutationObserver(this.syncTabIndex) this.mutationObserver.observe(finalRef, { childList: true, subtree: true, @@ -122,6 +128,32 @@ class ModalBody extends Component { this.mutationObserver?.disconnect() } + // The body is a tab stop only while it can be scrolled but holds nothing + // focusable. Both inputs come from the DOM, so the observers recompute them + // on resize and on subtree changes — which is most changes inside the body, + // the vast majority of them leaving the result identical. Comparing against + // the last computed value before calling setState keeps those callbacks from + // scheduling an update at all, rather than scheduling one React later + // discards: `setState` warns about updates outside `act()` in tests as soon + // as it schedules, so bailing out inside the updater would be too late. + syncTabIndex = () => { + const finalRef = this.getFinalRef(this.ref) + const hasScrollbar = + !!finalRef && + Math.abs( + (finalRef.scrollHeight ?? 0) - + (finalRef.getBoundingClientRect()?.height ?? 0) + ) > 1 + const needsTabIndex = hasScrollbar && findTabbable(finalRef).length === 0 + + if (needsTabIndex === this.lastNeedsTabIndex) return + this.lastNeedsTabIndex = needsTabIndex + this.setState({ needsTabIndex }) + } + + // this recursive function is needed because `ref` can be a React component. + // TODO rethink, the 'as' prop, likely its not a good idea to allow React + // components. See INSTUI-4674 getFinalRef(el: UIElement): Element | undefined { if (!el) { return undefined @@ -152,18 +184,7 @@ class ModalBody extends Component { ModalBody ) const isFit = overflow === 'fit' - // this recursive function is needed because `ref` can be a React component. - // TODO rethink, the 'as' prop, likely its not a good idea to allow React - // components. See INSTUI-4674 - const finalRef = this.getFinalRef(this.ref) - const hasScrollbar = - finalRef && - Math.abs( - (finalRef.scrollHeight ?? 0) - - (finalRef.getBoundingClientRect()?.height ?? 0) - ) > 1 - const hasTabbableChildren = !!finalRef && findTabbable(finalRef).length > 0 - const needsTabIndex = hasScrollbar && !hasTabbableChildren + const { needsTabIndex } = this.state return ( {(value) => ( diff --git a/packages/ui-modal/src/Modal/v2/ModalBody/props.ts b/packages/ui-modal/src/Modal/v2/ModalBody/props.ts index ad6c0c8386..d86d22f6d1 100644 --- a/packages/ui-modal/src/Modal/v2/ModalBody/props.ts +++ b/packages/ui-modal/src/Modal/v2/ModalBody/props.ts @@ -62,6 +62,7 @@ type ModalBodyStyle = ComponentStyle<'modalBody'> type ModalBodyState = { isFirefox: boolean + needsTabIndex: boolean } const allowedProps: AllowedPropKeys = [ 'children',