Skip to content

Commit

Permalink
fix(#4108): Picker: isLoading state should include "Loading..." in th…
Browse files Browse the repository at this point in the history
…e element description (#4109)
  • Loading branch information
majornista authored May 26, 2023
1 parent 5c64bcb commit 4348bb8
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
11 changes: 7 additions & 4 deletions packages/@react-spectrum/picker/src/Picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {HiddenSelect, useSelect} from '@react-aria/select';
// @ts-ignore
import intlMessages from '../intl/*.json';
import {ListBoxBase, useListBoxLayout} from '@react-spectrum/listbox';
import {mergeProps, useLayoutEffect, useResizeObserver} from '@react-aria/utils';
import {mergeProps, useId, useLayoutEffect, useResizeObserver} from '@react-aria/utils';
import {Popover, Tray} from '@react-spectrum/overlays';
import {PressResponder, useHover} from '@react-aria/interactions';
import {ProgressCircle} from '@react-spectrum/progress';
Expand Down Expand Up @@ -68,21 +68,23 @@ function Picker<T extends object>(props: SpectrumPickerProps<T>, ref: DOMRef<HTM
let unwrappedTriggerRef = useUnwrapDOMRef(triggerRef);
let listboxRef = useRef();

let isLoadingInitial = props.isLoading && state.collection.size === 0;
let isLoadingMore = props.isLoading && state.collection.size > 0;
let progressCircleId = useId();

// We create the listbox layout in Picker and pass it to ListBoxBase below
// so that the layout information can be cached even while the listbox is not mounted.
// We also use the layout as the keyboard delegate for type to select.
let layout = useListBoxLayout(state);
let {labelProps, triggerProps, valueProps, menuProps, descriptionProps, errorMessageProps} = useSelect({
...props,
'aria-describedby': (isLoadingInitial ? progressCircleId : undefined),
keyboardDelegate: layout
}, state, unwrappedTriggerRef);

let isMobile = useIsMobileDevice();
let {hoverProps, isHovered} = useHover({isDisabled});

let isLoadingInitial = props.isLoading && state.collection.size === 0;
let isLoadingMore = props.isLoading && state.collection.size > 0;

// On small screen devices, the listbox is rendered in a tray, otherwise a popover.
let listbox = (
<ListBoxBase
Expand Down Expand Up @@ -205,6 +207,7 @@ function Picker<T extends object>(props: SpectrumPickerProps<T>, ref: DOMRef<HTM
</SlotProvider>
{isLoadingInitial &&
<ProgressCircle
id={progressCircleId}
isIndeterminate
size="S"
aria-label={stringFormatter.format('loading')}
Expand Down
27 changes: 26 additions & 1 deletion packages/@react-spectrum/picker/test/Picker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1831,7 +1831,7 @@ describe('Picker', function () {

describe('async loading', function () {
it('should display a spinner while loading', function () {
let {getByRole, rerender} = render(
let {getByRole, getByText, rerender} = render(
<Provider theme={theme}>
<Picker label="Test" items={[]} isLoading>
{item => <Item>{item.name}</Item>}
Expand All @@ -1843,6 +1843,7 @@ describe('Picker', function () {
let progressbar = within(picker).getByRole('progressbar');
expect(progressbar).toHaveAttribute('aria-label', 'Loading…');
expect(progressbar).not.toHaveAttribute('aria-valuenow');
expect(picker).toHaveAttribute('aria-describedby', `${progressbar.id}`);

rerender(
<Provider theme={theme}>
Expand All @@ -1853,6 +1854,30 @@ describe('Picker', function () {
);

expect(progressbar).not.toBeInTheDocument();
expect(picker).not.toHaveAttribute('aria-describedby');

rerender(
<Provider theme={theme}>
<Picker label="Test" description="Please select an item." items={[]} isLoading>
{item => <Item>{item.name}</Item>}
</Picker>
</Provider>
);

let description = getByText('Please select an item.');
expect(description).toHaveAttribute('id');
expect(picker).toHaveAttribute('aria-describedby', `${description.id} ${progressbar.id}`);

rerender(
<Provider theme={theme}>
<Picker label="Test" description="Please select an item." items={[]}>
{item => <Item>{item.name}</Item>}
</Picker>
</Provider>
);

expect(progressbar).not.toBeInTheDocument();
expect(picker).toHaveAttribute('aria-describedby', `${description.id}`);
});

it('should display a spinner inside the listbox when loading more', function () {
Expand Down

0 comments on commit 4348bb8

Please sign in to comment.