Skip to content

Commit

Permalink
replace button role with combobox role
Browse files Browse the repository at this point in the history
  • Loading branch information
Lingzhi Xu committed Sep 14, 2023
1 parent 942de8f commit f47dcd2
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 53 deletions.
89 changes: 45 additions & 44 deletions packages/mui-material/src/Select/Select.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ describe('<Select />', () => {
</Select>,
);

expect(getByRole('button')).to.have.property('tabIndex', 0);
expect(getByRole('combobox')).to.have.property('tabIndex', 0);
});

it('should accept null child', () => {
Expand Down Expand Up @@ -120,7 +120,7 @@ describe('<Select />', () => {
<MenuItem value={10}>Ten</MenuItem>
</Select>,
);
const trigger = getByRole('button');
const trigger = getByRole('combobox');

fireEvent.mouseDown(trigger);

Expand Down Expand Up @@ -157,7 +157,7 @@ describe('<Select />', () => {
<MenuItem value="">none</MenuItem>
</Select>,
);
const trigger = screen.getByRole('button');
const trigger = screen.getByRole('combobox');
act(() => {
trigger.focus();
});
Expand All @@ -177,7 +177,7 @@ describe('<Select />', () => {
<MenuItem value="">none</MenuItem>
</Select>,
);
const button = getByRole('button');
const button = getByRole('combobox');
act(() => {
button.focus();
});
Expand Down Expand Up @@ -236,13 +236,13 @@ describe('<Select />', () => {

fireEvent.click(getByTestId('label'));

expect(getByRole('button')).toHaveFocus();
expect(getByRole('combobox')).toHaveFocus();
});

it('should focus list if no selection', () => {
const { getByRole } = render(<Select value="" autoFocus />);

fireEvent.mouseDown(getByRole('button'));
fireEvent.mouseDown(getByRole('combobox'));

// TODO not matching WAI-ARIA authoring practices. It should focus the first (or selected) item.
expect(getByRole('listbox')).toHaveFocus();
Expand All @@ -258,7 +258,7 @@ describe('<Select />', () => {
<MenuItem value="2" />
</Select>,
);
fireEvent.mouseDown(getByRole('button'));
fireEvent.mouseDown(getByRole('combobox'));
act(() => {
getAllByRole('option')[1].click();
});
Expand All @@ -279,7 +279,7 @@ describe('<Select />', () => {
</Select>,
);

fireEvent.mouseDown(getByRole('button'));
fireEvent.mouseDown(getByRole('combobox'));
act(() => {
getAllByRole('option')[1].click();
});
Expand All @@ -296,7 +296,7 @@ describe('<Select />', () => {
<MenuItem value="2" />
</Select>,
);
fireEvent.mouseDown(getByRole('button'));
fireEvent.mouseDown(getByRole('combobox'));
act(() => {
getAllByRole('option')[1].click();
});
Expand All @@ -308,7 +308,7 @@ describe('<Select />', () => {
describe('prop: defaultOpen', () => {
it('should be open on mount', () => {
const { getByRole } = render(<Select defaultOpen value="" />);
expect(getByRole('button', { hidden: true })).to.have.attribute('aria-expanded', 'true');
expect(getByRole('combobox', { hidden: true })).to.have.attribute('aria-expanded', 'true');
});
});

Expand Down Expand Up @@ -371,7 +371,7 @@ describe('<Select />', () => {
</Select>,
);

expect(getByRole('button')).to.have.text('Twenty');
expect(getByRole('combobox')).to.have.text('Twenty');
});

describe('warnings', () => {
Expand Down Expand Up @@ -439,19 +439,19 @@ describe('<Select />', () => {
// technically matter. This is only here in case we keep the rest accessible
const { getByRole } = render(<Select open value="" />);

expect(getByRole('button', { hidden: true })).to.have.attribute('aria-expanded', 'true');
expect(getByRole('combobox', { hidden: true })).to.have.attribute('aria-expanded', 'true');
});

specify('ARIA 1.2: aria-expanded="false" if the listbox isn\'t displayed', () => {
const { getByRole } = render(<Select value="" />);

expect(getByRole('button')).to.have.attribute('aria-expanded', 'false');
expect(getByRole('combobox')).to.have.attribute('aria-expanded', 'false');
});

it('sets aria-disabled="true" when component is disabled', () => {
const { getByRole } = render(<Select disabled value="" />);

expect(getByRole('button')).to.have.attribute('aria-disabled', 'true');
expect(getByRole('combobox')).to.have.attribute('aria-disabled', 'true');
});

it('sets disabled attribute in input when component is disabled', () => {
Expand All @@ -463,13 +463,13 @@ describe('<Select />', () => {
specify('aria-disabled is not present if component is not disabled', () => {
const { getByRole } = render(<Select disabled={false} value="" />);

expect(getByRole('button')).not.to.have.attribute('aria-disabled');
expect(getByRole('combobox')).not.to.have.attribute('aria-disabled');
});

it('indicates that activating the button displays a listbox', () => {
const { getByRole } = render(<Select value="" />);

expect(getByRole('button')).to.have.attribute('aria-haspopup', 'listbox');
expect(getByRole('combobox')).to.have.attribute('aria-haspopup', 'listbox');
});

it('renders an element with listbox behavior', () => {
Expand All @@ -478,10 +478,11 @@ describe('<Select />', () => {
expect(getByRole('listbox')).toBeVisible();
});

it('indicates that input element has combobox role and aria-controls set to id of popup', () => {
it('indicates that input element has combobox role and aria-controls set to id of listbox', () => {
const { getByRole } = render(<Select open value="" />);
const listboxId = getByRole('listbox').id;

expect(getByRole('combobox')).to.have.attribute('aria-controls', 'menu-');
expect(getByRole('combobox')).to.have.attribute('aria-controls', listboxId);
});

specify('the listbox is focusable', () => {
Expand Down Expand Up @@ -709,15 +710,15 @@ describe('<Select />', () => {
const { getByRole } = render(<Select value="" />);

// TODO what is the accessible name actually?
expect(getByRole('button')).not.to.have.attribute('aria-labelledby');
expect(getByRole('combobox')).not.to.have.attribute('aria-labelledby');
});

it('is labelled by itself when it has a name', () => {
const { getByRole } = render(<Select name="select" value="" />);

expect(getByRole('button')).to.have.attribute(
expect(getByRole('combobox')).to.have.attribute(
'aria-labelledby',
getByRole('button').getAttribute('id'),
getByRole('combobox').getAttribute('id'),
);
});

Expand All @@ -731,7 +732,7 @@ describe('<Select />', () => {
</React.Fragment>,
);

const triggers = getAllByRole('button');
const triggers = getAllByRole('combobox');

expect(triggers[0]).to.have.attribute(
'aria-labelledby',
Expand All @@ -751,9 +752,9 @@ describe('<Select />', () => {
</React.Fragment>,
);

expect(getByRole('button')).to.have.attribute(
expect(getByRole('combobox')).to.have.attribute(
'aria-labelledby',
`select-label ${getByRole('button').getAttribute('id')}`,
`select-label ${getByRole('combobox').getAttribute('id')}`,
);
});

Expand Down Expand Up @@ -782,7 +783,7 @@ describe('<Select />', () => {
</React.Fragment>,
);

const target = getByRole('button');
const target = getByRole('combobox');
expect(target).to.have.attribute('aria-describedby', 'select-helper-text');
expect(target).toHaveAccessibleDescription('Helper text content');
});
Expand All @@ -796,7 +797,7 @@ describe('<Select />', () => {
<MenuItem value={20}>Twenty</MenuItem>
</Select>,
);
const trigger = screen.getByRole('button');
const trigger = screen.getByRole('combobox');
act(() => {
trigger.focus();
});
Expand All @@ -818,7 +819,7 @@ describe('<Select />', () => {
</Select>,
);

fireEvent.mouseDown(getByRole('button'));
fireEvent.mouseDown(getByRole('combobox'));
clock.tick(99);

expect(onEntered.callCount).to.equal(0);
Expand Down Expand Up @@ -855,7 +856,7 @@ describe('<Select />', () => {
);

const paper = getByTestId('paper');
const selectButton = getByRole('button', { hidden: true });
const selectButton = getByRole('combobox', { hidden: true });

expect(paper.style).to.have.property('minWidth', `${selectButton.clientWidth}px`);
});
Expand All @@ -869,7 +870,7 @@ describe('<Select />', () => {
</Select>,
);

expect(getByRole('button')).to.have.attribute('data-test', 'SelectDisplay');
expect(getByRole('combobox')).to.have.attribute('data-test', 'SelectDisplay');
});
});

Expand All @@ -883,7 +884,7 @@ describe('<Select />', () => {
</Select>,
);

expect(getByRole('button')).to.have.text('Ten');
expect(getByRole('combobox')).to.have.text('Ten');
});
});

Expand All @@ -897,7 +898,7 @@ describe('<Select />', () => {
</Select>,
);

expect(getByRole('button')).to.have.text('0b100');
expect(getByRole('combobox')).to.have.text('0b100');
});
});

Expand Down Expand Up @@ -955,7 +956,7 @@ describe('<Select />', () => {
}
const { getByRole, queryByRole } = render(<ControlledWrapper />);

fireEvent.mouseDown(getByRole('button'));
fireEvent.mouseDown(getByRole('combobox'));
expect(getByRole('listbox')).not.to.equal(null);

act(() => {
Expand Down Expand Up @@ -995,7 +996,7 @@ describe('<Select />', () => {
</Select>,
);

const trigger = getByRole('button');
const trigger = getByRole('combobox');

// If clicked by the right/middle mouse button, no options list should be opened
fireEvent.mouseDown(trigger, { button: 1 });
Expand All @@ -1014,7 +1015,7 @@ describe('<Select />', () => {
</Select>,
);
const parentEl = container.querySelector('.MuiInputBase-root');
const button = getByRole('button');
const button = getByRole('combobox');
stub(parentEl, 'clientWidth').get(() => 14);

fireEvent.mouseDown(button);
Expand All @@ -1028,7 +1029,7 @@ describe('<Select />', () => {
</Select>,
);
const parentEl = container.querySelector('.MuiInputBase-root');
const button = getByRole('button');
const button = getByRole('combobox');
stub(parentEl, 'clientWidth').get(() => 14);

fireEvent.mouseDown(button);
Expand Down Expand Up @@ -1064,7 +1065,7 @@ describe('<Select />', () => {
</Select>,
);

expect(getByRole('button')).to.have.text('Ten, Twenty, Thirty');
expect(getByRole('combobox')).to.have.text('Ten, Twenty, Thirty');
});

it('should not throw an error if `value` is an empty array', () => {
Expand Down Expand Up @@ -1180,7 +1181,7 @@ describe('<Select />', () => {
});
const { getByRole, getAllByRole } = render(<ControlledSelectInput onChange={onChange} />);

fireEvent.mouseDown(getByRole('button'));
fireEvent.mouseDown(getByRole('combobox'));
const options = getAllByRole('option');
fireEvent.click(options[2]);

Expand Down Expand Up @@ -1254,7 +1255,7 @@ describe('<Select />', () => {
it('should focus select after Select did mount', () => {
const { getByRole } = render(<Select value="" autoFocus />);

expect(getByRole('button')).toHaveFocus();
expect(getByRole('combobox')).toHaveFocus();
});
});

Expand Down Expand Up @@ -1288,21 +1289,21 @@ describe('<Select />', () => {
ref.current.focus();
});

expect(getByRole('button')).toHaveFocus();
expect(getByRole('combobox')).toHaveFocus();
});
});

describe('prop: name', () => {
it('should have no id when name is not provided', () => {
const { getByRole } = render(<Select value="" />);

expect(getByRole('button')).not.to.have.attribute('id');
expect(getByRole('combobox')).not.to.have.attribute('id');
});

it('should have select-`name` id when name is provided', () => {
const { getByRole } = render(<Select name="foo" value="" />);

expect(getByRole('button')).to.have.attribute('id', 'mui-component-select-foo');
expect(getByRole('combobox')).to.have.attribute('id', 'mui-component-select-foo');
});
});

Expand Down Expand Up @@ -1375,7 +1376,7 @@ describe('<Select />', () => {
});

expect(onChangeHandler.calledOnce).to.equal(true);
expect(getByRole('button')).to.have.text('France');
expect(getByRole('combobox')).to.have.text('France');
});

it('should support native form validation', function test() {
Expand Down Expand Up @@ -1425,7 +1426,7 @@ describe('<Select />', () => {
<MenuItem value={2}>2</MenuItem>
</Select>,
);
expect(document.activeElement).to.equal(getByRole('button'));
expect(document.activeElement).to.equal(getByRole('combobox'));
});

it('should not override the event.target on mouse events', () => {
Expand Down Expand Up @@ -1664,6 +1665,6 @@ describe('<Select />', () => {

fireEvent.click(getByTestId('test-element'));

expect(getByRole('button')).not.toHaveFocus();
expect(getByRole('combobox')).not.toHaveFocus();
});
});
Loading

0 comments on commit f47dcd2

Please sign in to comment.