Skip to content

Commit bac416c

Browse files
authored
fix: When quickly selecting different instances, the instance is not selected. (#4370)
## Description partial fix for 1 from #4095 The issue was incorrect clickCount count ## Steps for reproduction 1. click button 2. expect xyz ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 5de6) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file
1 parent 11603f4 commit bac416c

File tree

2 files changed

+78
-68
lines changed

2 files changed

+78
-68
lines changed

apps/builder/app/builder/features/workspace/canvas-tools/outline/hovered-instance-outline.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Label } from "./label";
1010
import { applyScale } from "./apply-scale";
1111
import { $scale } from "~/builder/shared/nano-states";
1212
import { findClosestSlot } from "~/shared/instance-utils";
13-
import deepEqual from "fast-deep-equal";
13+
import { shallowEqual } from "shallow-equal";
1414

1515
export const HoveredInstanceOutline = () => {
1616
const instances = useStore($instances);
@@ -24,7 +24,7 @@ export const HoveredInstanceOutline = () => {
2424
}
2525

2626
if (
27-
deepEqual(hoveredInstanceSelector, textEditingInstanceSelector?.selector)
27+
shallowEqual(hoveredInstanceSelector, textEditingInstanceSelector?.selector)
2828
) {
2929
return;
3030
}

apps/builder/app/canvas/instance-selection.ts

Lines changed: 76 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -7,86 +7,96 @@ import {
77
} from "~/shared/nano-states";
88
import { $textEditingInstanceSelector } from "~/shared/nano-states";
99
import { emitCommand } from "./shared/commands";
10+
import { shallowEqual } from "shallow-equal";
11+
12+
const isElementBeingEdited = (element: Element) => {
13+
if (element.closest("[contenteditable=true]")) {
14+
return true;
15+
}
16+
17+
return false;
18+
};
1019

1120
export const subscribeInstanceSelection = ({
1221
signal,
1322
}: {
1423
signal: AbortSignal;
1524
}) => {
16-
let pointerDownElement: undefined | Element = undefined;
17-
let lastPointerUpTime = 0;
18-
let clickCount = 1;
19-
20-
const handlePointerDown = (event: PointerEvent) => {
21-
pointerDownElement = event.target as Element;
22-
// track multiple clicks when pointerdown is fired within 500ms after last pointerup
23-
const currentTime = performance.now();
24-
if (currentTime - lastPointerUpTime < 500) {
25-
clickCount += 1;
26-
} else {
27-
clickCount = 1;
28-
}
29-
};
30-
31-
const handlePointerUp = (event: PointerEvent) => {
32-
const element = event.target as Element;
33-
34-
// when user is selecting text inside content editable and mouse goes up
35-
// on a different instance - we don't want to select a different instance
36-
// because that would cancel the text selection.
37-
if (pointerDownElement === undefined || pointerDownElement !== element) {
38-
return;
39-
}
40-
pointerDownElement = undefined;
41-
42-
// track double clicks manually because pointer events do not support event.detail
43-
// for clicks count
44-
lastPointerUpTime = performance.now();
45-
46-
if (clickCount === 1) {
47-
// notify whole app about click on document
48-
// e.g. to hide the side panel
25+
addEventListener(
26+
"click",
27+
(event) => {
28+
const element = event.target;
29+
4930
emitCommand("clickCanvas");
50-
}
51-
52-
// prevent selecting instances inside text editor while editing text
53-
if (element.closest("[contenteditable=true]")) {
54-
return;
55-
}
56-
57-
const instanceSelector = getInstanceSelectorFromElement(element);
58-
if (instanceSelector === undefined) {
59-
return;
60-
}
61-
62-
// the first click in double click or the only one in regular click
63-
if (clickCount === 1) {
64-
$selectedInstanceSelector.set(instanceSelector);
65-
// reset text editor when another instance is selected
66-
$textEditingInstanceSelector.set(undefined);
67-
}
68-
69-
// the second click in a double click.
70-
if (clickCount === 2) {
31+
32+
if (!(element instanceof Element)) {
33+
return;
34+
}
35+
36+
if (isElementBeingEdited(element)) {
37+
return;
38+
}
39+
40+
const instanceSelector = getInstanceSelectorFromElement(element);
41+
42+
if (instanceSelector === undefined) {
43+
return;
44+
}
45+
46+
// Prevent unnecessary updates (2 clicks are registered before a double click)
47+
if ($textEditingInstanceSelector.get() !== undefined) {
48+
$textEditingInstanceSelector.set(undefined);
49+
}
50+
51+
// Prevent unnecessary updates (2 clicks are registered before a double click)
52+
if (!shallowEqual(instanceSelector, $selectedInstanceSelector.get())) {
53+
$selectedInstanceSelector.set(instanceSelector);
54+
}
55+
},
56+
{ passive: true, signal }
57+
);
58+
59+
addEventListener(
60+
"dblclick",
61+
(event) => {
62+
const element = event.target;
63+
64+
if (!(element instanceof Element)) {
65+
return;
66+
}
67+
68+
if (isElementBeingEdited(element)) {
69+
return;
70+
}
71+
72+
const instanceSelector = getInstanceSelectorFromElement(element);
73+
74+
if (instanceSelector === undefined) {
75+
return;
76+
}
77+
7178
const editableInstanceSelector = findClosestEditableInstanceSelector(
7279
instanceSelector,
7380
$instances.get(),
7481
$registeredComponentMetas.get()
7582
);
7683

77-
// enable text editor when double click on its instance or one of its descendants
78-
if (editableInstanceSelector) {
84+
if (editableInstanceSelector === undefined) {
85+
return;
86+
}
87+
88+
// Prevent unnecessary updates (should already be selected during click)
89+
if (!shallowEqual(instanceSelector, editableInstanceSelector)) {
7990
$selectedInstanceSelector.set(editableInstanceSelector);
80-
$textEditingInstanceSelector.set({
81-
selector: editableInstanceSelector,
82-
reason: "click",
83-
mouseX: event.clientX,
84-
mouseY: event.clientY,
85-
});
8691
}
87-
}
88-
};
8992

90-
addEventListener("pointerdown", handlePointerDown, { passive: true, signal });
91-
addEventListener("pointerup", handlePointerUp, { passive: true, signal });
93+
$textEditingInstanceSelector.set({
94+
selector: editableInstanceSelector,
95+
reason: "click",
96+
mouseX: event.clientX,
97+
mouseY: event.clientY,
98+
});
99+
},
100+
{ passive: true, signal }
101+
);
92102
};

0 commit comments

Comments
 (0)