From 9ff6ef1aeffd5a6466ae316e8dc892da2ed35c0c Mon Sep 17 00:00:00 2001 From: Pavithra Kodmad Date: Mon, 6 Nov 2023 10:05:07 +1100 Subject: [PATCH 1/9] Make resize vertical splitter keyboard accessible --- src/PageLayout/PageLayout.tsx | 164 +++++++++++++++++----------------- 1 file changed, 81 insertions(+), 83 deletions(-) diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index e516be2189f..124593fc490 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -1,4 +1,4 @@ -import React from 'react' +import React, {useRef} from 'react' import {createGlobalStyle} from 'styled-components' import Box from '../Box' import {useId} from '../hooks/useId' @@ -10,7 +10,6 @@ import {Theme} from '../ThemeProvider' import {canUseDOM} from '../utils/environment' import {useOverflow} from '../internal/hooks/useOverflow' import {warning} from '../utils/warning' -import VisuallyHidden from '../_VisuallyHidden' import {useStickyPaneHeight} from './useStickyPaneHeight' const REGION_ORDER = { @@ -31,6 +30,7 @@ const PageLayoutContext = React.createContext<{ padding: keyof typeof SPACING_MAP rowGap: keyof typeof SPACING_MAP columnGap: keyof typeof SPACING_MAP + paneRef?: React.RefObject enableStickyPane?: (top: number | string) => void disableStickyPane?: () => void contentTopRef?: (node?: Element | null | undefined) => void @@ -76,6 +76,8 @@ const Root: React.FC> = ({ const {rootRef, enableStickyPane, disableStickyPane, contentTopRef, contentBottomRef, stickyPaneHeight} = useStickyPaneHeight() + const paneRef = useRef(null) + const [slots, rest] = useSlots(children, slotsConfig ?? {header: Header, footer: Footer}) return ( @@ -88,6 +90,7 @@ const Root: React.FC> = ({ disableStickyPane, contentTopRef, contentBottomRef, + paneRef, }} > void - onDrag?: (delta: number) => void + onDrag?: (delta: number, isKeyboard: boolean) => void onDragEnd?: () => void onDoubleClick?: () => void } @@ -224,11 +227,34 @@ const VerticalDivider: React.FC { const [isDragging, setIsDragging] = React.useState(false) + const [isKeyboardDrag, setIsKeyboardDrag] = React.useState(false) const responsiveVariant = useResponsiveValue(variant, 'none') const stableOnDrag = React.useRef(onDrag) const stableOnDragEnd = React.useRef(onDragEnd) + const {paneRef} = React.useContext(PageLayoutContext) + + const [minWidth, setMinWidth] = React.useState(0) + const [maxWidth, setMaxWidth] = React.useState(0) + const [currentWidth, setCurrentWidth] = React.useState(0) + + React.useEffect(() => { + if (paneRef?.current !== null) { + const paneStyles = getComputedStyle(paneRef?.current as Element) + const maxPaneWidthDiffPixels = paneStyles.getPropertyValue('--pane-max-width-diff') + const minWidthPixels = paneStyles.getPropertyValue('--pane-min-width') + const paneWidth = paneRef?.current.getBoundingClientRect().width + const maxPaneWidthDiff = Number(maxPaneWidthDiffPixels.split('px')[0]) + const minPaneWidth = Number(minWidthPixels.split('px')[0]) + const viewportWidth = window.innerWidth + const maxPaneWidth = viewportWidth > maxPaneWidthDiff ? viewportWidth - maxPaneWidthDiff : viewportWidth + setMinWidth(minPaneWidth) + setMaxWidth(maxPaneWidth) + setCurrentWidth(paneWidth || 0) + } + }, [paneRef, isKeyboardDrag, isDragging]) + React.useEffect(() => { stableOnDrag.current = onDrag }, [onDrag]) @@ -239,7 +265,7 @@ const VerticalDivider: React.FC { function handleDrag(event: MouseEvent) { - stableOnDrag.current?.(event.movementX) + stableOnDrag.current?.(event.movementX, false) event.preventDefault() } @@ -249,14 +275,38 @@ const VerticalDivider: React.FC minWidth) { + delta = -3 + } else if (event.key === 'ArrowRight' && currentWidth < maxWidth) { + delta = 3 + } else { + return + } + setCurrentWidth(currentWidth + delta) + stableOnDrag.current?.(delta, true) + event.preventDefault() + } + + function handleKeyDragEnd(event: KeyboardEvent) { + setIsKeyboardDrag(false) + stableOnDragEnd.current?.() + event.preventDefault() + } // TODO: Support touch events - if (isDragging) { + if (isDragging || isKeyboardDrag) { window.addEventListener('mousemove', handleDrag) + window.addEventListener('keydown', handleKeyDrag) window.addEventListener('mouseup', handleDragEnd) + window.addEventListener('keyup', handleKeyDragEnd) document.body.setAttribute('data-page-layout-dragging', 'true') } else { window.removeEventListener('mousemove', handleDrag) window.removeEventListener('mouseup', handleDragEnd) + window.removeEventListener('keydown', handleKeyDrag) + window.removeEventListener('keyup', handleKeyDragEnd) document.body.removeAttribute('data-page-layout-dragging') } @@ -265,7 +315,7 @@ const VerticalDivider: React.FC { setIsDragging(true) onDragStart?.() }} + onKeyDown={event => { + if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') { + setIsKeyboardDrag(true) + onDragStart?.() + } + }} onDoubleClick={onDoubleClick} /> @@ -593,6 +656,8 @@ const Pane = React.forwardRef(null) React.useEffect(() => { if (sticky) { @@ -637,55 +702,10 @@ const Pane = React.forwardRef(null) useRefObjectAsForwardedRef(forwardRef, paneRef) - const [minPercent, setMinPercent] = React.useState(0) - const [maxPercent, setMaxPercent] = React.useState(0) const hasOverflow = useOverflow(paneRef) - const measuredRef = React.useCallback(() => { - if (paneRef.current !== null) { - const maxPaneWidthDiffPixels = getComputedStyle(paneRef.current as Element).getPropertyValue( - '--pane-max-width-diff', - ) - const paneWidth = paneRef.current.getBoundingClientRect().width - const maxPaneWidthDiff = Number(maxPaneWidthDiffPixels.split('px')[0]) - const viewportWidth = window.innerWidth - const maxPaneWidth = viewportWidth > maxPaneWidthDiff ? viewportWidth - maxPaneWidthDiff : viewportWidth - - const minPercent = Math.round((100 * minWidth) / viewportWidth) - setMinPercent(minPercent) - - const maxPercent = Math.round((100 * maxPaneWidth) / viewportWidth) - setMaxPercent(maxPercent) - - const widthPercent = Math.round((100 * paneWidth) / viewportWidth) - setWidthPercent(widthPercent.toString()) - } - }, [paneRef, minWidth]) - - const [widthPercent, setWidthPercent] = React.useState('') - const [prevPercent, setPrevPercent] = React.useState('') - - const handleWidthFormSubmit = (event: React.FormEvent) => { - event.preventDefault() - let percent = Number(widthPercent) - if (Number.isNaN(percent)) { - percent = Number(prevPercent) || minPercent - } else if (percent > maxPercent) { - percent = maxPercent - } else if (percent < minPercent) { - percent = minPercent - } - - setWidthPercent(percent.toString()) - // Cache previous valid percent. - setPrevPercent(percent.toString()) - - updatePaneWidth((percent / 100) * window.innerWidth) - } - const paneId = useId(id) const labelProp: {'aria-labelledby'?: string; 'aria-label'?: string} = {} @@ -706,7 +726,6 @@ const Pane = React.forwardRef merge( @@ -756,14 +775,19 @@ const Pane = React.forwardRef { + onDrag={(delta, isKeyboard = false) => { // Get the number of pixels the divider was dragged - const deltaWithDirection = position === 'end' ? -delta : delta + let deltaWithDirection + if (isKeyboard) { + deltaWithDirection = delta + } else { + deltaWithDirection = position === 'end' ? -delta : delta + } updatePaneWidth(paneWidth + deltaWithDirection) }} // Ensure `paneWidth` state and actual pane width are in sync when the drag ends onDragEnd={() => { - const paneRect = paneRef.current?.getBoundingClientRect() + const paneRect = paneRef?.current?.getBoundingClientRect() if (!paneRect) return updatePaneWidth(paneRect.width) }} @@ -798,32 +822,6 @@ const Pane = React.forwardRef {children} - {resizable && ( - // eslint-disable-next-line github/a11y-no-visually-hidden-interactive-element - -
- -

- Use a value between {minPercent}% and {maxPercent}% -

- { - setWidthPercent(event.target.value) - }} - /> - -
- - )} ) From 6cc3864ff10dbe72a37bc520a8e35eac9b417b8f Mon Sep 17 00:00:00 2001 From: Pavithra Kodmad Date: Mon, 6 Nov 2023 10:53:34 +1100 Subject: [PATCH 2/9] Fix types around paneRef --- src/PageLayout/PageLayout.tsx | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index 124593fc490..e2816db4366 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -30,7 +30,7 @@ const PageLayoutContext = React.createContext<{ padding: keyof typeof SPACING_MAP rowGap: keyof typeof SPACING_MAP columnGap: keyof typeof SPACING_MAP - paneRef?: React.RefObject + paneRef: React.RefObject enableStickyPane?: (top: number | string) => void disableStickyPane?: () => void contentTopRef?: (node?: Element | null | undefined) => void @@ -39,6 +39,7 @@ const PageLayoutContext = React.createContext<{ padding: 'normal', rowGap: 'normal', columnGap: 'normal', + paneRef: {current: null}, }) // ---------------------------------------------------------------------------- @@ -240,11 +241,11 @@ const VerticalDivider: React.FC { - if (paneRef?.current !== null) { - const paneStyles = getComputedStyle(paneRef?.current as Element) + if (paneRef.current !== null) { + const paneStyles = getComputedStyle(paneRef.current as Element) const maxPaneWidthDiffPixels = paneStyles.getPropertyValue('--pane-max-width-diff') const minWidthPixels = paneStyles.getPropertyValue('--pane-min-width') - const paneWidth = paneRef?.current.getBoundingClientRect().width + const paneWidth = paneRef.current.getBoundingClientRect().width const maxPaneWidthDiff = Number(maxPaneWidthDiffPixels.split('px')[0]) const minPaneWidth = Number(minWidthPixels.split('px')[0]) const viewportWidth = window.innerWidth @@ -315,7 +316,7 @@ const VerticalDivider: React.FC(null) + const {rowGap, columnGap, enableStickyPane, disableStickyPane, paneRef} = React.useContext(PageLayoutContext) React.useEffect(() => { if (sticky) { @@ -787,7 +786,7 @@ const Pane = React.forwardRef { - const paneRect = paneRef?.current?.getBoundingClientRect() + const paneRect = paneRef.current?.getBoundingClientRect() if (!paneRect) return updatePaneWidth(paneRect.width) }} From 775b30a47ac3f02773b930e836df0c8b364c802f Mon Sep 17 00:00:00 2001 From: Pavithra Kodmad Date: Tue, 7 Nov 2023 15:41:27 +1100 Subject: [PATCH 3/9] Fix leaking drag handler event --- src/PageLayout/PageLayout.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index e2816db4366..8827a3caf68 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -314,6 +314,8 @@ const VerticalDivider: React.FC { window.removeEventListener('mousemove', handleDrag) window.removeEventListener('mouseup', handleDragEnd) + window.removeEventListener('keydown', handleKeyDrag) + window.removeEventListener('keyup', handleKeyDragEnd) document.body.removeAttribute('data-page-layout-dragging') } }, [isDragging, isKeyboardDrag, currentWidth, minWidth, maxWidth]) From b9805f1db09eee184fdeba82fa9691a63195a49c Mon Sep 17 00:00:00 2001 From: Pavithra Kodmad Date: Wed, 22 Nov 2023 17:37:56 +1100 Subject: [PATCH 4/9] Change role to slider --- src/PageLayout/PageLayout.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index 8827a3caf68..6c3a7cf9629 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -345,7 +345,7 @@ const VerticalDivider: React.FC Date: Wed, 22 Nov 2023 17:48:28 +1100 Subject: [PATCH 5/9] Change aria label --- src/PageLayout/PageLayout.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index 6c3a7cf9629..7b86a224be8 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -346,7 +346,7 @@ const VerticalDivider: React.FC Date: Thu, 23 Nov 2023 10:15:39 +1100 Subject: [PATCH 6/9] Implement feedback from accessibility expert --- src/PageLayout/PageLayout.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index 7b86a224be8..4ee795c38b9 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -278,10 +278,10 @@ const VerticalDivider: React.FC minWidth) { + // https://github.com/github/accessibility/issues/5101#issuecomment-1822870655 + if ((event.key === 'ArrowLeft' || event.key === 'ArrowDown') && currentWidth > minWidth) { delta = -3 - } else if (event.key === 'ArrowRight' && currentWidth < maxWidth) { + } else if ((event.key === 'ArrowRight' || event.key === 'ArrowUp') && currentWidth < maxWidth) { delta = 3 } else { return @@ -349,7 +349,6 @@ const VerticalDivider: React.FC Date: Thu, 23 Nov 2023 10:25:11 +1100 Subject: [PATCH 7/9] Update test to use role slider --- src/PageLayout/PageLayout.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PageLayout/PageLayout.test.tsx b/src/PageLayout/PageLayout.test.tsx index ab96d4beaed..243ba9eef93 100644 --- a/src/PageLayout/PageLayout.test.tsx +++ b/src/PageLayout/PageLayout.test.tsx @@ -191,7 +191,7 @@ describe('PageLayout', () => { const pane = placeholder.parentNode const initialWidth = (pane as HTMLElement).style.getPropertyValue('--pane-width') - const divider = await screen.findByRole('separator') + const divider = await screen.findByRole('slider') // Moving divider should resize pane. fireEvent.mouseDown(divider) fireEvent.mouseMove(divider) From 0b3bf60e0d6f14678fbf35fb7d22d08bb5989b3c Mon Sep 17 00:00:00 2001 From: Pavithra Kodmad Date: Thu, 23 Nov 2023 10:34:42 +1100 Subject: [PATCH 8/9] Fix arrow keys --- src/PageLayout/PageLayout.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index 4ee795c38b9..a2321b06b40 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -357,7 +357,12 @@ const VerticalDivider: React.FC { - if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') { + if ( + event.key === 'ArrowLeft' || + event.key === 'ArrowRight' || + event.key === 'ArrowUp' || + event.key === 'ArrowDown' + ) { setIsKeyboardDrag(true) onDragStart?.() } From 595fd420c341dee4523075e8bfba29fa5ae813d7 Mon Sep 17 00:00:00 2001 From: Pavithra Kodmad Date: Thu, 23 Nov 2023 10:35:06 +1100 Subject: [PATCH 9/9] Create proud-dingos-occur.md --- .changeset/proud-dingos-occur.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/proud-dingos-occur.md diff --git a/.changeset/proud-dingos-occur.md b/.changeset/proud-dingos-occur.md new file mode 100644 index 00000000000..a9c0a86f44e --- /dev/null +++ b/.changeset/proud-dingos-occur.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Make resize vertical splitter keyboard accessible