Skip to content
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.

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

[material-ui][Select] Fix Select when using the spacebar #43966

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
30 changes: 30 additions & 0 deletions packages/mui-material/src/Select/Select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,36 @@ describe('<Select />', () => {
expect(options[1]).to.have.attribute('data-value', '20');
});

it('should select an option when the space key is pressed', () => {
const handleChange = spy();
const handleKeyDown = spy();
const { getAllByRole, getByRole } = render(
<Select value="0" onChange={handleChange}>
<MenuItem value="0" onKeyDown={handleKeyDown}>
Zero
</MenuItem>
<MenuItem value="1" onKeyDown={handleKeyDown}>
One
</MenuItem>
<MenuItem value="2" onKeyDown={handleKeyDown}>
Two
</MenuItem>
</Select>,
);

const trigger = getByRole('combobox');
fireEvent.mouseDown(trigger);

const options = getAllByRole('option');
fireEvent.keyDown(options[0], { key: 'ArrowDown' });
fireEvent.keyDown(options[1], { key: 'ArrowDown' });
fireEvent.keyDown(options[2], { key: ' ' });

expect(handleChange.callCount).to.equal(1);
expect(handleKeyDown.callCount).to.equal(3);
expect(handleChange.firstCall.args[0].target.value).to.equal('2');
});

[' ', 'ArrowUp', 'ArrowDown', 'Enter'].forEach((key) => {
it(`should open menu when pressed ${key} key on select`, async () => {
render(
Expand Down
47 changes: 35 additions & 12 deletions packages/mui-material/src/Select/SelectInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,21 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
}
};

const clonedOnChange = (event, newValue, child) => {
// Redefine target to allow name and value to be read.
// This allows seamless integration with the most popular form libraries.
// https://github.com/mui/material-ui/issues/13485#issuecomment-676048492
// Clone the event to not override `target` of the original event.
const nativeEvent = event.nativeEvent || event;
const clonedEvent = new nativeEvent.constructor(nativeEvent.type, nativeEvent);

Object.defineProperty(clonedEvent, 'target', {
writable: true,
value: { value: newValue, name },
});
onChange(clonedEvent, child);
};

const handleItemClick = (child) => (event) => {
let newValue;

Expand Down Expand Up @@ -286,18 +301,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
setValueState(newValue);

if (onChange) {
// Redefine target to allow name and value to be read.
// This allows seamless integration with the most popular form libraries.
// https://github.com/mui/material-ui/issues/13485#issuecomment-676048492
// Clone the event to not override `target` of the original event.
const nativeEvent = event.nativeEvent || event;
const clonedEvent = new nativeEvent.constructor(nativeEvent.type, nativeEvent);

Object.defineProperty(clonedEvent, 'target', {
writable: true,
value: { value: newValue, name },
});
onChange(clonedEvent, child);
clonedOnChange(event, newValue, child);
}
}

Expand All @@ -324,6 +328,24 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
}
};

const handleItemKeyDown = (child) => (event) => {
if (event.key === ' ') {
const newValue = child.props.value;
if (newValue !== value) {
setValueState(newValue);

if (onChange) {
clonedOnChange(event, newValue, child);
}
}
update(false, event);
}

if (child.props.onKeyDown) {
child.props.onKeyDown(event);
}
sai6855 marked this conversation as resolved.
Show resolved Hide resolved
};

const open = displayNode !== null && openState;

const handleBlur = (event) => {
Expand Down Expand Up @@ -408,6 +430,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
child.props.onKeyUp(event);
}
},
onKeyDown: handleItemKeyDown(child),
role: 'option',
selected,
value: undefined, // The value is most likely not a valid HTML attribute.
Expand Down