Skip to content

fix: Fix keyboard DnD navigation in expandable rows RAC Table #9773

Open
LFDanLu wants to merge 5 commits intomainfrom
table_dnd_expandable
Open

fix: Fix keyboard DnD navigation in expandable rows RAC Table #9773
LFDanLu wants to merge 5 commits intomainfrom
table_dnd_expandable

Conversation

@LFDanLu
Copy link
Member

@LFDanLu LFDanLu commented Mar 10, 2026

Closes RSP Component Milestones (view)

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

  1. Go to the RAC Table DnD docs and expand all the rows.
  2. Start a keyboard drag operation on "Budget" and press ArrowDown twice. That should cause the drop indicator to become wider since you are changing the drop position to be the drop position after higher level parent rows.
  3. Press ArrowUp multiple times. You should see the drop indicator become smaller and then move visually after "Weekly Report"

Previously step 3 would skip directly to be after "Weekly Report"

🧢 Your Project:

RSP

LFDanLu added 2 commits March 10, 2026 14:02
this is specifcally handling the case where the user navigates upwards through nested row drop targets
@github-actions github-actions bot added the RAC label Mar 10, 2026
@LFDanLu LFDanLu changed the title Table dnd expandable fix: Fix keyboard DnD navigation in expandable rows RAC Table Mar 10, 2026
@rspbot
Copy link

rspbot commented Mar 10, 2026

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it works well.

Unrelated, but I keep getting the double focus ring with that example in particular. I wonder if it's related to fix useFocusRing perf ( and collection items not necessarily re-rendering. will have to look into that more

@LFDanLu
Copy link
Member Author

LFDanLu commented Mar 10, 2026

@snowystinger yeah, it actually seems to be the easiest way to reproduce the double focus rings that I've found now haha

