Skip to content

Commit a32c05e

Browse files
TableView and ListView skip focusing when empty, but have a focusable child (#3581)
* TableView doesn't get focus when empty, but there is a link * Refactor plus getting to work with ListView and Virtualizer components * some small improvments and lint type fixes * making lint and test pass for a WIP build * reverting skipped tests * refactored useTabbableChildre to not use a MutationObserver * Factoring in empty collection to new Virtualizer tabIndex logic * improving fragile test and making sure a hook is always called * moving hook to fix circular dependancy and adding listview test * refactored new hook to use hooks useState and useEffect * refactoring hook to return boolean for tabbable child existence instead of setting tabIndex * removing code that didn't fix a test * fixing lint warning useEffect dependencies Co-authored-by: Robert Snow <[email protected]>
1 parent fc97a5d commit a32c05e

File tree

6 files changed

+68
-10
lines changed

6 files changed

+68
-10
lines changed

packages/@react-aria/virtualizer/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
},
1919
"dependencies": {
2020
"@babel/runtime": "^7.6.2",
21+
"@react-aria/focus": "^3.9.0",
2122
"@react-aria/i18n": "^3.6.1",
2223
"@react-aria/interactions": "^3.12.0",
2324
"@react-aria/utils": "^3.14.0",

packages/@react-aria/virtualizer/src/Virtualizer.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {getInteractionModality} from '@react-aria/interactions';
1616
import {Layout, Rect, ReusableView, useVirtualizerState, VirtualizerState} from '@react-stately/virtualizer';
1717
import React, {FocusEvent, HTMLAttributes, Key, ReactElement, RefObject, useCallback, useEffect, useMemo, useRef} from 'react';
1818
import {ScrollView} from './ScrollView';
19+
import {useHasTabbableChild} from './useHasTabbableChild';
1920
import {VirtualizerItem} from './VirtualizerItem';
2021

2122
interface VirtualizerProps<T extends object, V> extends HTMLAttributes<HTMLElement> {
@@ -178,13 +179,21 @@ export function useVirtualizer<T extends object, V, W>(props: VirtualizerOptions
178179
}
179180
});
180181

182+
let hasTabbableChild = useHasTabbableChild({
183+
isEmpty: virtualizer.collection.size === 0,
184+
hasRenderedAnything: virtualizer.contentSize.height > 0 || virtualizer.contentSize.width > 0
185+
}, ref);
186+
181187
// Set tabIndex to -1 if the focused view is in the DOM, otherwise 0 so that the collection
182188
// itself is tabbable. When the collection receives focus, we scroll the focused item back into
183189
// view, which will allow it to be properly focused. If using virtual focus, don't set a
184190
// tabIndex at all so that VoiceOver on iOS 14 doesn't try to move real DOM focus to the element anyway.
185191
let tabIndex: number;
186192
if (!shouldUseVirtualFocus) {
187-
tabIndex = focusedView ? -1 : 0;
193+
// When there is no focusedView the default tabIndex is 0. We include logic for empty collections too.
194+
// For collections that are empty, but have a link in the empty children we want to skip focusing this
195+
// and let focus move to the link similar to link moving to children.
196+
tabIndex = focusedView || hasTabbableChild ? -1 : 0;
188197
}
189198

