Skip to content

Commit 73ca53e

Browse files
authored
Merge pull request #521 from scratchfoundation/fix-pointer-event-hover-detection
fix: hover detection during block drags
2 parents df0a34e + c2a69e7 commit 73ca53e

11 files changed

Lines changed: 231 additions & 47 deletions

File tree

.github/actions/test-package/action.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,14 @@ runs:
2525
env:
2626
TAP_REPORTER: "junit"
2727
TAP_REPORTER_FILE: "test-results/junit.xml"
28+
# xvfb-run provides a virtual display for browsers that need one.
29+
# Firefox's non-headless mode (used because headless Firefox lacks
30+
# WebGL support) requires an X server, and xvfb fills that role
31+
# on the headless CI runner. Chromium doesn't need it but works
32+
# fine under xvfb too.
2833
run: |
2934
mkdir -p test-results
30-
npm run test
35+
xvfb-run --auto-servernum npm run test
3136
- name: Store Test Results
3237
if: ${{ !cancelled() }}
3338
uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5

packages/scratch-gui/playwright.config.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,12 @@ module.exports = defineConfig({
5252
},
5353
{
5454
name: 'firefox',
55-
use: {...devices['Desktop Firefox']}
55+
use: {
56+
...devices['Desktop Firefox'],
57+
// Firefox's built-in headless mode doesn't support WebGL,
58+
// which Scratch requires. Run headed under xvfb in CI.
59+
headless: false
60+
}
5661
}
5762
]
5863
});

