diff --git a/packages/material-ui/src/ButtonBase/ButtonBase.js b/packages/material-ui/src/ButtonBase/ButtonBase.js index 92fcf6dd9a4f4c..4980dbd785382b 100644 --- a/packages/material-ui/src/ButtonBase/ButtonBase.js +++ b/packages/material-ui/src/ButtonBase/ButtonBase.js @@ -182,6 +182,11 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) { } }); + const isNonNativeButton = () => { + const button = getButtonNode(); + return component && component !== 'button' && !(button.tagName === 'A' && button.href); + }; + /** * IE 11 shim for https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/repeat */ @@ -206,15 +211,8 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) { onKeyDown(event); } - const button = getButtonNode(); // Keyboard accessibility for non interactive elements - if ( - event.target === event.currentTarget && - component && - component !== 'button' && - (event.key === ' ' || event.key === 'Enter') && - !(button.tagName === 'A' && button.href) - ) { + if (event.target === event.currentTarget && isNonNativeButton() && event.key === 'Enter') { event.preventDefault(); if (onClick) { onClick(event); @@ -232,6 +230,14 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) { if (onKeyUp) { onKeyUp(event); } + + // Keyboard accessibility for non interactive elements + if (event.target === event.currentTarget && isNonNativeButton() && event.key === ' ') { + event.preventDefault(); + if (onClick) { + onClick(event); + } + } }); let ComponentProp = component; diff --git a/packages/material-ui/src/ButtonBase/ButtonBase.test.js b/packages/material-ui/src/ButtonBase/ButtonBase.test.js index 0cd9bb54dc24e0..0378f48d3d5d5e 100644 --- a/packages/material-ui/src/ButtonBase/ButtonBase.test.js +++ b/packages/material-ui/src/ButtonBase/ButtonBase.test.js @@ -680,7 +680,7 @@ describe('', () => { }); describe('keyboard accessibility for non interactive elements', () => { - it('calls onClick when a spacebar is pressed on the element', () => { + it('does not call onClick when a spacebar is pressed on the element', () => { const onClickSpy = spy(event => event.defaultPrevented); const { getByRole } = render( @@ -694,7 +694,24 @@ describe('', () => { key: ' ', }); - expect(onClickSpy.calledOnce).to.equal(true); + expect(onClickSpy.callCount).to.equal(0); + }); + + it('does call onClick when a spacebar is released on the element', () => { + const onClickSpy = spy(event => event.defaultPrevented); + const { getByRole } = render( + + Hello + , + ); + const button = getByRole('button'); + button.focus(); + + fireEvent.keyUp(document.activeElement || document.body, { + key: ' ', + }); + + expect(onClickSpy.callCount).to.equal(1); // defaultPrevented? expect(onClickSpy.returnValues[0]).to.equal(true); }); @@ -735,20 +752,20 @@ describe('', () => { expect(onClickSpy.callCount).to.equal(0); }); - it('does not call onClick if Space was pressed on a child', () => { + it('does not call onClick if Space was released on a child', () => { const onClickSpy = spy(event => event.defaultPrevented); - const onKeyDownSpy = spy(); + const onKeyUpSpy = spy(); render( - + , ); - fireEvent.keyDown(document.activeElement, { + fireEvent.keyUp(document.activeElement, { key: ' ', }); - expect(onKeyDownSpy.callCount).to.equal(1); + expect(onKeyUpSpy.callCount).to.equal(1); expect(onClickSpy.callCount).to.equal(0); }); diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js index 294e2de2597703..f528fcbe2ab23a 100644 --- a/packages/material-ui/src/Chip/Chip.js +++ b/packages/material-ui/src/Chip/Chip.js @@ -277,7 +277,6 @@ const Chip = React.forwardRef(function Chip(props, ref) { label, onClick, onDelete, - onKeyDown, onKeyUp, size = 'medium', variant = 'default', @@ -295,21 +294,6 @@ const Chip = React.forwardRef(function Chip(props, ref) { } }; - const handleKeyDown = event => { - if (onKeyDown) { - onKeyDown(event); - } - - // Ignore events from children of `Chip`. - if (event.currentTarget !== event.target) { - return; - } - - if ([' ', 'Enter', 'Backspace', 'Delete', 'Escape'].indexOf(event.key) !== -1) { - event.preventDefault(); - } - }; - const handleKeyUp = event => { if (onKeyUp) { onKeyUp(event); @@ -409,7 +393,6 @@ const Chip = React.forwardRef(function Chip(props, ref) { aria-disabled={disabled ? true : undefined} tabIndex={clickable || onDelete ? 0 : undefined} onClick={onClick} - onKeyDown={handleKeyDown} onKeyUp={handleKeyUp} ref={handleRef} {...moreProps} diff --git a/packages/material-ui/src/Chip/Chip.test.js b/packages/material-ui/src/Chip/Chip.test.js index 833f7f4e702169..f4f9abf3cbbcc7 100644 --- a/packages/material-ui/src/Chip/Chip.test.js +++ b/packages/material-ui/src/Chip/Chip.test.js @@ -416,15 +416,15 @@ describe('', () => { onClickSpy.resetHistory(); }); - it('should call onClick when `space` is pressed ', () => { + it('should call onClick when `space` is released ', () => { const preventDefaultSpy = spy(); const spaceKeyDown = { preventDefault: preventDefaultSpy, key: ' ', }; wrapper.find('div').simulate('keyDown', spaceKeyDown); - assert.strictEqual(preventDefaultSpy.callCount, 2); - assert.strictEqual(onClickSpy.callCount, 1); + assert.strictEqual(preventDefaultSpy.callCount, 0); + assert.strictEqual(onClickSpy.callCount, 0); const spaceKeyUp = { key: ' ', @@ -441,7 +441,7 @@ describe('', () => { key: 'Enter', }; wrapper.find('div').simulate('keyDown', enterKeyDown); - assert.strictEqual(preventDefaultSpy.callCount, 2); + assert.strictEqual(preventDefaultSpy.callCount, 1); assert.strictEqual(onClickSpy.callCount, 1); const enterKeyUp = { @@ -464,7 +464,7 @@ describe('', () => { key: 'Backspace', }; wrapper.find('div').simulate('keyDown', backspaceKeyDown); - assert.strictEqual(preventDefaultSpy.callCount, 1); + assert.strictEqual(preventDefaultSpy.callCount, 0); assert.strictEqual(onDeleteSpy.callCount, 0); const backspaceKeyUp = { @@ -485,7 +485,7 @@ describe('', () => { key: 'Delete', }; wrapper.find('div').simulate('keyDown', backspaceKeyDown); - assert.strictEqual(preventDefaultSpy.callCount, 1); + assert.strictEqual(preventDefaultSpy.callCount, 0); assert.strictEqual(onDeleteSpy.callCount, 0); const backspaceKeyUp = {