190199
return {
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright 2022 Adobe. All rights reserved.
3+
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License. You may obtain a copy
5+
* of the License at http://www.apache.org/licenses/LICENSE-2.0
6+
*
7+
* Unless required by applicable law or agreed to in writing, software distributed under
8+
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
9+
* OF ANY KIND, either express or implied. See the License for the specific language
10+
* governing permissions and limitations under the License.
11+
*/
12+
13+
import {getFocusableTreeWalker} from '@react-aria/focus';
14+
import {RefObject, useEffect, useState} from 'react';
15+
16+
interface AriaHasTabbableChildProps {
17+
isEmpty: boolean,
18+
hasRenderedAnything: boolean
19+
}
20+
21+
// This was created for a special empty case of a component that can have child or
22+
// be empty, like Collection/Virtualizer/Table/ListView/etc. When these components
23+
// are empty they can have a message with a tabbable element, which is like them
24+
// being not empty, when it comes to focus and tab order.
25+
//
26+
// This looks at the element's children and determines if any are tabbable.
27+
export function useHasTabbableChild({isEmpty, hasRenderedAnything}: AriaHasTabbableChildProps, ref: RefObject<HTMLElement>): boolean {
28+
let [hasTabbableChild, setHasTabbableChild] = useState(false);
29+
30+
useEffect(() => {
31+
if (ref?.current && isEmpty && hasRenderedAnything) {
32+
// Detect if there are any tabbable elements and update the tabIndex accordingly.
33+
let walker = getFocusableTreeWalker(ref.current, {tabbable: true});
34+
setHasTabbableChild(!!walker.nextNode());
35+
}
36+
}, [ref, isEmpty, hasRenderedAnything]);
37+
38+
return hasTabbableChild;
39+
}

packages/@react-spectrum/list/test/ListView.test.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,13 @@ describe('ListView', function () {
552552
expect(progressbar).toBeTruthy();
553553
});
554554

555+
it('should allow you to tab to ListView body if loading (no tabbable children)', function () {
556+
let {getByRole} = render(<ListView aria-label="List" loadingState="loading">{[]}</ListView>);
557+
let grid = getByRole('grid');
558+
userEvent.tab();
559+
expect(document.activeElement).toBe(grid);
560+
});
561+
555562
it('should display loading affordance with proper height (isLoadingMore)', function () {
556563
let items = [
557564
{key: 'foo', label: 'Foo'},
@@ -575,12 +582,18 @@ describe('ListView', function () {
575582
expect(getByText('No results')).toBeTruthy();
576583
});
577584

578-
it('should allow you to tab into ListView body if empty', function () {
579-
let {getByRole} = render(<ListView aria-label="List" renderEmptyState={renderEmptyState} />);
580-
let grid = getByRole('grid');
585+
it('should allow you to tab into ListView body if empty with link', function () {
586+
let {getByRole} = render(
587+
<>
588+
<ActionButton>Toggle</ActionButton>
589+
<ListView aria-label="List" renderEmptyState={renderEmptyState}>{[]}</ListView>
590+
</>
591+
);
592+
let toggleButton = getByRole('button');
581593
let link = getByRole('link');
594+
582595
userEvent.tab();
583-
expect(document.activeElement).toBe(grid);
596+
expect(document.activeElement).toBe(toggleButton);
584597
userEvent.tab();
585598
expect(document.activeElement).toBe(link);
586599
});

packages/@react-spectrum/table/src/TableView.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,8 +468,7 @@ function TableVirtualizer({layout, collection, lastResizeInteractionModality, fo
468468
return (
469469
<FocusScope>
470470
<div
471-
// Override virtualizer provided tabindex if TableView is empty, so it is tabbable.
472-
{...mergeProps(otherProps, virtualizerProps, collection.size === 0 && {tabIndex: 0})}
471+
{...mergeProps(otherProps, virtualizerProps)}
473472
ref={domRef}>
474473
<div
475474
role="presentation"

packages/@react-spectrum/table/test/Table.test.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4226,15 +4226,12 @@ describe('TableView', function () {
42264226

42274227
it('should allow the user to tab into the table body', function () {
42284228
let tree = render(<EmptyStateTable />);
4229-
let table = tree.getByRole('grid');
42304229
let toggleButton = tree.getAllByRole('button')[0];
42314230
let link = tree.getByRole('link');
42324231

42334232
userEvent.tab();
42344233
expect(document.activeElement).toBe(toggleButton);
42354234
userEvent.tab();
4236-
expect(document.activeElement).toBe(table);
4237-
userEvent.tab();
42384235
expect(document.activeElement).toBe(link);
42394236
});
42404237

0 commit comments

Comments
 (0)