-
Notifications
You must be signed in to change notification settings - Fork 670
Export PageLayout resizable functionality
#8160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
74c731b
Export `PageLayout` resizable functionality
TylerJDev dc7f02e
Add dev story
TylerJDev 85c1936
chore: auto-fix lint and formatting issues
TylerJDev b543c58
Update snapshots
TylerJDev ee349a4
Address some feedback
TylerJDev 172e8a5
Remove some exports
TylerJDev 8be3084
Address more feedback
TylerJDev 86ff1e5
Address even more feedback
TylerJDev 7b683f4
Potential fix for pull request finding
TylerJDev 9991fe0
Potential fix for pull request finding
TylerJDev ce55f77
Potential fix for pull request finding
TylerJDev 775dfea
Potential fix for pull request finding
TylerJDev 495eac3
Fix test
TylerJDev b07d552
Fix another test
TylerJDev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@primer/react': minor | ||
| --- | ||
|
|
||
| PageLayout: Export the resize primitives `usePaneWidth` and `DragHandle` (with `defaultPaneWidth` and related types) so pane-resize behavior can be reused outside of `PageLayout`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,250 @@ | ||
| import React, {memo} from 'react' | ||
| import classes from './PageLayout.module.css' | ||
| import {ARROW_KEY_STEP, defaultFormatValueText} from './usePaneWidth' | ||
| import {setDraggingStyles, removeDraggingStyles} from './paneUtils' | ||
|
|
||
| const isArrowKey = (key: string) => | ||
| key === 'ArrowLeft' || key === 'ArrowRight' || key === 'ArrowUp' || key === 'ArrowDown' | ||
| const isShrinkKey = (key: string) => key === 'ArrowLeft' || key === 'ArrowDown' | ||
|
|
||
| export type DragHandleProps = { | ||
| /** Ref for imperative ARIA updates during drag */ | ||
| handleRef: React.RefObject<HTMLDivElement> | ||
| /** Called once when drag starts with initial cursor X position */ | ||
| onDragStart: (clientX: number) => void | ||
| /** Called on each drag tick with cursor position (pointer) or delta (keyboard) */ | ||
| onDrag: (value: number, isKeyboard: boolean) => void | ||
| /** Called when drag operation completes */ | ||
| onDragEnd: () => void | ||
| /** Reset width on double-click */ | ||
| onDoubleClick?: React.MouseEventHandler<HTMLDivElement> | ||
| /** | ||
| * Accessible name for the slider. Defaults to `'Draggable pane splitter'`. | ||
| * Provide a context-specific or localized label for standalone usage. | ||
| * Ignored when `aria-labelledby` is provided. | ||
| */ | ||
| 'aria-label'?: string | ||
| /** Id of an element that labels the slider. Takes precedence over `aria-label`. */ | ||
| 'aria-labelledby'?: string | ||
| /** ARIA slider min value */ | ||
| 'aria-valuemin'?: number | ||
| /** ARIA slider max value */ | ||
| 'aria-valuemax'?: number | ||
| /** ARIA slider current value */ | ||
| 'aria-valuenow'?: number | ||
| /** | ||
| * Ref to the element that receives drag-time styling/containment optimizations | ||
| * (e.g. the resizable pane). When omitted, those side effects are skipped. | ||
| */ | ||
| dragTargetRef?: React.RefObject<HTMLElement | null> | ||
| /** | ||
| * Ref to a content wrapper element that receives drag-time containment. | ||
| * Only needed for layouts (like PageLayout) that optimize sibling content during drag. | ||
| */ | ||
| contentWrapperRef?: React.RefObject<HTMLElement | null> | ||
| /** | ||
| * Formats the `aria-valuetext` announced for the current width. | ||
| * @default valueNow => `Pane width ${valueNow} pixels` | ||
| */ | ||
| formatValueText?: (valueNow: number) => string | ||
| /** Value for the `data-component` attribute. */ | ||
| 'data-component'?: string | ||
| } | ||
|
|
||
| /** | ||
| * DragHandle - handles all pointer and keyboard interactions for resizing. | ||
| * ARIA values are set in JSX for SSR accessibility, then updated via DOM | ||
| * manipulation during drag for performance. The consumer owns writing the width | ||
| * (via the `onDrag` callback); this component only reports interactions. | ||
| */ | ||
| export const DragHandle = memo<DragHandleProps>(function DragHandle({ | ||
| handleRef, | ||
| onDragStart, | ||
| onDrag, | ||
| onDragEnd, | ||
| onDoubleClick, | ||
| 'aria-label': ariaLabel, | ||
| 'aria-labelledby': ariaLabelledBy, | ||
| 'aria-valuemin': ariaValueMin, | ||
| 'aria-valuemax': ariaValueMax, | ||
| 'aria-valuenow': ariaValueNow, | ||
| dragTargetRef, | ||
| contentWrapperRef, | ||
| formatValueText = defaultFormatValueText, | ||
| 'data-component': dataComponent = 'PageLayout.DragHandle', | ||
| }) { | ||
|
Comment on lines
+60
to
+75
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A near 1:1 with what we already had in There are a few changes, such as props to improve the portability. Behavior and functionality should remain the same within |
||
| const stableOnDragStart = React.useRef(onDragStart) | ||
| const stableOnDrag = React.useRef(onDrag) | ||
| const stableOnDragEnd = React.useRef(onDragEnd) | ||
| React.useEffect(() => { | ||
| stableOnDragStart.current = onDragStart | ||
| stableOnDrag.current = onDrag | ||
| stableOnDragEnd.current = onDragEnd | ||
| }) | ||
|
|
||
| // Dragging state as a ref - cheaper than reading from DOM style | ||
| const isDraggingRef = React.useRef(false) | ||
|
|
||
| // Set inline styles for drag optimizations - zero overhead at rest | ||
| const startDragging = React.useCallback(() => { | ||
| if (isDraggingRef.current) return | ||
| setDraggingStyles({ | ||
| handle: handleRef.current, | ||
| pane: dragTargetRef?.current ?? null, | ||
| contentWrapper: contentWrapperRef?.current ?? null, | ||
| }) | ||
| isDraggingRef.current = true | ||
| }, [handleRef, contentWrapperRef, dragTargetRef]) | ||
|
|
||
| const endDragging = React.useCallback(() => { | ||
| if (!isDraggingRef.current) return | ||
| removeDraggingStyles({ | ||
| handle: handleRef.current, | ||
| pane: dragTargetRef?.current ?? null, | ||
| contentWrapper: contentWrapperRef?.current ?? null, | ||
| }) | ||
| isDraggingRef.current = false | ||
| }, [handleRef, contentWrapperRef, dragTargetRef]) | ||
|
|
||
| /** | ||
| * Pointer down starts a drag operation | ||
| * Capture the pointer to continue receiving events outside the handle area | ||
| */ | ||
| const handlePointerDown = React.useCallback( | ||
| (event: React.PointerEvent<HTMLDivElement>) => { | ||
| if (event.button !== 0) return | ||
| event.preventDefault() | ||
| const target = event.currentTarget | ||
| // Try to capture pointer - may fail in test environments or if pointer is already released | ||
| try { | ||
| target.setPointerCapture(event.pointerId) | ||
| } catch { | ||
| // Ignore - pointer capture is a nice-to-have for dragging outside the element | ||
| } | ||
| stableOnDragStart.current(event.clientX) | ||
| startDragging() | ||
| }, | ||
| [startDragging], | ||
| ) | ||
|
|
||
| // Simple rAF throttle - one update per frame, latest position wins | ||
| const rafIdRef = React.useRef<number | null>(null) | ||
| const pendingClientXRef = React.useRef<number | null>(null) | ||
|
|
||
| /** | ||
| * Pointer move during drag | ||
| * Calls onDrag with absolute cursor X position | ||
| * Uses rAF to coalesce updates to one per frame | ||
| */ | ||
| const handlePointerMove = React.useCallback((event: React.PointerEvent<HTMLDivElement>) => { | ||
| if (!isDraggingRef.current) return | ||
| event.preventDefault() | ||
|
|
||
| // Store latest position - only the final position before rAF fires matters | ||
| pendingClientXRef.current = event.clientX | ||
|
|
||
| // Schedule update if not already scheduled | ||
| if (rafIdRef.current === null) { | ||
| rafIdRef.current = requestAnimationFrame(() => { | ||
| rafIdRef.current = null | ||
| if (pendingClientXRef.current !== null) { | ||
| stableOnDrag.current(pendingClientXRef.current, false) | ||
| pendingClientXRef.current = null | ||
| } | ||
| }) | ||
| } | ||
| }, []) | ||
|
|
||
| /** | ||
| * Pointer up - cleanup is handled by onLostPointerCapture event | ||
| * which fires when pointer capture is released (including on pointerup) | ||
| */ | ||
| const handlePointerUp = React.useCallback((event: React.PointerEvent<HTMLDivElement>) => { | ||
| if (!isDraggingRef.current) return | ||
| event.preventDefault() | ||
| }, []) | ||
|
|
||
| /** | ||
| * Lost pointer capture ends a drag operation | ||
| * Cleans up dragging state and cancels any pending rAF | ||
| * Calls onDragEnd callback | ||
| */ | ||
| const handleLostPointerCapture = React.useCallback(() => { | ||
| if (!isDraggingRef.current) return | ||
| // Cancel any pending rAF to prevent stale updates | ||
| if (rafIdRef.current !== null) { | ||
| cancelAnimationFrame(rafIdRef.current) | ||
| rafIdRef.current = null | ||
| pendingClientXRef.current = null | ||
| } | ||
| endDragging() | ||
| stableOnDragEnd.current() | ||
| }, [endDragging]) | ||
|
|
||
| /** | ||
| * Keyboard handling for accessibility | ||
| * Arrow keys adjust the pane size in 3px increments | ||
| * Prevents default scrolling behavior | ||
| * Sets and clears dragging state via data attribute | ||
| * Calls onDrag | ||
| */ | ||
| const handleKeyDown = React.useCallback( | ||
| (event: React.KeyboardEvent<HTMLDivElement>) => { | ||
| if (!isArrowKey(event.key)) return | ||
| event.preventDefault() | ||
|
|
||
| // https://github.com/github/accessibility/issues/5101#issuecomment-1822870655 | ||
| const delta = isShrinkKey(event.key) ? -ARROW_KEY_STEP : ARROW_KEY_STEP | ||
|
|
||
| // Only set dragging on first keydown (not repeats) | ||
| if (!isDraggingRef.current) { | ||
| startDragging() | ||
| } | ||
| stableOnDrag.current(delta, true) | ||
| }, | ||
| [startDragging], | ||
| ) | ||
|
|
||
| const handleKeyUp = React.useCallback( | ||
| (event: React.KeyboardEvent<HTMLDivElement>) => { | ||
| if (!isArrowKey(event.key)) return | ||
| event.preventDefault() | ||
| endDragging() | ||
| stableOnDragEnd.current() | ||
| }, | ||
| [endDragging], | ||
| ) | ||
|
|
||
| // Cleanup rAF on unmount to prevent stale callbacks | ||
| React.useEffect(() => { | ||
| return () => { | ||
| if (rafIdRef.current !== null) { | ||
| cancelAnimationFrame(rafIdRef.current) | ||
| rafIdRef.current = null | ||
| } | ||
| } | ||
| }, []) | ||
|
|
||
| return ( | ||
| <div | ||
| ref={handleRef} | ||
| className={classes.DraggableHandle} | ||
| data-component={dataComponent} | ||
| role="slider" | ||
| aria-label={ariaLabelledBy ? undefined : (ariaLabel ?? 'Draggable pane splitter')} | ||
| aria-labelledby={ariaLabelledBy} | ||
| aria-valuemin={ariaValueMin} | ||
| aria-valuemax={ariaValueMax} | ||
| aria-valuenow={ariaValueNow} | ||
| aria-valuetext={ariaValueNow !== undefined ? formatValueText(ariaValueNow) : undefined} | ||
| tabIndex={0} | ||
|
TylerJDev marked this conversation as resolved.
|
||
| onPointerDown={handlePointerDown} | ||
| onPointerMove={handlePointerMove} | ||
| onPointerUp={handlePointerUp} | ||
| onLostPointerCapture={handleLostPointerCapture} | ||
| onKeyDown={handleKeyDown} | ||
| onKeyUp={handleKeyUp} | ||
| onDoubleClick={onDoubleClick} | ||
| /> | ||
| ) | ||
| }) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is different compared to how it was in
PageLayout. Before we directly usedpaneRefvia context, but now we passpaneRefvia this prop inPageLayout.This is to support usage outside of
PageLayout, where the target ref differs from the one within the PageLayout component.