Skip to content

Commit 50b1e19

Browse files
jguddassnowystingerLFDanLu
authored
fix: useSelect trigger label (#4949)
* fix: useSelect trigger label * fix lint and tests * fix: fixed label merge logic * fix tests --------- Co-authored-by: Robert Snow <rsnow@adobe.com> Co-authored-by: Daniel Lu <dl1644@gmail.com>
1 parent cea5511 commit 50b1e19

File tree

5 files changed

+19
-18
lines changed

5 files changed

+19
-18
lines changed

‎packages/@react-aria/select/src/useSelect.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,12 @@ export function useSelect<T>(props: AriaSelectOptions<T>, state: SelectState<T>,
140140
isDisabled,
141141
onKeyDown: chain(triggerProps.onKeyDown, onKeyDown, props.onKeyDown),
142142
onKeyUp: props.onKeyUp,
143+
'aria-label': `, ${props['aria-label'] ?? ''}`,
143144
'aria-labelledby': [
144145
valueId,
145-
triggerProps['aria-labelledby'],
146-
triggerProps['aria-label'] && !triggerProps['aria-labelledby'] ? triggerProps.id : null
146+
!(props['aria-labelledby'] && props['aria-label']) && triggerProps.id,
147+
!(props['aria-labelledby'] || props['aria-label']) && labelProps.id,
148+
props['aria-labelledby']
147149
].filter(Boolean).join(' '),
148150
onFocus(e: FocusEvent) {
149151
if (state.isFocused) {

‎packages/@react-spectrum/picker/test/Picker.test.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,7 @@ describe('Picker', function () {
808808
let value = getByText('Select an option…');
809809
expect(label).toHaveAttribute('id');
810810
expect(value).toHaveAttribute('id');
811-
expect(picker).toHaveAttribute('aria-labelledby', `${value.id} ${label.id}`);
811+
expect(picker).toHaveAttribute('aria-labelledby', `${value.id} ${picker.id} ${label.id}`);
812812

813813
triggerPress(picker);
814814
act(() => jest.runAllTimers());
@@ -833,7 +833,7 @@ describe('Picker', function () {
833833
let value = getByText('Select an option…');
834834
expect(picker).toHaveAttribute('id');
835835
expect(value).toHaveAttribute('id');
836-
expect(picker).toHaveAttribute('aria-label', 'Test');
836+
expect(picker).toHaveAttribute('aria-label', ', Test');
837837
expect(picker).toHaveAttribute('aria-labelledby', `${value.id} ${picker.id}`);
838838

839839
triggerPress(picker);
@@ -859,7 +859,7 @@ describe('Picker', function () {
859859
let value = getByText('Select an option…');
860860
expect(picker).toHaveAttribute('id');
861861
expect(value).toHaveAttribute('id');
862-
expect(picker).toHaveAttribute('aria-labelledby', `${value.id} foo`);
862+
expect(picker).toHaveAttribute('aria-labelledby', `${value.id} ${picker.id} foo`);
863863

864864
triggerPress(picker);
865865
act(() => jest.runAllTimers());
@@ -884,8 +884,8 @@ describe('Picker', function () {
884884
let value = getByText('Select an option…');
885885
expect(picker).toHaveAttribute('id');
886886
expect(value).toHaveAttribute('id');
887-
expect(picker).toHaveAttribute('aria-label', 'Test');
888-
expect(picker).toHaveAttribute('aria-labelledby', `${value.id} ${picker.id} foo`);
887+
expect(picker).toHaveAttribute('aria-label', ', Test');
888+
expect(picker).toHaveAttribute('aria-labelledby', `${value.id} foo`);
889889

890890
triggerPress(picker);
891891
act(() => jest.runAllTimers());
@@ -918,7 +918,7 @@ describe('Picker', function () {
918918
let value = getByText('Select an option…');
919919
expect(label).toHaveAttribute('id');
920920
expect(value).toHaveAttribute('id');
921-
expect(picker).toHaveAttribute('aria-labelledby', `${value.id} ${label.id}`);
921+
expect(picker).toHaveAttribute('aria-labelledby', `${value.id} ${picker.id} ${label.id}`);
922922

923923
triggerPress(picker);
924924
act(() => jest.runAllTimers());

‎packages/@react-spectrum/searchwithin/test/SearchWithin.test.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ describe('SearchWithin labeling', function () {
187187

188188
expect(group).not.toHaveAttribute('aria-labelledby');
189189
expect(searchfield).toHaveAttribute('aria-labelledby', `${hiddenLabel.id} ${picker.id}`);
190-
expect(picker).toHaveAttribute('aria-labelledby', `${picker.childNodes[0].id} ${hiddenLabel.id}`);
190+
expect(picker).toHaveAttribute('aria-labelledby', `${picker.childNodes[0].id} ${picker.id} ${hiddenLabel.id}`);
191191
});
192192

193193
it('label = foo', function () {
@@ -203,7 +203,7 @@ describe('SearchWithin labeling', function () {
203203

204204
expect(group).toHaveAttribute('aria-labelledby', label.id);
205205
expect(searchfield).toHaveAttribute('aria-labelledby', `${label.id} ${hiddenLabel.id} ${picker.id}`);
206-
expect(picker).toHaveAttribute('aria-labelledby', `${picker.childNodes[0].id} ${label.id} ${hiddenLabel.id}`);
206+
expect(picker).toHaveAttribute('aria-labelledby', `${picker.childNodes[0].id} ${picker.id} ${label.id} ${hiddenLabel.id}`);
207207

208208
expect(label).toHaveAttribute('for', searchfield.id);
209209
});
@@ -220,7 +220,7 @@ describe('SearchWithin labeling', function () {
220220

221221
expect(group).not.toHaveAttribute('aria-labelledby');
222222
expect(searchfield).toHaveAttribute('aria-labelledby', `${group.id} ${hiddenLabel.id} ${picker.id}`);
223-
expect(picker).toHaveAttribute('aria-labelledby', `${picker.childNodes[0].id} ${group.id} ${hiddenLabel.id}`);
223+
expect(picker).toHaveAttribute('aria-labelledby', `${picker.childNodes[0].id} ${picker.id} ${group.id} ${hiddenLabel.id}`);
224224
});
225225

226226
it('aria-labelledby = {id}', function () {
@@ -251,6 +251,6 @@ describe('SearchWithin labeling', function () {
251251
expect(group).toHaveAttribute('aria-labelledby', 'id-foo-label');
252252
expect(searchfield).toHaveAttribute('aria-labelledby', `id-foo-label ${hiddenLabel.id} ${picker.id}`);
253253
expect(searchfield).toHaveAttribute('id', 'id-searchfield');
254-
expect(picker).toHaveAttribute('aria-labelledby', `${picker.childNodes[0].id} id-foo-label ${hiddenLabel.id}`);
254+
expect(picker).toHaveAttribute('aria-labelledby', `${picker.childNodes[0].id} ${picker.id} id-foo-label ${hiddenLabel.id}`);
255255
});
256256
});

‎packages/@react-spectrum/tabs/test/Tabs.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,8 @@ describe('Tabs', function () {
398398

399399
let picker = getByRole('button');
400400
let pickerLabel = within(picker).getByText('Tab 1');
401-
expect(picker).toHaveAttribute('aria-label', 'Test Tabs');
402-
expect(picker).toHaveAttribute('aria-labelledby', `${pickerLabel.id} ${picker.id} external label`);
401+
expect(picker).toHaveAttribute('aria-label', ', Test Tabs');
402+
expect(picker).toHaveAttribute('aria-labelledby', `${pickerLabel.id} external label`);
403403

404404
triggerPress(picker);
405405
act(() => jest.runAllTimers());

‎packages/react-aria-components/test/Select.test.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ let TestSelect = (props) => (
3535

3636
describe('Select', () => {
3737
it('provides slots', () => {
38-
let {getByRole} = render(<TestSelect />);
38+
let {getByRole, getByText} = render(<TestSelect />);
3939

4040
let button = getByRole('button');
4141
expect(button).toHaveTextContent('Select an item');
@@ -45,9 +45,8 @@ describe('Select', () => {
4545
expect(select).toHaveAttribute('data-foo', 'bar');
4646

4747
expect(button).toHaveAttribute('aria-labelledby');
48-
let label = document.getElementById(button.getAttribute('aria-labelledby').split(' ')[1]);
48+
let label = getByText('Favorite Animal');
4949
expect(label).toHaveAttribute('class', 'react-aria-Label');
50-
expect(label).toHaveTextContent('Favorite Animal');
5150

5251
let valueOrPlaceholder = document.getElementById(button.getAttribute('aria-labelledby').split(' ')[0]);
5352
expect(valueOrPlaceholder).toHaveAttribute('class', 'react-aria-SelectValue');
@@ -80,7 +79,7 @@ describe('Select', () => {
8079

8180
let button = getByRole('button');
8281
expect(button.closest('.react-aria-Select')).toHaveAttribute('slot', 'test');
83-
expect(button).toHaveAttribute('aria-label', 'test');
82+
expect(button).toHaveAttribute('aria-label', ', test');
8483
});
8584

8685
it('supports items with render props', () => {

0 commit comments

Comments
 (0)