From 2b05b6d984e1b37d4ac76f0bb305433d8f57d988 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 14 Dec 2023 12:52:49 -0800 Subject: [PATCH] Fix hover in RAC ListBox and remove onCellAction (#5580) * Making focus and hover separate in ListBox and getting rid of onCellAction onCellAction isnt ready for GA * fix lint --- packages/react-aria-components/example/index.css | 5 +++++ packages/react-aria-components/src/ListBox.tsx | 10 ++-------- packages/react-aria-components/src/Table.tsx | 2 -- .../react-aria-components/stories/index.stories.tsx | 8 +++++++- packages/react-aria-components/stories/utils.tsx | 5 +++-- packages/react-aria-components/test/ListBox.test.js | 3 ++- scripts/extractStarter.mjs | 1 - 7 files changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/react-aria-components/example/index.css b/packages/react-aria-components/example/index.css index 2e354f22100..c6342118909 100644 --- a/packages/react-aria-components/example/index.css +++ b/packages/react-aria-components/example/index.css @@ -48,6 +48,11 @@ html { color: white; } +.item.item.hovered { + background: lightsalmon; + color: white; +} + .item.selected { background: purple; color: white; diff --git a/packages/react-aria-components/src/ListBox.tsx b/packages/react-aria-components/src/ListBox.tsx index 51386c88d56..c5f968fac15 100644 --- a/packages/react-aria-components/src/ListBox.tsx +++ b/packages/react-aria-components/src/ListBox.tsx @@ -12,7 +12,7 @@ import {AriaListBoxOptions, AriaListBoxProps, DraggableItemResult, DragPreviewRenderer, DroppableCollectionResult, DroppableItemResult, FocusScope, ListKeyboardDelegate, mergeProps, useCollator, useFocusRing, useHover, useListBox, useListBoxSection, useLocale, useOption} from 'react-aria'; import {CollectionDocumentContext, CollectionPortal, CollectionProps, ItemRenderProps, useCachedChildren, useCollection, useSSRCollectionNode} from './Collection'; -import {ContextValue, forwardRefType, HiddenContext, Provider, RenderProps, ScrollableProps, SlotProps, StyleProps, StyleRenderProps, useContextProps, useRenderProps, useSlot, useSlottedContext} from './utils'; +import {ContextValue, forwardRefType, HiddenContext, Provider, RenderProps, ScrollableProps, SlotProps, StyleProps, StyleRenderProps, useContextProps, useRenderProps, useSlot} from './utils'; import {DragAndDropContext, DragAndDropHooks, DropIndicator, DropIndicatorContext, DropIndicatorProps} from './useDragAndDrop'; import {DraggableCollectionState, DroppableCollectionState, ListState, Node, Orientation, SelectionBehavior, useListState} from 'react-stately'; import {filterDOMProps, mergeRefs, useObjectRef} from '@react-aria/utils'; @@ -360,7 +360,6 @@ interface OptionProps { function Option({item}: OptionProps) { let ref = useObjectRef(item.props.ref); let state = useContext(ListStateContext)!; - let {shouldFocusOnHover} = useSlottedContext(ListBoxContext)! as AriaListBoxOptions; let {dragAndDropHooks, dragState, dropState} = useContext(DragAndDropContext)!; let {optionProps, labelProps, descriptionProps, ...states} = useOption( {key: item.key}, @@ -369,14 +368,9 @@ function Option({item}: OptionProps) { ); let {hoverProps, isHovered} = useHover({ - isDisabled: shouldFocusOnHover || (!states.allowsSelection && !states.hasAction) + isDisabled: !states.allowsSelection && !states.hasAction }); - if (shouldFocusOnHover) { - hoverProps = {}; - isHovered = states.isFocused; - } - let draggableItem: DraggableItemResult | null = null; if (dragState && dragAndDropHooks) { draggableItem = dragAndDropHooks.useDraggableItem!({key: item.key}, dragState); diff --git a/packages/react-aria-components/src/Table.tsx b/packages/react-aria-components/src/Table.tsx index dd7c6050fbd..f948858f9e1 100644 --- a/packages/react-aria-components/src/Table.tsx +++ b/packages/react-aria-components/src/Table.tsx @@ -288,8 +288,6 @@ export interface TableProps extends Omit, 'children'>, Sty disabledBehavior?: DisabledBehavior, /** Handler that is called when a user performs an action on the row. */ onRowAction?: (key: Key) => void, - /** Handler that is called when a user performs an action on the cell. */ - onCellAction?: (key: Key) => void, /** The drag and drop hooks returned by `useDragAndDrop` used to enable drag and drop behavior for the Table. */ dragAndDropHooks?: DragAndDropHooks } diff --git a/packages/react-aria-components/stories/index.stories.tsx b/packages/react-aria-components/stories/index.stories.tsx index 05a4cd7fc68..957716ab22f 100644 --- a/packages/react-aria-components/stories/index.stories.tsx +++ b/packages/react-aria-components/stories/index.stories.tsx @@ -37,7 +37,8 @@ export const ListBoxExample = (args) => ( ListBoxExample.story = { args: { selectionMode: 'none', - selectionBehavior: 'toggle' + selectionBehavior: 'toggle', + shouldFocusOnHover: false }, argTypes: { selectionMode: { @@ -52,6 +53,11 @@ ListBoxExample.story = { options: ['toggle', 'replace'] } } + }, + parameters: { + description: { + data: 'Hover styles should have higher specificity than focus style for testing purposes. Hover style should not be applied on keyboard focus even if shouldFocusOnHover is true' + } } }; diff --git a/packages/react-aria-components/stories/utils.tsx b/packages/react-aria-components/stories/utils.tsx index da1d15d4697..ac286158f29 100644 --- a/packages/react-aria-components/stories/utils.tsx +++ b/packages/react-aria-components/stories/utils.tsx @@ -7,9 +7,10 @@ export const MyListBoxItem = (props: ListBoxItemProps) => { return ( classNames(styles, 'item', { + className={({isFocused, isSelected, isHovered}) => classNames(styles, 'item', { focused: isFocused, - selected: isSelected + selected: isSelected, + hovered: isHovered })} /> ); }; diff --git a/packages/react-aria-components/test/ListBox.test.js b/packages/react-aria-components/test/ListBox.test.js index f38d88bca6f..da97eda83e7 100644 --- a/packages/react-aria-components/test/ListBox.test.js +++ b/packages/react-aria-components/test/ListBox.test.js @@ -308,7 +308,7 @@ describe('ListBox', () => { }); it('should support focus ring', async () => { - let {getAllByRole} = renderListbox({selectionMode: 'multiple'}, {className: ({isFocusVisible}) => isFocusVisible ? 'focus' : ''}); + let {getAllByRole} = renderListbox({selectionMode: 'multiple', shouldFocusOnHover: true}, {className: ({isFocusVisible}) => isFocusVisible ? 'focus' : ''}); let option = getAllByRole('option')[0]; expect(option).not.toHaveAttribute('data-focus-visible'); @@ -318,6 +318,7 @@ describe('ListBox', () => { expect(document.activeElement).toBe(option); expect(option).toHaveAttribute('data-focus-visible', 'true'); expect(option).toHaveClass('focus'); + expect(option).not.toHaveAttribute('data-hovered'); fireEvent.keyDown(option, {key: 'ArrowDown'}); fireEvent.keyUp(option, {key: 'ArrowDown'}); diff --git a/scripts/extractStarter.mjs b/scripts/extractStarter.mjs index df7a5bc9c8b..09026e82140 100644 --- a/scripts/extractStarter.mjs +++ b/scripts/extractStarter.mjs @@ -324,7 +324,6 @@ export const Example = () => ( args.set('onAction', t.nullLiteral()); } else if (name === 'Table') { args.set('onRowAction', t.nullLiteral()); - args.set('onCellAction', t.nullLiteral()); } traverse.default(ast, {