Skip to content

Commit 288aa72

Browse files
authored
Merge pull request scratchfoundation#4915 from chrisgarrity/4911-delete-icon
Add larger delete icon to make it easier to do on touch devices
2 parents f2ac39f + 4e47cc8 commit 288aa72

File tree

9 files changed

+131
-24
lines changed

9 files changed

+131
-24
lines changed
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
@import "../../css/colors.css";
2+
@import "../../css/units.css";
3+
4+
/* the delete button has .25rem invisible 'slop' around the visible button
5+
to make it easier to tap on a touch device */
6+
.delete-button {
7+
display: flex;
8+
align-items: center;
9+
justify-content: center;
10+
11+
overflow: hidden; /* Mask the icon animation */
12+
width: 2rem;
13+
height: 2rem;
14+
user-select: none;
15+
cursor: pointer;
16+
transition: all 0.15s ease-out;
17+
18+
}
19+
20+
.delete-button-visible {
21+
display: flex;
22+
align-items: center;
23+
justify-content: center;
24+
25+
overflow: hidden; /* Mask the icon animation */
26+
width: 1.75rem;
27+
height: 1.75rem;
28+
box-shadow: 0px 0px 0px 2px $motion-transparent;
29+
background-color: $motion-primary;
30+
color: $ui-white;
31+
border-radius: 50%;
32+
font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
33+
user-select: none;
34+
cursor: pointer;
35+
transition: all 0.15s ease-out;
36+
}
37+
38+
.delete-icon {
39+
position: relative;
40+
margin: 0.25rem;
41+
user-select: none;
42+
transform-origin: 50%;
43+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import PropTypes from 'prop-types';
2+
import React from 'react';
3+
import classNames from 'classnames';
4+
5+
import styles from './delete-button.css';
6+
import deleteIcon from './icon--delete.svg';
7+
8+
const DeleteButton = props => (
9+
<div
10+
aria-label="Delete"
11+
className={classNames(
12+
styles.deleteButton,
13+
props.className
14+
)}
15+
role="button"
16+
tabIndex={props.tabIndex}
17+
onClick={props.onClick}
18+
>
19+
<div className={styles.deleteButtonVisible}>
20+
<img
21+
className={styles.deleteIcon}
22+
src={deleteIcon}
23+
/>
24+
</div>
25+
</div>
26+
27+
);
28+
29+
DeleteButton.propTypes = {
30+
className: PropTypes.string,
31+
onClick: PropTypes.func.isRequired,
32+
tabIndex: PropTypes.number
33+
};
34+
35+
DeleteButton.defaultProps = {
36+
tabIndex: 0
37+
};
38+
39+
export default DeleteButton;
Lines changed: 18 additions & 0 deletions
Loading

src/components/sprite-selector-item/sprite-selector-item.css

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,16 +94,16 @@
9494

9595
.delete-button {
9696
position: absolute;
97-
top: 0.125rem;
97+
top: -.625rem;
9898
z-index: auto;
9999
}
100100

101101
[dir="ltr"] .delete-button {
102-
right: 0.125rem;
102+
right: -.625rem;
103103
}
104104

105105
[dir="rtl"] .delete-button {
106-
left: 0.125rem;
106+
left: -.625rem;
107107
}
108108

109109
.number {

src/components/sprite-selector-item/sprite-selector-item.jsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import classNames from 'classnames';
22
import PropTypes from 'prop-types';
33
import React from 'react';
44

5-
import CloseButton from '../close-button/close-button.jsx';
5+
import DeleteButton from '../delete-button/delete-button.jsx';
66
import styles from './sprite-selector-item.css';
77
import {ContextMenuTrigger} from 'react-contextmenu';
88
import {DangerousMenuItem, ContextMenu, MenuItem} from '../context-menu/context-menu.jsx';
@@ -48,9 +48,8 @@ const SpriteSelectorItem = props => (
4848
) : null}
4949
</div>
5050
{(props.selected && props.onDeleteButtonClick) ? (
51-
<CloseButton
51+
<DeleteButton
5252
className={styles.deleteButton}
53-
size={CloseButton.SIZE_SMALL}
5453
onClick={props.onDeleteButtonClick}
5554
/>
5655
) : null }

test/integration/sprites.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ describe('Working with sprites', () => {
7575
test('Deleting by x button on sprite tile', async () => {
7676
await loadUri(uri);
7777
await new Promise(resolve => setTimeout(resolve, 1000)); // Wait for scroll animation
78-
await clickXpath('//*[@aria-label="Close"]'); // Only visible close button is on the sprite
78+
await clickXpath('//*[@aria-label="Delete"]'); // Only visible close button is on the sprite
7979
// Confirm that the stage has been switched to
8080
await findByText('Stage selected: no motion blocks');
8181
const logs = await getLogs();

test/unit/components/__snapshots__/sprite-selector-item.test.jsx.snap

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,20 @@ exports[`SpriteSelectorItemComponent matches snapshot when given a number and de
4646
</div>
4747
</div>
4848
<div
49-
aria-label="Close"
49+
aria-label="Delete"
5050
className=""
5151
onClick={[Function]}
5252
role="button"
53-
tabIndex="0"
53+
tabIndex={0}
5454
>
55-
<img
56-
className=""
57-
src="test-file-stub"
58-
/>
55+
<div
56+
className={undefined}
57+
>
58+
<img
59+
className={undefined}
60+
src="test-file-stub"
61+
/>
62+
</div>
5963
</div>
6064
<nav
6165
className="react-contextmenu"
@@ -126,16 +130,20 @@ exports[`SpriteSelectorItemComponent matches snapshot when selected 1`] = `
126130
</div>
127131
</div>
128132
<div
129-
aria-label="Close"
133+
aria-label="Delete"
130134
className=""
131135
onClick={[Function]}
132136
role="button"
133-
tabIndex="0"
137+
tabIndex={0}
134138
>
135-
<img
136-
className=""
137-
src="test-file-stub"
138-
/>
139+
<div
140+
className={undefined}
141+
>
142+
<img
143+
className={undefined}
144+
src="test-file-stub"
145+
/>
146+
</div>
139147
</div>
140148
<nav
141149
className="react-contextmenu"

test/unit/components/sprite-selector-item.test.jsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React from 'react';
22
import {mountWithIntl, shallowWithIntl, componentWithIntl} from '../../helpers/intl-helpers.jsx';
33
import SpriteSelectorItemComponent from '../../../src/components/sprite-selector-item/sprite-selector-item';
4-
import CloseButton from '../../../src/components/close-button/close-button';
4+
import DeleteButton from '../../../src/components/delete-button/delete-button';
55

66
describe('SpriteSelectorItemComponent', () => {
77
let className;
@@ -56,7 +56,7 @@ describe('SpriteSelectorItemComponent', () => {
5656
test('does not have a close box when not selected', () => {
5757
selected = false;
5858
const wrapper = shallowWithIntl(getComponent());
59-
expect(wrapper.find(CloseButton).exists()).toBe(false);
59+
expect(wrapper.find(DeleteButton).exists()).toBe(false);
6060
});
6161

6262
test('triggers callback when Box component is clicked', () => {
@@ -68,7 +68,7 @@ describe('SpriteSelectorItemComponent', () => {
6868

6969
test('triggers callback when CloseButton component is clicked', () => {
7070
const wrapper = shallowWithIntl(getComponent());
71-
wrapper.find(CloseButton).simulate('click');
71+
wrapper.find(DeleteButton).simulate('click');
7272
expect(onDeleteButtonClick).toHaveBeenCalled();
7373
});
7474

test/unit/containers/sprite-selector-item.test.jsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import configureStore from 'redux-mock-store';
44
import {Provider} from 'react-redux';
55

66
import SpriteSelectorItem from '../../../src/containers/sprite-selector-item';
7-
import CloseButton from '../../../src/components/close-button/close-button';
7+
import DeleteButton from '../../../src/components/delete-button/delete-button';
88

99
describe('SpriteSelectorItem Container', () => {
1010
const mockStore = configureStore();
@@ -52,7 +52,7 @@ describe('SpriteSelectorItem Container', () => {
5252

5353
test('should delete the sprite', () => {
5454
const wrapper = mountWithIntl(getContainer());
55-
wrapper.find(CloseButton).simulate('click');
55+
wrapper.find(DeleteButton).simulate('click');
5656
expect(onDeleteButtonClick).toHaveBeenCalledWith(1337);
5757
});
5858
});

0 commit comments

Comments
 (0)