From cd7b5d93ee00ffaae7fa0b4a683c50dbcc49bf7a Mon Sep 17 00:00:00 2001 From: Chris Garrity Date: Tue, 23 Jul 2019 16:33:47 -0400 Subject: [PATCH 1/3] fix focus styling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don’t use default focus style it’s ugly. Don’t allow the image to be dragged. --- src/components/delete-button/delete-button.css | 6 ++++++ src/components/delete-button/delete-button.jsx | 1 + .../__snapshots__/sprite-selector-item.test.jsx.snap | 2 ++ 3 files changed, 9 insertions(+) diff --git a/src/components/delete-button/delete-button.css b/src/components/delete-button/delete-button.css index 6abca7ec0..b8f937eff 100644 --- a/src/components/delete-button/delete-button.css +++ b/src/components/delete-button/delete-button.css @@ -6,11 +6,17 @@ display: flex; align-items: center; justify-content: center; + border-radius: 50%; user-select: none; cursor: pointer; transition: all 0.15s ease-out; } +.delete-button:focus { + outline: none; + box-shadow: 0px 0px 0px 4px $motion-transparent; +} + .delete-button-visible { display: flex; align-items: center; diff --git a/src/components/delete-button/delete-button.jsx b/src/components/delete-button/delete-button.jsx index 6e84fb983..acbb365a4 100644 --- a/src/components/delete-button/delete-button.jsx +++ b/src/components/delete-button/delete-button.jsx @@ -19,6 +19,7 @@ const DeleteButton = props => (
diff --git a/test/unit/components/__snapshots__/sprite-selector-item.test.jsx.snap b/test/unit/components/__snapshots__/sprite-selector-item.test.jsx.snap index f7f19434a..36e144933 100644 --- a/test/unit/components/__snapshots__/sprite-selector-item.test.jsx.snap +++ b/test/unit/components/__snapshots__/sprite-selector-item.test.jsx.snap @@ -57,6 +57,7 @@ exports[`SpriteSelectorItemComponent matches snapshot when given a number and de > @@ -141,6 +142,7 @@ exports[`SpriteSelectorItemComponent matches snapshot when selected 1`] = ` > From 1b4b50b09a8ae03298786fb26a404ca144743d07 Mon Sep 17 00:00:00 2001 From: Chris Garrity Date: Fri, 26 Jul 2019 09:37:39 -0400 Subject: [PATCH 2/3] Make space and enter actually work if focused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the delete button is the current active element (e.g. focused by tab) make hitting ‘space’ or ‘Enter’ do the actual delete. --- .../delete-button/delete-button.jsx | 69 +++++++++++++------ 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/src/components/delete-button/delete-button.jsx b/src/components/delete-button/delete-button.jsx index acbb365a4..62dfcc08a 100644 --- a/src/components/delete-button/delete-button.jsx +++ b/src/components/delete-button/delete-button.jsx @@ -1,31 +1,58 @@ import PropTypes from 'prop-types'; import React from 'react'; +import bindAll from 'lodash.bindall'; import classNames from 'classnames'; import styles from './delete-button.css'; import deleteIcon from './icon--delete.svg'; -const DeleteButton = props => ( -
-
- -
-
- -); +class DeleteButton extends React.Component { + constructor (props) { + super(props); + bindAll(this, [ + 'handleKeyPress', + 'setRef' + ]); + } + componentDidMount () { + document.addEventListener('keydown', this.handleKeyPress); + } + componentWillUnmount () { + document.removeEventListener('keydown', this.handleKeyPress); + } + setRef (ref) { + this.ref = ref; + } + handleKeyPress (event) { + if (this.ref === event.currentTarget.activeElement && (event.key === 'Enter' || event.key === ' ')) { + this.props.onClick(event); + event.preventDefault(); + } + } + render () { + return ( +
+
+ +
+
+ ); + } +} DeleteButton.propTypes = { className: PropTypes.string, From 3701a4fb44079d0f3a3d3828d4402afa328ce836 Mon Sep 17 00:00:00 2001 From: Chris Garrity Date: Wed, 31 Jul 2019 16:14:42 -0400 Subject: [PATCH 3/3] Refactor to separate container/component Also noticed that Delete aria label was not localized. --- .../delete-button/delete-button.jsx | 93 +++++++++---------- .../sprite-selector-item.jsx | 2 +- src/containers/delete-button.jsx | 52 +++++++++++ .../components/sprite-selector-item.test.jsx | 2 +- 4 files changed, 97 insertions(+), 52 deletions(-) create mode 100644 src/containers/delete-button.jsx diff --git a/src/components/delete-button/delete-button.jsx b/src/components/delete-button/delete-button.jsx index 62dfcc08a..c4618d43f 100644 --- a/src/components/delete-button/delete-button.jsx +++ b/src/components/delete-button/delete-button.jsx @@ -1,67 +1,60 @@ import PropTypes from 'prop-types'; import React from 'react'; -import bindAll from 'lodash.bindall'; import classNames from 'classnames'; +import {defineMessages, injectIntl, intlShape} from 'react-intl'; import styles from './delete-button.css'; import deleteIcon from './icon--delete.svg'; -class DeleteButton extends React.Component { - constructor (props) { - super(props); - bindAll(this, [ - 'handleKeyPress', - 'setRef' - ]); +const messages = defineMessages({ + delete: { + id: 'gui.deleteButton.delete', + description: 'Title of the button to delete a sprite, costume or sound', + defaultMessage: 'Delete' } - componentDidMount () { - document.addEventListener('keydown', this.handleKeyPress); - } - componentWillUnmount () { - document.removeEventListener('keydown', this.handleKeyPress); - } - setRef (ref) { - this.ref = ref; - } - handleKeyPress (event) { - if (this.ref === event.currentTarget.activeElement && (event.key === 'Enter' || event.key === ' ')) { - this.props.onClick(event); - event.preventDefault(); - } - } - render () { - return ( -
-
- -
-
- ); - } -} +}); -DeleteButton.propTypes = { +const DeleteButtonComponent = ({ + className, + intl, + onClick, + setRef, + tabIndex, + ...props +}) => ( +
+
+ +
+
+); + + +DeleteButtonComponent.propTypes = { className: PropTypes.string, + intl: intlShape, onClick: PropTypes.func.isRequired, + setRef: PropTypes.func.isRequired, tabIndex: PropTypes.number }; -DeleteButton.defaultProps = { +DeleteButtonComponent.defaultProps = { tabIndex: 0 }; -export default DeleteButton; +export default injectIntl(DeleteButtonComponent); diff --git a/src/components/sprite-selector-item/sprite-selector-item.jsx b/src/components/sprite-selector-item/sprite-selector-item.jsx index 458df186f..e3ed71735 100644 --- a/src/components/sprite-selector-item/sprite-selector-item.jsx +++ b/src/components/sprite-selector-item/sprite-selector-item.jsx @@ -2,7 +2,7 @@ import classNames from 'classnames'; import PropTypes from 'prop-types'; import React from 'react'; -import DeleteButton from '../delete-button/delete-button.jsx'; +import DeleteButton from '../../containers/delete-button.jsx'; import styles from './sprite-selector-item.css'; import {ContextMenuTrigger} from 'react-contextmenu'; import {DangerousMenuItem, ContextMenu, MenuItem} from '../context-menu/context-menu.jsx'; diff --git a/src/containers/delete-button.jsx b/src/containers/delete-button.jsx new file mode 100644 index 000000000..600fbcdee --- /dev/null +++ b/src/containers/delete-button.jsx @@ -0,0 +1,52 @@ +import PropTypes from 'prop-types'; +import React from 'react'; +import bindAll from 'lodash.bindall'; + +import DeleteButtonComponent from '../components/delete-button/delete-button.jsx'; + +class DeleteButton extends React.Component { + constructor (props) { + super(props); + bindAll(this, [ + 'handleKeyPress', + 'setRef' + ]); + } + componentDidMount () { + document.addEventListener('keydown', this.handleKeyPress); + } + componentWillUnmount () { + document.removeEventListener('keydown', this.handleKeyPress); + } + setRef (ref) { + this.ref = ref; + } + handleKeyPress (event) { + if (this.ref === event.currentTarget.activeElement && (event.key === 'Enter' || event.key === ' ')) { + this.props.onClick(event); + event.preventDefault(); + } + } + render () { + return ( + + ); + } +} + +DeleteButton.propTypes = { + className: PropTypes.string, + onClick: PropTypes.func.isRequired, + tabIndex: PropTypes.number +}; + +DeleteButton.defaultProps = { + tabIndex: 0 +}; + +export default DeleteButton; diff --git a/test/unit/components/sprite-selector-item.test.jsx b/test/unit/components/sprite-selector-item.test.jsx index fb103b4ef..b274d967b 100644 --- a/test/unit/components/sprite-selector-item.test.jsx +++ b/test/unit/components/sprite-selector-item.test.jsx @@ -1,7 +1,7 @@ import React from 'react'; import {mountWithIntl, shallowWithIntl, componentWithIntl} from '../../helpers/intl-helpers.jsx'; import SpriteSelectorItemComponent from '../../../src/components/sprite-selector-item/sprite-selector-item'; -import DeleteButton from '../../../src/components/delete-button/delete-button'; +import DeleteButton from '../../../src/containers/delete-button.jsx'; describe('SpriteSelectorItemComponent', () => { let className;