Skip to content

Fixing focus ring and type ahead stories in ListView #2325

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.

Sign up for GitHub

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 8 commits into from
Sep 27, 2021
Merged

Fixing focus ring and type ahead stories in ListView #2325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1a5ccc7
fixing focus ring and type ahead stories
LFDanLu Sep 13, 2021
769b2e6
fixing lint
LFDanLu Sep 13, 2021
a0c1b90
Merge branch 'main' into fix_listview_focusring
LFDanLu Sep 15, 2021
1b5ba74
adding keyup to listview tests
LFDanLu Sep 16, 2021
6bbeb7f
Merge branch 'fix_listview_focusring' of github.com:adobe/react-spect…
LFDanLu Sep 16, 2021
b4b3164
Merge branch 'main' into fix_listview_focusring
dannify Sep 20, 2021
27b2726
Merge branch 'main' into fix_listview_focusring
LFDanLu Sep 27, 2021
f814ce9
Merge branch 'main' into fix_listview_focusring
LFDanLu Sep 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/@react-spectrum/list/src/ListView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ function ListView<T extends object>(props: ListViewProps<T>, ref: DOMRef<HTMLDiv
ref: domRef,
direction,
collator,
// Focus the ListView cell instead of the row so that focus doesn't change with left/right arrow keys when there aren't any
// focusable children in the cell.
focusMode: 'cell'
}), [state, domRef, direction, collator]);
let {gridProps} = useGrid({
Expand Down
6 changes: 3 additions & 3 deletions packages/@react-spectrum/list/src/ListViewItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export function ListViewItem(props) {
focusMode: 'cell'
}, state, cellRef);
const mergedProps = mergeProps(
rowProps,
gridCellProps,
hoverProps,
focusWithinProps,
focusProps
Expand All @@ -74,7 +74,7 @@ export function ListViewItem(props) {
let showCheckbox = state.selectionManager.selectionMode !== 'none';
return (
<div
{...mergedProps}
{...rowProps}
ref={rowRef}>
<div
className={
Expand All @@ -89,7 +89,7 @@ export function ListViewItem(props) {
)
}
ref={cellRef}
{...gridCellProps}>
{...mergedProps}>
<Grid UNSAFE_className={listStyles['react-spectrum-ListViewItem-grid']}>
{showCheckbox && (
<Checkbox
Expand Down
52 changes: 26 additions & 26 deletions packages/@react-spectrum/list/stories/ListView.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,29 @@ function renderEmptyState() {
storiesOf('ListView', module)
.add('default', () => (
<ListView width="250px">
<Item textValue="row1">row 1</Item>
<Item textValue="row2">row 2</Item>
<Item textValue="row3">row 3</Item>
<Item textValue="row 1">row 1</Item>
<Item textValue="row 2">row 2</Item>
<Item textValue="row 3">row 3</Item>
</ListView>
))
.add('isQuiet', () => (
<ListView width="250px" isQuiet>
<Item textValue="row1">row 1</Item>
<Item textValue="row2">row 2</Item>
<Item textValue="row3">row 3</Item>
<Item textValue="row 1">row 1</Item>
<Item textValue="row 2">row 2</Item>
<Item textValue="row 3">row 3</Item>
</ListView>
))
.add('with buttons', () => (
<ListView width="300px">
<Item textValue="one">
<Item textValue="row 1">
<Content>row 1</Content>
<ActionButton>Button 1</ActionButton>
</Item>
<Item textValue="two">
<Item textValue="row 2">
<Content>row 2</Content>
<ActionButton>Button 1</ActionButton>
</Item>
<Item textValue="three">
<Item textValue="row 3">
<Content>row 3</Content>
<ActionButton>Button 1</ActionButton>
</Item>
Expand All @@ -82,7 +82,7 @@ storiesOf('ListView', module)
return (
<ListView items={items} width="300px" height="250px">
{(item) => (
<Item key={item.key}>
<Item key={item.key} textValue={`Item ${item.key}`}>
<Content>
<Flex alignItems="center" gap="10px">
<View flexGrow={1}>Item {item.key}</View> {/* TODO */}
Expand Down Expand Up @@ -119,16 +119,16 @@ storiesOf('ListView', module)
))
.add('density: compact', () => (
<ListView width="250px" density="compact">
<Item textValue="row1">row 1</Item>
<Item textValue="row2">row 2</Item>
<Item textValue="row3">row 3</Item>
<Item textValue="row 1">row 1</Item>
<Item textValue="row 2">row 2</Item>
<Item textValue="row 3">row 3</Item>
</ListView>
))
.add('density: spacious', () => (
<ListView width="250px" density="spacious">
<Item textValue="row1">row 1</Item>
<Item textValue="row2">row 2</Item>
<Item textValue="row3">row 3</Item>
<Item textValue="row 1">row 1</Item>
<Item textValue="row 2">row 2</Item>
<Item textValue="row 3">row 3</Item>
</ListView>
))
.add('selection: none', () => (
Expand Down Expand Up @@ -182,13 +182,13 @@ function Example(props?) {
<Item key="folder1" hasChildItems>
<Content>folder 1</Content>
</Item>
<Item key="row1" textValue="row1">
<Item key="row1" textValue="row 1">
<Content>row 1</Content>
</Item>
<Item key="row2" textValue="row2">
<Item key="row2" textValue="row 2">
<Content>row 2</Content>
</Item>
<Item key="row3" textValue="row3">
<Item key="row3" textValue="row 3">
<Content>row 3</Content>
</Item>
</ListView>
Expand All @@ -201,13 +201,13 @@ function Example2(props?) {
<Item key="folder1" hasChildItems>
<Link>folder 1</Link>
</Item>
<Item textValue="row1">
<Item textValue="row 1">
<Content>row 1</Content>
</Item>
<Item textValue="row2">
<Item textValue="row 2">
<Content>row 2</Content>
</Item>
<Item textValue="row3">
<Item textValue="row 3">
<Content>row 3</Content>
</Item>
</ListView>
Expand All @@ -217,23 +217,23 @@ function Example2(props?) {
function renderActionsExample(renderActions, props?) {
return (
<ListView width="300px" selectionMode="single" {...props} onSelectionChange={keys => console.log('sel', keys)}>
<Item key="a" textValue="row1" hasChildItems>
<Item key="a" textValue="folder 1" hasChildItems>
<Folder />
<Link>folder 1</Link>
<Text slot="description">description for folder 1</Text>
{renderActions({onPress: action('actionPress')})}
</Item>
<Item key="b" textValue="row2">
<Item key="b" textValue="row 1">
<Text>row 1</Text>
<Text slot="description">description for row 1</Text>
{renderActions({onPress: action('actionPress')})}
</Item>
<Item key="c" textValue="row3">
<Item key="c" textValue="row 2">
<Text>row 2</Text>
<Text slot="description">description for row 2</Text>
{renderActions({onPress: action('actionPress')})}
</Item>
<Item key="d" textValue="row4">
<Item key="d" textValue="row 3">
<Text>row 3</Text>
<Text slot="description">description for row 3</Text>
{renderActions({onPress: action('actionPress')})}
Expand Down
18 changes: 18 additions & 0 deletions packages/@react-spectrum/list/test/ListView.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,22 @@ describe('ListView', function () {

let moveFocus = (key, opts = {}) => {fireEvent.keyDown(document.activeElement, {key, ...opts});};

describe('Type to select', function () {
it('focuses the correct cell when typing', function () {
let tree = renderList();
let target = getCell(tree, 'Baz');
let grid = tree.getByRole('grid');
act(() => grid.focus());
fireEvent.keyDown(grid, {key: 'B'});
Copy link
Member

@snowystinger snowystinger Sep 16, 2021

Choose a reason for hiding this comment

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

please include the keyUp as well

fireEvent.keyUp(grid, {key: 'Enter'});
fireEvent.keyDown(grid, {key: 'A'});
fireEvent.keyUp(grid, {key: 'A'});
fireEvent.keyDown(grid, {key: 'Z'});
fireEvent.keyUp(grid, {key: 'Z'});
expect(document.activeElement).toBe(target);
});
});

describe('ArrowRight', function () {
it('should not move focus if no focusables present', function () {
let tree = renderList();
Expand Down Expand Up @@ -300,6 +316,7 @@ describe('ListView', function () {
let row = tree.getAllByRole('row')[1];
expect(row).toHaveAttribute('aria-selected', 'false');
fireEvent.keyDown(row, {key: ' '});
fireEvent.keyUp(row, {key: ' '});

checkSelection(onSelectionChange, ['bar']);
expect(row).toHaveAttribute('aria-selected', 'true');
Expand All @@ -312,6 +329,7 @@ describe('ListView', function () {
let row = tree.getAllByRole('row')[1];
expect(row).toHaveAttribute('aria-selected', 'false');
fireEvent.keyDown(row, {key: 'Enter'});
fireEvent.keyUp(row, {key: 'Enter'});

checkSelection(onSelectionChange, ['bar']);
expect(row).toHaveAttribute('aria-selected', 'true');
Expand Down