packages/scratch-gui/src/components/backpack/backpack.jsx

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ const Backpack = ({
5353
showMore,
5454
onToggle,
5555
onDelete,
56-
onMouseEnter,
57-
onMouseLeave,
58-
onMore
56+
onMore,
57+
onPointerEnter,
58+
onPointerLeave
5959
}) => {
6060
const intl = useIntl();
6161
return (
@@ -93,8 +93,8 @@ const Backpack = ({
9393
[styles.dragOver]: dragOver || blockDragOver
9494
})}
9595
ref={containerRef}
96-
onMouseEnter={onMouseEnter}
97-
onMouseLeave={onMouseLeave}
96+
onPointerEnter={onPointerEnter}
97+
onPointerLeave={onPointerLeave}
9898
>
9999
{error ? (
100100
<div className={styles.statusMessage}>
@@ -181,8 +181,8 @@ Backpack.propTypes = {
181181
loading: PropTypes.bool,
182182
onDelete: PropTypes.func,
183183
onMore: PropTypes.func,
184-
onMouseEnter: PropTypes.func,
185-
onMouseLeave: PropTypes.func,
184+
onPointerEnter: PropTypes.func,
185+
onPointerLeave: PropTypes.func,
186186
onToggle: PropTypes.func,
187187
showMore: PropTypes.bool
188188
};

packages/scratch-gui/src/components/blocks/blocks.css

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,6 @@
8383

8484

8585
.blocks :global(.blocklyBlockDragSurface) {
86-
/*
87-
Fix an issue where the drag surface was preventing hover events for sharing blocks.
88-
This does not prevent user interaction on the blocks themselves.
89-
*/
90-
pointer-events: none;
9186
z-index: $z-index-drag-layer; /* make blocks match gui drag layer */
9287
}
9388

packages/scratch-gui/src/components/sprite-selector-item/sprite-selector-item.jsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ const SpriteSelectorItem = props => {
3535
[styles.isSelected]: props.selected
3636
})}
3737
onClick={props.onClick}
38-
onMouseEnter={props.onMouseEnter}
39-
onMouseLeave={props.onMouseLeave}
38+
onPointerEnter={props.onPointerEnter}
39+
onPointerLeave={props.onPointerLeave}
4040
onMouseDown={props.onMouseDown}
4141
onTouchStart={props.onMouseDown}
4242
ref={props.componentRef}
@@ -123,8 +123,8 @@ SpriteSelectorItem.propTypes = {
123123
onDuplicateButtonClick: PropTypes.func,
124124
onExportButtonClick: PropTypes.func,
125125
onMouseDown: PropTypes.func,
126-
onMouseEnter: PropTypes.func,
127-
onMouseLeave: PropTypes.func,
126+
onPointerEnter: PropTypes.func,
127+
onPointerLeave: PropTypes.func,
128128
preventContextMenu: PropTypes.bool,
129129
selected: PropTypes.bool.isRequired,
130130
isDeleteConfirmationModalOpened: PropTypes.bool

packages/scratch-gui/src/components/stage-selector/stage-selector.jsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ const StageSelector = props => {
5050
onBackdropFileUploadClick,
5151
onBackdropFileUpload,
5252
onClick,
53-
onMouseEnter,
54-
onMouseLeave,
53+
onPointerEnter,
54+
onPointerLeave,
5555
onNewBackdropClick,
5656
onSurpriseBackdropClick,
5757
onEmptyBackdropClick,
@@ -67,8 +67,8 @@ const StageSelector = props => {
6767
})}
6868
componentRef={containerRef}
6969
onClick={onClick}
70-
onMouseEnter={onMouseEnter}
71-
onMouseLeave={onMouseLeave}
70+
onPointerEnter={onPointerEnter}
71+
onPointerLeave={onPointerLeave}
7272
{...componentProps}
7373
>
7474
<div className={styles.header}>
@@ -138,8 +138,8 @@ StageSelector.propTypes = {
138138
onBackdropFileUploadClick: PropTypes.func,
139139
onClick: PropTypes.func,
140140
onEmptyBackdropClick: PropTypes.func,
141-
onMouseEnter: PropTypes.func,
142-
onMouseLeave: PropTypes.func,
141+
onPointerEnter: PropTypes.func,
142+
onPointerLeave: PropTypes.func,
143143
onNewBackdropClick: PropTypes.func,
144144
onSurpriseBackdropClick: PropTypes.func,
145145
raised: PropTypes.bool.isRequired,

packages/scratch-gui/src/containers/backpack.jsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ class Backpack extends React.Component {
2626
'handleToggle',
2727
'handleDelete',
2828
'getContents',
29-
'handleMouseEnter',
30-
'handleMouseLeave',
29+
'handlePointerEnter',
30+
'handlePointerLeave',
3131
'handleBlockDragEnd',
3232
'handleBlockDragUpdate',
3333
'handleMore'
@@ -200,14 +200,14 @@ class Backpack extends React.Component {
200200
blockDragOutsideWorkspace: isOutsideWorkspace
201201
});
202202
}
203-
handleMouseEnter () {
203+
handlePointerEnter () {
204204
if (this.state.blockDragOutsideWorkspace) {
205205
this.setState({
206206
blockDragOverBackpack: true
207207
});
208208
}
209209
}
210-
handleMouseLeave () {
210+
handlePointerLeave () {
211211
this.setState({
212212
blockDragOverBackpack: false
213213
});
@@ -242,8 +242,8 @@ class Backpack extends React.Component {
242242
onDelete={this.handleDelete}
243243
onDrop={this.handleDrop}
244244
onMore={this.handleMore}
245-
onMouseEnter={this.handleMouseEnter}
246-
onMouseLeave={this.handleMouseLeave}
245+
onPointerEnter={this.handlePointerEnter}
246+
onPointerLeave={this.handlePointerLeave}
247247
onToggle={this.props.host ? this.handleToggle : null}
248248
ariaRole={this.props.ariaRole}
249249
ariaLabel={this.props.ariaLabel}

packages/scratch-gui/src/containers/sprite-selector-item.jsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ class SpriteSelectorItem extends React.PureComponent {
2525
'handleClick',
2626
'handleDuplicate',
2727
'handleExport',
28-
'handleMouseEnter',
29-
'handleMouseLeave',
28+
'handlePointerEnter',
29+
'handlePointerLeave',
3030
'handleMouseDown',
3131
'handleDragEnd',
3232
'handleDrag',
@@ -93,7 +93,7 @@ class SpriteSelectorItem extends React.PureComponent {
9393
const {x, y} = getEventXY(e);
9494
const {top, left, bottom, right} = this.ref.getBoundingClientRect();
9595
if (x >= left && x <= right && y >= top && y <= bottom) {
96-
this.handleMouseEnter();
96+
this.handlePointerEnter();
9797
}
9898
}
9999
handleMouseDown (e) {
@@ -113,10 +113,10 @@ class SpriteSelectorItem extends React.PureComponent {
113113
e.stopPropagation();
114114
this.props.onExportButtonClick(this.props.id);
115115
}
116-
handleMouseLeave () {
116+
handlePointerLeave () {
117117
this.props.dispatchSetHoveredSprite(null);
118118
}
119-
handleMouseEnter () {
119+
handlePointerEnter () {
120120
this.props.dispatchSetHoveredSprite(this.props.id);
121121
}
122122
handleDeleteButtonClick (e) {
@@ -140,7 +140,7 @@ class SpriteSelectorItem extends React.PureComponent {
140140
}
141141
render () {
142142
const {
143-
143+
144144
asset,
145145
id,
146146
index,
@@ -153,7 +153,7 @@ class SpriteSelectorItem extends React.PureComponent {
153153
costumeURL,
154154
vm,
155155
deleteConfirmationModalPosition,
156-
156+
157157
...props
158158
} = this.props;
159159
return (<>
@@ -173,8 +173,8 @@ class SpriteSelectorItem extends React.PureComponent {
173173
onDuplicateButtonClick={onDuplicateButtonClick ? this.handleDuplicate : null}
174174
onExportButtonClick={onExportButtonClick ? this.handleExport : null}
175175
onMouseDown={this.handleMouseDown}
176-
onMouseEnter={this.handleMouseEnter}
177-
onMouseLeave={this.handleMouseLeave}
176+
onPointerEnter={this.handlePointerEnter}
177+
onPointerLeave={this.handlePointerLeave}
178178
isDeleteConfirmationModalOpened={this.state.isDeletePromptOpen}
179179
{...props}
180180
/>

packages/scratch-gui/src/containers/stage-selector.jsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ class StageSelector extends React.Component {
5252
'addBackdropFromLibraryItem',
5353
'handleFileUploadClick',
5454
'handleBackdropUpload',
55-
'handleMouseEnter',
56-
'handleMouseLeave',
55+
'handlePointerEnter',
56+
'handlePointerLeave',
5757
'handleTouchEnd',
5858
'handleDrop',
5959
'setFileInput',
@@ -86,7 +86,7 @@ class StageSelector extends React.Component {
8686
const {x, y} = getEventXY(e);
8787
const {top, left, bottom, right} = this.ref.getBoundingClientRect();
8888
if (x >= left && x <= right && y >= top && y <= bottom) {
89-
this.handleMouseEnter();
89+
this.handlePointerEnter();
9090
}
9191
}
9292
addBackdropFromLibraryItem (item, shouldActivateTab = true) {
@@ -124,7 +124,7 @@ class StageSelector extends React.Component {
124124
e.stopPropagation(); // Prevent click from falling through to selecting stage.
125125
// @todo should this not add a backdrop you already have?
126126
const backdrops = this.mergeDynamicAssets();
127-
127+
128128
const item = backdrops[Math.floor(Math.random() * backdrops.length)];
129129
this.addBackdropFromLibraryItem(item, false);
130130
}
@@ -154,10 +154,10 @@ class StageSelector extends React.Component {
154154
e.stopPropagation(); // Prevent click from selecting the stage, that is handled manually in backdrop upload
155155
this.fileInput.click();
156156
}
157-
handleMouseEnter () {
157+
handlePointerEnter () {
158158
this.props.dispatchSetHoveredSprite(this.props.id);
159159
}
160-
handleMouseLeave () {
160+
handlePointerLeave () {
161161
this.props.dispatchSetHoveredSprite(null);
162162
}
163163
handleDrop (dragInfo) {
@@ -201,8 +201,8 @@ class StageSelector extends React.Component {
201201
onClick={this.handleClick}
202202
onDrop={this.handleDrop}
203203
onEmptyBackdropClick={this.handleEmptyBackdrop}
204-
onMouseEnter={this.handleMouseEnter}
205-
onMouseLeave={this.handleMouseLeave}
204+
onPointerEnter={this.handlePointerEnter}
205+
onPointerLeave={this.handlePointerLeave}
206206
onSurpriseBackdropClick={this.handleSurpriseBackdrop}
207207
onNewBackdropClick={this.handleNewBackdropClick}
208208
{...componentProps}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// @ts-check
2+
const {test, expect} = require('@playwright/test');
3+
4+
// Regression test for the backpack drag-over highlight.
5+
//
6+
// Two bugs prevented the backpack from highlighting when a block was
7+
// dragged over it:
8+
//
9+
// 1. The backpack used onMouseEnter/onMouseLeave, but Blockly calls
10+
// preventDefault() on pointermove during drags, which suppresses
11+
// compatibility mouse events on Firefox. Fixed by switching to
12+
// onPointerEnter/onPointerLeave.
13+
//
14+
// 2. The dragged block's SVG children had pointer-events:auto, causing
15+
// them to steal hover/pointer events from elements underneath.
16+
// Fixed by setting pointer-events:none on all drag surface children
17+
// in scratch-blocks.
18+
19+
test('backpack highlights when a block is dragged over it', async ({page}) => {
20+
await page.goto('index.html?backpack_host=fake');
21+
22+
// Expand the backpack.
23+
await page.getByText('Backpack', {exact: true}).click();
24+
const backpackList = page.locator('[class*="backpack-list"]').first();
25+
await expect(backpackList).toBeVisible();
26+
27+
// Find a visible flyout block. The flyout contains blocks from all
28+
// categories (187+), most offscreen. Filter to blocks that are in
29+
// the viewport and large enough to be a real block (not a field).
30+
const block = await page.evaluate(() => {
31+
const blocks = document.querySelectorAll('.blocklyFlyout .blocklyDraggable');
32+
for (const b of blocks) {
33+
const rect = b.getBoundingClientRect();
34+
if (rect.y > 80 && rect.y < window.innerHeight && rect.height > 20 && rect.width > 50) {
35+
return {x: rect.x + (rect.width / 2), y: rect.y + (rect.height / 2)};
36+
}
37+
}
38+
return null;
39+
});
40+
expect(block, 'should find a visible flyout block').not.toBeNull();
41+
42+
const backpackBox = await backpackList.boundingBox();
43+
44+
// Drag a block from the flyout across the workspace to the backpack.
45+
await page.mouse.move(block.x, block.y);
46+
await page.mouse.down();
47+
await page.mouse.move(block.x + 200, block.y, {steps: 10});
48+
await page.mouse.move(
49+
backpackBox.x + (backpackBox.width / 2),
50+
backpackBox.y + (backpackBox.height / 2),
51+
{steps: 10}
52+
);
53+
54+
await expect(backpackList).toHaveClass(/drag-over/);
55+
56+
await page.mouse.up();
57+
});

0 commit comments

Comments
 (0)