feat: S2 DnD docs and followups#10082
Merged
Merged
Conversation
|
Build successful! 🎉 |
|
Build successful! 🎉 |
1 similar comment
|
Build successful! 🎉 |
done via isFocusVisibleWithinRow render prop support in TableCells and isFocusVisibleWithin render prop support in GridList row
LFDanLu
changed the title
|
Build successful! 🎉 |
LFDanLu
commented
May 20, 2026
Comment on lines
+451
to
+456
| /** | ||
| * Whether the item's children have keyboard focus. | ||
| * | ||
| * @selector [data-focus-visible-within] | ||
| */ | ||
| isFocusVisibleWithin: boolean; |
Member
Author
There was a problem hiding this comment.
this is really just parity with TableRow and TreeItemContent, and lets me get rid of a useFocusRing in S2 TreeView in favor of calling it here
LFDanLu
commented
May 20, 2026
Comment on lines
+2011
to
+2016
| /** | ||
| * Whether keyboard focus is visible anywhere within the parent row. | ||
| * | ||
| * @selector [data-focus-visible-within-row] | ||
| */ | ||
| isFocusVisibleWithinRow: boolean; |
Member
Author
There was a problem hiding this comment.
For discussion. As mentioned TableRow is a bit different from GridRow/TreeItemContent since it expects Cells as direct children and doesn't support a render prop render function. This means if we want the drag handle to be visible when focus is in the TableRow we can either do a css selector strat looking for the isFocusVisible data attribute or pass that state down to the Cells as well so the Cell render prop has this information to pass to its children
LFDanLu
commented
May 20, 2026
| } | ||
|
|
||
| &[data-layout='grid']:not(:has(.react-aria-GridListSection)) { | ||
| &[data-layout='grid']:not(:has(.react-aria-GridListSection)):not([data-empty]) { |
Member
Author
There was a problem hiding this comment.
display: grid was overriding the empty state styles and thus breaking the text centering for empty state
LFDanLu
commented
May 20, 2026
Member
Author
There was a problem hiding this comment.
changes in here actually bring us back to main's original state, my dnd PR has a left over change from when the drop insertion indicators were reserving a non 0 height in the layout
| Node | ||
| } from '@react-types/shared'; | ||
| import DragHandle from '../ui-icons/DragHandle'; | ||
| import {DragHandleButton} from './dnd-utils'; |
Member
There was a problem hiding this comment.
Suggested change
| import {DragHandleButton} from './dnd-utils'; | |
| import {DragHandleButton, InsertionIndicator} from './dnd-utils'; |
| import {GridNode} from 'react-stately/private/grid/GridCollection'; | ||
| import {IconContext} from './Icon'; | ||
| import {InsertionIndicator} from './ListView'; | ||
| import {InsertionIndicator} from './dnd-utils'; |
Member
There was a problem hiding this comment.
Suggested change
| import {InsertionIndicator} from './dnd-utils'; |
snowystinger
previously approved these changes
May 21, 2026
Member
snowystinger
left a comment
snowystinger
left a comment
There was a problem hiding this comment.
tested on chrome and ios, looks good
|
Build successful! 🎉 |
## API Changes
react-aria-components/react-aria-components:GridListItemRenderProps GridListItemRenderProps {
allowsDragging?: boolean
id?: Key
isDisabled: boolean
isDragging?: boolean
isDropTarget?: boolean
isFocusVisible: boolean
+ isFocusVisibleWithin: boolean
isFocused: boolean
isHovered: boolean
isPressed: boolean
isSelected: boolean
selectionMode: SelectionMode
state: ListState<unknown>
}/react-aria-components:CellRenderProps CellRenderProps {
columnIndex?: number | null
hasChildItems: boolean
id?: Key
isDisabled: boolean
isExpanded: boolean
isFocusVisible: boolean
+ isFocusVisibleWithinRow: boolean
isFocused: boolean
isHovered: boolean
isPressed: boolean
isSelected: boolean
level: number
} |
Agent Skills ChangesAdded (1)
Removed (116)
Modified (143)
InstallReact Spectrum S2: React Aria: |
yihuiliao
approved these changes
May 21, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Follow ups from testing and PR review:
isFocusVisibleWithinRowtoTableCellrenderPropsandisFocusVisibleWithinforGridRow)✅ Pull Request Checklist:
📝 Test Instructions:
Go through the S2 DnD docs and verify things look fine. Smoke test DnD insertion indicators styling/appearance/positioning and that drag handles are properly visually hidden still and other mentioned bugs in description. Test DnD component in test apps (see fece8b9#comments for links)
See chromatic: https://www.chromatic.com/build?appId=5f0dd5ad2b5fc10022a2e320&number=1198
🧢 Your Project:
RSP