Skip to content

Commit

Permalink
Fix hover in RAC ListBox and remove onCellAction (#5580)
Browse files Browse the repository at this point in the history
* Making focus and hover separate in ListBox and getting rid of onCellAction

onCellAction isnt ready for GA

* fix lint
  • Loading branch information
LFDanLu authored Dec 14, 2023
1 parent 99e6100 commit 2b05b6d
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 15 deletions.
5 changes: 5 additions & 0 deletions packages/react-aria-components/example/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ html {
color: white;
}

.item.item.hovered {
background: lightsalmon;
color: white;
}

.item.selected {
background: purple;
color: white;
Expand Down
10 changes: 2 additions & 8 deletions packages/react-aria-components/src/ListBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -360,7 +360,6 @@ interface OptionProps<T> {
function Option<T>({item}: OptionProps<T>) {
let ref = useObjectRef<any>(item.props.ref);
let state = useContext(ListStateContext)!;
let {shouldFocusOnHover} = useSlottedContext(ListBoxContext)! as AriaListBoxOptions<T>;
let {dragAndDropHooks, dragState, dropState} = useContext(DragAndDropContext)!;
let {optionProps, labelProps, descriptionProps, ...states} = useOption(
{key: item.key},
Expand All @@ -369,14 +368,9 @@ function Option<T>({item}: OptionProps<T>) {
);

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);
Expand Down
2 changes: 0 additions & 2 deletions packages/react-aria-components/src/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,6 @@ export interface TableProps extends Omit<SharedTableProps<any>, '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
}
Expand Down
8 changes: 7 additions & 1 deletion packages/react-aria-components/stories/index.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ export const ListBoxExample = (args) => (
ListBoxExample.story = {
args: {
selectionMode: 'none',
selectionBehavior: 'toggle'
selectionBehavior: 'toggle',
shouldFocusOnHover: false
},
argTypes: {
selectionMode: {
Expand All @@ -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'
}
}
};

Expand Down
5 changes: 3 additions & 2 deletions packages/react-aria-components/stories/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ export const MyListBoxItem = (props: ListBoxItemProps) => {
return (
<ListBoxItem
{...props}
className={({isFocused, isSelected}) => classNames(styles, 'item', {
className={({isFocused, isSelected, isHovered}) => classNames(styles, 'item', {
focused: isFocused,
selected: isSelected
selected: isSelected,
hovered: isHovered
})} />
);
};
3 changes: 2 additions & 1 deletion packages/react-aria-components/test/ListBox.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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'});
Expand Down
1 change: 0 additions & 1 deletion scripts/extractStarter.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down

1 comment on commit 2b05b6d

@rspbot
Copy link

@rspbot rspbot commented on 2b05b6d Dec 14, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.