getChildren(key: Key): Iterable<Node<Item>> {
let item = this.map.get(key);
return item?.childNodes ?? [];
return Array.from(item?.childNodes || [])?.filter(item => item.type !== 'item') ?? [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid double traversal, #9013 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol it's just a test, this is not important

@rspbot
Copy link

rspbot commented Mar 13, 2026

@rspbot
Copy link

rspbot commented Mar 13, 2026

@rspbot
Copy link

rspbot commented Mar 13, 2026

## API Changes

react-aria-components

/react-aria-components:GridList

 GridList <T extends {}> {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children?: ReactNode | ({}) => ReactNode
   className?: ClassNameOrFunction<GridListRenderProps> = 'react-aria-GridList'
   defaultSelectedKeys?: 'all' | Iterable<Key>
   dependencies?: ReadonlyArray<any>
   disabledBehavior?: DisabledBehavior = "all"
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
   disallowTypeAhead?: boolean = false
   dragAndDropHooks?: DragAndDropHooks<NoInfer<{}>>
   escapeKeyBehavior?: 'clearSelection' | 'none' = 'clearSelection'
   id?: string
   items?: Iterable<T>
   keyboardNavigationBehavior?: 'arrow' | 'tab' = 'arrow'
   layout?: 'stack' | 'grid' = 'stack'
   onAction?: (Key) => void
   onSelectionChange?: (Selection) => void
-  orientation?: 'horizontal' | 'vertical' = 'vertical'
   render?: DOMRenderFunction<keyof React.JSX.IntrinsicElements, GridListRenderProps>
   renderEmptyState?: (GridListRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionBehavior?: SelectionBehavior = "toggle"
   shouldSelectOnPressUp?: boolean
   slot?: string | null
   style?: StyleOrFunction<GridListRenderProps>
 }

/react-aria-components:GridListProps

 GridListProps <T> {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children?: ReactNode | (T) => ReactNode
   className?: ClassNameOrFunction<GridListRenderProps> = 'react-aria-GridList'
   defaultSelectedKeys?: 'all' | Iterable<Key>
   dependencies?: ReadonlyArray<any>
   disabledBehavior?: DisabledBehavior = "all"
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
   disallowTypeAhead?: boolean = false
   dragAndDropHooks?: DragAndDropHooks<NoInfer<T>>
   escapeKeyBehavior?: 'clearSelection' | 'none' = 'clearSelection'
   id?: string
   items?: Iterable<T>
   keyboardNavigationBehavior?: 'arrow' | 'tab' = 'arrow'
   layout?: 'stack' | 'grid' = 'stack'
   onAction?: (Key) => void
   onSelectionChange?: (Selection) => void
-  orientation?: 'horizontal' | 'vertical' = 'vertical'
   render?: DOMRenderFunction<keyof React.JSX.IntrinsicElements, GridListRenderProps>
   renderEmptyState?: (GridListRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionBehavior?: SelectionBehavior = "toggle"
   shouldSelectOnPressUp?: boolean
   slot?: string | null
   style?: StyleOrFunction<GridListRenderProps>
 }

/react-aria-components:TreeEmptyStateRenderProps

-TreeEmptyStateRenderProps {
-  allowsDragging: boolean
-  isFocusVisible: boolean
-  isFocused: boolean
-  selectionMode: SelectionMode
-  state: TreeState<unknown>
-}

@react-aria/utils

/@react-aria/utils:getNonce

-getNonce {
-  doc?: Document
-  returnVal: undefined
-}

@react-spectrum/s2

/@react-spectrum/s2:CardView

 CardView <T extends {}> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children?: ReactNode | (T) => ReactNode
   defaultSelectedKeys?: 'all' | Iterable<Key>
   density?: 'compact' | 'regular' | 'spacious' = 'regular'
   dependencies?: ReadonlyArray<any>
   disabledBehavior?: DisabledBehavior = "all"
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
   disallowTypeAhead?: boolean = false
   dragAndDropHooks?: DragAndDropHooks<NoInfer<T>>
   escapeKeyBehavior?: 'clearSelection' | 'none' = 'clearSelection'
   id?: string
   items?: Iterable<T>
   layout?: 'grid' | 'waterfall' = 'grid'
   loadingState?: LoadingState
   onAction?: (Key) => void
   onLoadMore?: () => void
   onSelectionChange?: (Selection) => void
-  orientation?: 'horizontal' | 'vertical' = 'vertical'
   renderActionBar?: ('all' | Set<Key>) => ReactElement
   renderEmptyState?: (GridListRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionMode?: SelectionMode
   shouldSelectOnPressUp?: boolean
   size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesPropWithHeight
   variant?: 'primary' | 'secondary' | 'tertiary' | 'quiet' = 'primary'
 }

/@react-spectrum/s2:ListView

 ListView <T extends {}> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children: ReactNode | ({}) => ReactNode
   defaultSelectedKeys?: 'all' | Iterable<Key>
   dependencies?: ReadonlyArray<any>
   disabledBehavior?: DisabledBehavior = "all"
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
   disallowTypeAhead?: boolean = false
   escapeKeyBehavior?: 'clearSelection' | 'none' = 'clearSelection'
   hideLinkOutIcon?: boolean
   id?: string
   isQuiet?: boolean
   items?: Iterable<T>
   loadingState?: LoadingState
   onAction?: (Key) => void
   onLoadMore?: () => void
   onSelectionChange?: (Selection) => void
-  orientation?: 'horizontal' | 'vertical' = 'vertical'
   overflowMode?: 'wrap' | 'truncate' = 'truncate'
   renderActionBar?: ('all' | Set<Key>) => ReactElement
   renderEmptyState?: (GridListRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionStyle?: 'highlight' | 'checkbox' = 'checkbox'
   shouldSelectOnPressUp?: boolean
   slot?: string | null
   styles?: StylesPropWithHeight
 }

/@react-spectrum/s2:CardViewProps

 CardViewProps <T> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children?: ReactNode | (T) => ReactNode
   defaultSelectedKeys?: 'all' | Iterable<Key>
   density?: 'compact' | 'regular' | 'spacious' = 'regular'
   dependencies?: ReadonlyArray<any>
   disabledBehavior?: DisabledBehavior = "all"
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
   disallowTypeAhead?: boolean = false
   dragAndDropHooks?: DragAndDropHooks<NoInfer<T>>
   escapeKeyBehavior?: 'clearSelection' | 'none' = 'clearSelection'
   id?: string
   items?: Iterable<T>
   layout?: 'grid' | 'waterfall' = 'grid'
   loadingState?: LoadingState
   onAction?: (Key) => void
   onLoadMore?: () => void
   onSelectionChange?: (Selection) => void
-  orientation?: 'horizontal' | 'vertical' = 'vertical'
   renderActionBar?: ('all' | Set<Key>) => ReactElement
   renderEmptyState?: (GridListRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionMode?: SelectionMode
   shouldSelectOnPressUp?: boolean
   size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesPropWithHeight
   variant?: 'primary' | 'secondary' | 'tertiary' | 'quiet' = 'primary'
 }

/@react-spectrum/s2:ListViewProps

 ListViewProps <T> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children: ReactNode | (T) => ReactNode
   defaultSelectedKeys?: 'all' | Iterable<Key>
   dependencies?: ReadonlyArray<any>
   disabledBehavior?: DisabledBehavior = "all"
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
   disallowTypeAhead?: boolean = false
   escapeKeyBehavior?: 'clearSelection' | 'none' = 'clearSelection'
   hideLinkOutIcon?: boolean
   id?: string
   isQuiet?: boolean
   items?: Iterable<T>
   loadingState?: LoadingState
   onAction?: (Key) => void
   onLoadMore?: () => void
   onSelectionChange?: (Selection) => void
-  orientation?: 'horizontal' | 'vertical' = 'vertical'
   overflowMode?: 'wrap' | 'truncate' = 'truncate'
   renderActionBar?: ('all' | Set<Key>) => ReactElement
   renderEmptyState?: (GridListRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionStyle?: 'highlight' | 'checkbox' = 'checkbox'
   shouldSelectOnPressUp?: boolean
   slot?: string | null
   styles?: StylesPropWithHeight
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants