Skip to content

Commit 746c261

Browse files
aidenybaiaidenybai
authored
fix: improve freeze system and scheduling for element detection (#291)
1 parent ddcbb4c commit 746c261

15 files changed

Lines changed: 136 additions & 150 deletions

‎packages/react-grab/e2e/clear-history-prompt.spec.ts‎

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ const copyElement = async (reactGrab: ReactGrabPageObject, selector: string) =>
77
await reactGrab.typeInInput("comment");
88
await reactGrab.submitInput();
99
await expect.poll(() => reactGrab.getClipboardContent(), { timeout: 5000 }).toBeTruthy();
10-
// HACK: Wait for copy feedback transition and comments item addition
11-
await reactGrab.page.waitForTimeout(300);
10+
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 5000 }).toBe(true);
1211
};
1312

1413
test.describe("Toolbar Copy All Button", () => {
@@ -24,18 +23,18 @@ test.describe("Toolbar Copy All Button", () => {
2423
await copyElement(reactGrab, "li:first-child");
2524
await reactGrab.clickCommentsButton();
2625

27-
await expect.poll(() => reactGrab.isToolbarCopyAllVisible(), { timeout: 2000 }).toBe(true);
26+
await expect.poll(() => reactGrab.isToolbarCopyAllVisible(), { timeout: 5000 }).toBe(true);
2827
});
2928

3029
test("should hide when comments dropdown is closed", async ({ reactGrab }) => {
3130
await copyElement(reactGrab, "li:first-child");
3231
await reactGrab.clickCommentsButton();
3332

34-
await expect.poll(() => reactGrab.isToolbarCopyAllVisible(), { timeout: 2000 }).toBe(true);
33+
await expect.poll(() => reactGrab.isToolbarCopyAllVisible(), { timeout: 5000 }).toBe(true);
3534

3635
await reactGrab.clickCommentsButton();
3736

38-
await expect.poll(() => reactGrab.isToolbarCopyAllVisible(), { timeout: 2000 }).toBe(false);
37+
await expect.poll(() => reactGrab.isToolbarCopyAllVisible(), { timeout: 5000 }).toBe(false);
3938
});
4039
});
4140

@@ -60,7 +59,7 @@ test.describe("Toolbar Copy All Button", () => {
6059
await reactGrab.clickToolbarCopyAll();
6160

6261
await expect
63-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
62+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
6463
.toBe(true);
6564
});
6665
});
@@ -74,7 +73,7 @@ test.describe("Clear History Prompt", () => {
7473
await reactGrab.clickToolbarCopyAll();
7574

7675
await expect
77-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
76+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
7877
.toBe(true);
7978
});
8079

@@ -84,7 +83,7 @@ test.describe("Clear History Prompt", () => {
8483
await reactGrab.clickCommentsCopyAll();
8584

8685
await expect
87-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
86+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
8887
.toBe(true);
8988
});
9089
});
@@ -98,13 +97,13 @@ test.describe("Clear History Prompt", () => {
9897
await reactGrab.clickToolbarCopyAll();
9998

10099
await expect
101-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
100+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
102101
.toBe(true);
103102

104103
await reactGrab.confirmClearCommentsPrompt();
105104
await reactGrab.page.waitForTimeout(200);
106105

107-
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 2000 }).toBe(false);
106+
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 5000 }).toBe(false);
108107
});
109108

110109
test("should clear comments when confirmed via Enter key", async ({ reactGrab }) => {
@@ -114,17 +113,17 @@ test.describe("Clear History Prompt", () => {
114113
await reactGrab.clickToolbarCopyAll();
115114

116115
await expect
117-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
116+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
118117
.toBe(true);
119118

120119
await reactGrab.pressEnter();
121120
await reactGrab.page.waitForTimeout(200);
122121

123122
await expect
124-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
123+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
125124
.toBe(false);
126125

127-
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 2000 }).toBe(false);
126+
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 5000 }).toBe(false);
128127
});
129128

130129
test("should dismiss the prompt after confirming", async ({ reactGrab }) => {
@@ -134,13 +133,13 @@ test.describe("Clear History Prompt", () => {
134133
await reactGrab.clickToolbarCopyAll();
135134

136135
await expect
137-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
136+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
138137
.toBe(true);
139138

140139
await reactGrab.confirmClearCommentsPrompt();
141140

142141
await expect
143-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
142+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
144143
.toBe(false);
145144
});
146145

@@ -153,23 +152,23 @@ test.describe("Clear History Prompt", () => {
153152
await reactGrab.clickToolbarCopyAll();
154153

155154
await expect
156-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
155+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
157156
.toBe(true);
158157

159158
await reactGrab.confirmClearCommentsPrompt();
160159

161-
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 2000 }).toBe(false);
160+
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 5000 }).toBe(false);
162161

163162
await copyElement(reactGrab, "li:last-child");
164163

165164
await reactGrab.clickCommentsButton();
166165
await reactGrab.clickToolbarCopyAll();
167166

168167
await expect
169-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
168+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
170169
.toBe(false);
171170

172-
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 2000 }).toBe(false);
171+
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 5000 }).toBe(false);
173172
});
174173
});
175174

@@ -181,17 +180,17 @@ test.describe("Clear History Prompt", () => {
181180
await reactGrab.clickToolbarCopyAll();
182181

183182
await expect
184-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
183+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
185184
.toBe(true);
186185

187186
await reactGrab.cancelClearCommentsPrompt();
188187
await reactGrab.page.waitForTimeout(200);
189188

190189
await expect
191-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
190+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
192191
.toBe(false);
193192

194-
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 2000 }).toBe(true);
193+
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 5000 }).toBe(true);
195194
});
196195

197196
test("should dismiss prompt when cancelled via Escape key", async ({ reactGrab }) => {
@@ -201,17 +200,17 @@ test.describe("Clear History Prompt", () => {
201200
await reactGrab.clickToolbarCopyAll();
202201

203202
await expect
204-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
203+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
205204
.toBe(true);
206205

207206
await reactGrab.pressEscape();
208207
await reactGrab.page.waitForTimeout(200);
209208

210209
await expect
211-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
210+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
212211
.toBe(false);
213212

214-
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 2000 }).toBe(true);
213+
await expect.poll(() => reactGrab.isCommentsButtonVisible(), { timeout: 5000 }).toBe(true);
215214
});
216215
});
217216

@@ -223,7 +222,7 @@ test.describe("Clear History Prompt", () => {
223222
await reactGrab.clickToolbarCopyAll();
224223

225224
await expect
226-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
225+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
227226
.toBe(true);
228227

229228
await reactGrab.activate();
@@ -232,7 +231,7 @@ test.describe("Clear History Prompt", () => {
232231
await reactGrab.rightClickElement("li:first-child");
233232

234233
await expect
235-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
234+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
236235
.toBe(false);
237236
});
238237

@@ -243,14 +242,14 @@ test.describe("Clear History Prompt", () => {
243242
await reactGrab.clickToolbarCopyAll();
244243

245244
await expect
246-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
245+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
247246
.toBe(true);
248247

249248
await reactGrab.clickToolbarEnabled();
250249
await reactGrab.page.waitForTimeout(200);
251250

252251
await expect
253-
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 2000 })
252+
.poll(() => reactGrab.isClearCommentsPromptVisible(), { timeout: 5000 })
254253
.toBe(false);
255254
});
256255
});

‎packages/react-grab/e2e/context-menu.spec.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ test.describe("Context Menu", () => {
4646
const element = reactGrab.page.locator("li").first();
4747
await element.click({ button: "right", force: true });
4848

49-
await expect.poll(() => reactGrab.isContextMenuVisible(), { timeout: 2000 }).toBe(true);
49+
await expect.poll(() => reactGrab.isContextMenuVisible(), { timeout: 5000 }).toBe(true);
5050

5151
await reactGrab.page.keyboard.up("c");
5252
await reactGrab.page.keyboard.up(reactGrab.modifierKey);
@@ -118,10 +118,8 @@ test.describe("Context Menu", () => {
118118
expect(isVisibleBefore).toBe(true);
119119

120120
await reactGrab.page.keyboard.press("Escape");
121-
await reactGrab.page.waitForTimeout(200);
122121

123-
const isVisibleAfter = await reactGrab.isContextMenuVisible();
124-
expect(isVisibleAfter).toBe(false);
122+
await expect.poll(() => reactGrab.isContextMenuVisible(), { timeout: 5000 }).toBe(false);
125123
});
126124

127125
test("should dismiss context menu when clicking outside", async ({ reactGrab }) => {
@@ -134,10 +132,8 @@ test.describe("Context Menu", () => {
134132
expect(isVisibleBefore).toBe(true);
135133

136134
await reactGrab.page.mouse.click(10, 10);
137-
await reactGrab.page.waitForTimeout(200);
138135

139-
const isVisibleAfter = await reactGrab.isContextMenuVisible();
140-
expect(isVisibleAfter).toBe(false);
136+
await expect.poll(() => reactGrab.isContextMenuVisible(), { timeout: 5000 }).toBe(false);
141137
});
142138

143139
test("should dismiss context menu after Copy action", async ({ reactGrab }) => {

‎packages/react-grab/e2e/fixtures.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ const createReactGrabPageObject = (page: Page): ReactGrabPageObject => {
265265
const hoverElement = async (selector: string) => {
266266
const element = page.locator(selector).first();
267267
await element.hover({ force: true });
268-
await page.waitForTimeout(250);
268+
await page.waitForTimeout(350);
269269
};
270270

271271
const clickElement = async (selector: string) => {
@@ -422,23 +422,23 @@ const createReactGrabPageObject = (page: Page): ReactGrabPageObject => {
422422
return expectedVisible ? menuItem !== null : menuItem === null;
423423
},
424424
{ attrName: ATTRIBUTE_NAME, expectedVisible: visible },
425-
{ timeout: 2000 },
425+
{ timeout: 5000 },
426426
);
427427
};
428428

429429
const rightClickElement = async (selector: string) => {
430+
const wasActive = await isOverlayVisible();
430431
const element = page.locator(selector).first();
431432
await element.click({ button: "right", force: true });
432-
const isActive = await isOverlayVisible();
433-
if (isActive) {
433+
if (wasActive) {
434434
await waitForContextMenu(true);
435435
}
436436
};
437437

438438
const rightClickAtPosition = async (x: number, y: number) => {
439+
const wasActive = await isOverlayVisible();
439440
await page.mouse.click(x, y, { button: "right" });
440-
const isActive = await isOverlayVisible();
441-
if (isActive) {
441+
if (wasActive) {
442442
await waitForContextMenu(true);
443443
}
444444
};
@@ -576,7 +576,7 @@ const createReactGrabPageObject = (page: Page): ReactGrabPageObject => {
576576
return api?.getState()?.isPromptMode === expected;
577577
},
578578
active,
579-
{ timeout: 2000 },
579+
{ timeout: 5000 },
580580
);
581581
};
582582

@@ -600,7 +600,7 @@ const createReactGrabPageObject = (page: Page): ReactGrabPageObject => {
600600
return state?.isSelectionBoxVisible || state?.targetElement !== null;
601601
},
602602
undefined,
603-
{ timeout: 2000 },
603+
{ timeout: 5000 },
604604
)
605605
.then(() => true)
606606
.catch(() => false);

‎packages/react-grab/e2e/freeze-updates.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ test.describe("Freeze Updates", () => {
235235
await reactGrab.enterPromptMode("[data-testid='dynamic-element-1']");
236236
await reactGrab.deactivate();
237237
// HACK: allow freeze cleanup to fully propagate before next iteration
238-
await reactGrab.page.waitForTimeout(300);
238+
await reactGrab.page.waitForTimeout(500);
239239
}
240240

241241
const getElementCount = async () => {

‎packages/react-grab/src/components/toolbar/index.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,14 +248,14 @@ export const Toolbar: Component<ToolbarProps> = (props) => {
248248
setTooltipVisible: (visible: boolean) => void,
249249
options?: FreezeHandlersOptions,
250250
) => ({
251-
onMouseEnter: () => {
251+
onMouseEnter: (event: MouseEvent) => {
252252
if (drag.isDragging()) return;
253253
safePolygonTracker.stop();
254254
setTooltipVisible(true);
255255
if (options?.shouldFreezeInteractions !== false && !unfreezeUpdatesCallback) {
256256
unfreezeUpdatesCallback = freezeUpdates();
257257
freezeGlobalAnimations();
258-
freezePseudoStates();
258+
freezePseudoStates(event.clientX, event.clientY);
259259
}
260260
options?.onHoverChange?.(true);
261261
},

‎packages/react-grab/src/core/context.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { getTagName } from "../utils/get-tag-name.js";
2626
import { truncateString } from "../utils/truncate-string.js";
2727
import { getNextBasePath } from "../utils/get-next-base-path.js";
2828
import { normalizeFilePath } from "../utils/normalize-file-path.js";
29+
import { isInternalAttribute } from "../utils/strip-internal-attributes.js";
2930

3031
const NON_COMPONENT_PREFIXES = new Set([
3132
"_",
@@ -518,6 +519,7 @@ const getFallbackContext = (element: Element): string => {
518519

519520
let attrsText = "";
520521
for (const { name, value } of element.attributes) {
522+
if (isInternalAttribute(name)) continue;
521523
attrsText += ` ${name}="${value}"`;
522524
}
523525

@@ -565,6 +567,7 @@ export const getHTMLPreview = (element: Element): string => {
565567

566568
let attrsText = "";
567569
for (const { name, value } of element.attributes) {
570+
if (isInternalAttribute(name)) continue;
568571
attrsText += ` ${name}="${truncateAttrValue(value)}"`;
569572
}
570573

‎packages/react-grab/src/core/index.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,6 @@ import { createArrowNavigator } from "./arrow-navigation.js";
118118
import { getRequiredModifiers, setupKeyboardEventClaimer } from "./keyboard-handlers.js";
119119
import { createAutoScroller, getAutoScrollDirection } from "./auto-scroll.js";
120120
import { logIntro } from "./log-intro.js";
121-
import { onIdle } from "../utils/on-idle.js";
122121
import { getScriptOptions } from "../utils/get-script-options.js";
123122
import { isEnterCode } from "../utils/is-enter-code.js";
124123
import { isMac } from "../utils/is-mac.js";
@@ -231,7 +230,8 @@ export const init = (rawOptions?: Options): ReactGrabAPI => {
231230
const isDragging = createMemo(
232231
() =>
233232
store.current.state === "active" &&
234-
(store.current.phase === "dragging-select" || store.current.phase === "dragging-reposition"),
233+
(store.current.phase === "dragging-select" ||
234+
store.current.phase === "dragging-reposition"),
235235
);
236236
const isDragRepositioning = createMemo(
237237
() => store.current.state === "active" && store.current.phase === "dragging-reposition",
@@ -257,7 +257,7 @@ export const init = (rawOptions?: Options): ReactGrabAPI => {
257257
createEffect(
258258
on(isActivated, (activated, previousActivated) => {
259259
if (activated && !previousActivated) {
260-
freezePseudoStates();
260+
freezePseudoStates(store.pointer.x, store.pointer.y);
261261
freezeGlobalAnimations();
262262
document.body.style.touchAction = "none";
263263
// iOS Safari auto-zooms on focused inputs with font-size < 16px,
@@ -1624,7 +1624,7 @@ export const init = (rawOptions?: Options): ReactGrabAPI => {
16241624
) {
16251625
elementDetectionState.lastDetectionTimestamp = now;
16261626
elementDetectionState.pendingDetectionScheduledAt = now;
1627-
onIdle(() => {
1627+
setTimeout(() => {
16281628
const candidate = getElementAtPosition(
16291629
elementDetectionState.latestPointerX,
16301630
elementDetectionState.latestPointerY,

0 commit comments

Comments
 (0)