Repository navigation
Refine canvas trackpad navigation - #64
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds normalized wheel and Safari pinch gesture handling with bounded, pointer-centered zoom. It also updates blank-canvas selection clearing and focus-visible styling for canvas and diagram nodes. ChangesCanvas gesture handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant DiagramCanvas
participant CanvasWheelGesture
participant CanvasCamera
Browser->>DiagramCanvas: wheel or Safari gesture event
DiagramCanvas->>CanvasWheelGesture: convert eligible input
CanvasWheelGesture->>CanvasCamera: apply pan or pointer-centered zoom
DiagramCanvas-->>Browser: prevent default browser behavior
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/src/lib/canvas-wheel-gesture.test.ts (1)
45-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCompare the anchored point with a tolerance.
The anchored coordinates come from floating-point multiplication and subtraction (
camera.zoom * 1.05is not exact).toEqualrequires bit-exact equality, so this assertion can break after a small change to the zoom scale or camera values, even when the anchoring logic stays correct. UsetoBeCloseTofor each axis.♻️ Proposed refactor
- expect({ - x: origin.left + next.panX + (anchoredCanvasPoint.x * next.zoom), - y: origin.top + next.panY + (anchoredCanvasPoint.y * next.zoom), - }).toEqual(client); + expect(origin.left + next.panX + (anchoredCanvasPoint.x * next.zoom)).toBeCloseTo(client.x); + expect(origin.top + next.panY + (anchoredCanvasPoint.y * next.zoom)).toBeCloseTo(client.y);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/canvas-wheel-gesture.test.ts` around lines 45 - 48, Update the anchored-point assertion in the canvas wheel gesture test to compare the computed x and y coordinates independently with toBeCloseTo rather than using exact toEqual object equality. Preserve the existing expected coordinate formulas and client values while allowing normal floating-point rounding differences.apps/web/src/components/diagram-canvas.test.ts (1)
44-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider behavioral assertions instead of source-text regexes.
These assertions match the component source text and the stylesheet text. They break on formatting changes, such as a different argument order in
addEventListeneroptions or a line break inside the callback. They also pass when the behavior is broken, for example when the effect never attaches the listener. The e2e suite covers the trusted-gesture path, so a small DOM-level test would close the remaining gap: render the canvas, dispatch awheelevent withcancelable: true, and assertdefaultPreventedand the resulting transform. Keep the regex checks only for facts a DOM test cannot observe, such aspassive: false.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/diagram-canvas.test.ts` around lines 44 - 60, Replace the behavioral source-text regexes in the “canvas wheel ownership” test with a DOM-level test that renders the canvas, dispatches a cancelable wheel event, and asserts that the event is defaultPrevented and the canvas transform changes. Retain only the regex assertion needed to verify the native listener uses passive: false, and remove regex checks for callback structure or listener attachment behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/diagram-canvas.tsx`:
- Around line 1572-1605: Update handleSafariGestureStart and
handleSafariGestureChange to reject gesture.scale values that are not finite or
are less than or equal to zero. Apply the positive-scale validation in both
handlers before storing the scale or calculating the scale ratio, while
preserving the existing guard behavior for invalid gestures.
In `@e2e-workspace-ux-validate.ts`:
- Around line 495-533: Add camera restoration to the end of
expectWheelGestureCameraControls: click the exact “Fit diagram” button using the
existing verifiedClick helper, then call waitForStableCanvasTransform with the
label. Keep this cleanup after all wheel assertions so subsequent checks start
from the fitted, stable camera state.
---
Nitpick comments:
In `@apps/web/src/components/diagram-canvas.test.ts`:
- Around line 44-60: Replace the behavioral source-text regexes in the “canvas
wheel ownership” test with a DOM-level test that renders the canvas, dispatches
a cancelable wheel event, and asserts that the event is defaultPrevented and the
canvas transform changes. Retain only the regex assertion needed to verify the
native listener uses passive: false, and remove regex checks for callback
structure or listener attachment behavior.
In `@apps/web/src/lib/canvas-wheel-gesture.test.ts`:
- Around line 45-48: Update the anchored-point assertion in the canvas wheel
gesture test to compare the computed x and y coordinates independently with
toBeCloseTo rather than using exact toEqual object equality. Preserve the
existing expected coordinate formulas and client values while allowing normal
floating-point rounding differences.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d58904d0-e8f0-4281-8009-fd88331ddf84
📒 Files selected for processing (6)
apps/web/src/app/globals.cssapps/web/src/components/diagram-canvas.test.tsapps/web/src/components/diagram-canvas.tsxapps/web/src/lib/canvas-wheel-gesture.test.tsapps/web/src/lib/canvas-wheel-gesture.tse2e-workspace-ux-validate.ts
Comment on lines
+1572
to
+1605
| const handleSafariGestureStart = useCallback((event: Event) => { | ||
| const container = containerRef.current; | ||
| const gesture = event as SafariGestureEvent; | ||
| if (!container || !canHandleCanvasWheel(event.target, container) || !Number.isFinite(gesture.scale)) { | ||
| return; | ||
| } | ||
|
|
||
| event.preventDefault(); | ||
| safariPinchScaleRef.current = gesture.scale; | ||
| }, []); | ||
|
|
||
| const handleSafariGestureChange = useCallback((event: Event) => { | ||
| const container = containerRef.current; | ||
| const gesture = event as SafariGestureEvent; | ||
| const previousScale = safariPinchScaleRef.current; | ||
| if (!container || previousScale === null || !canHandleCanvasWheel(event.target, container) || !Number.isFinite(gesture.scale)) { | ||
| return; | ||
| } | ||
|
|
||
| event.preventDefault(); | ||
| safariPinchScaleRef.current = gesture.scale; | ||
| const rect = container.getBoundingClientRect(); | ||
| const client = { | ||
| x: Number.isFinite(gesture.clientX) ? gesture.clientX : rect.left + (rect.width / 2), | ||
| y: Number.isFinite(gesture.clientY) ? gesture.clientY : rect.top + (rect.height / 2), | ||
| }; | ||
|
|
||
| setAnimateTransform(false); | ||
| setViewport((current) => applyCanvasWheelGesture(current, { | ||
| client, | ||
| kind: 'zoom', | ||
| scale: Math.pow(gesture.scale / previousScale, 0.4), | ||
| }, rect, MIN_ZOOM, MAX_ZOOM)); | ||
| }, []); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Reject a non-positive Safari pinch scale.
handleSafariGestureStart accepts any finite gesture.scale, including 0. If previousScale is 0, then gesture.scale / previousScale is 0/0 = NaN or Infinity, and Math.pow(NaN, 0.4) is NaN. applyCanvasWheelGesture then produces NaN for zoom, panX, and panY, because clamp propagates NaN. The camera stays broken until a full remount, because every later gesture divides by the stored NaN scale. Require a positive scale in both handlers.
🛡️ Proposed fix
const handleSafariGestureStart = useCallback((event: Event) => {
const container = containerRef.current;
const gesture = event as SafariGestureEvent;
- if (!container || !canHandleCanvasWheel(event.target, container) || !Number.isFinite(gesture.scale)) {
+ if (!container || !canHandleCanvasWheel(event.target, container) || !(gesture.scale > 0) || !Number.isFinite(gesture.scale)) {
return;
}
@@
const previousScale = safariPinchScaleRef.current;
- if (!container || previousScale === null || !canHandleCanvasWheel(event.target, container) || !Number.isFinite(gesture.scale)) {
+ if (!container || previousScale === null || previousScale <= 0 || !canHandleCanvasWheel(event.target, container) || !(gesture.scale > 0) || !Number.isFinite(gesture.scale)) {
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleSafariGestureStart = useCallback((event: Event) => { | |
| const container = containerRef.current; | |
| const gesture = event as SafariGestureEvent; | |
| if (!container || !canHandleCanvasWheel(event.target, container) || !Number.isFinite(gesture.scale)) { | |
| return; | |
| } | |
| event.preventDefault(); | |
| safariPinchScaleRef.current = gesture.scale; | |
| }, []); | |
| const handleSafariGestureChange = useCallback((event: Event) => { | |
| const container = containerRef.current; | |
| const gesture = event as SafariGestureEvent; | |
| const previousScale = safariPinchScaleRef.current; | |
| if (!container || previousScale === null || !canHandleCanvasWheel(event.target, container) || !Number.isFinite(gesture.scale)) { | |
| return; | |
| } | |
| event.preventDefault(); | |
| safariPinchScaleRef.current = gesture.scale; | |
| const rect = container.getBoundingClientRect(); | |
| const client = { | |
| x: Number.isFinite(gesture.clientX) ? gesture.clientX : rect.left + (rect.width / 2), | |
| y: Number.isFinite(gesture.clientY) ? gesture.clientY : rect.top + (rect.height / 2), | |
| }; | |
| setAnimateTransform(false); | |
| setViewport((current) => applyCanvasWheelGesture(current, { | |
| client, | |
| kind: 'zoom', | |
| scale: Math.pow(gesture.scale / previousScale, 0.4), | |
| }, rect, MIN_ZOOM, MAX_ZOOM)); | |
| }, []); | |
| const handleSafariGestureStart = useCallback((event: Event) => { | |
| const container = containerRef.current; | |
| const gesture = event as SafariGestureEvent; | |
| if (!container || !canHandleCanvasWheel(event.target, container) || !(gesture.scale > 0) || !Number.isFinite(gesture.scale)) { | |
| return; | |
| } | |
| event.preventDefault(); | |
| safariPinchScaleRef.current = gesture.scale; | |
| }, []); | |
| const handleSafariGestureChange = useCallback((event: Event) => { | |
| const container = containerRef.current; | |
| const gesture = event as SafariGestureEvent; | |
| const previousScale = safariPinchScaleRef.current; | |
| if (!container || previousScale === null || previousScale <= 0 || !canHandleCanvasWheel(event.target, container) || !(gesture.scale > 0) || !Number.isFinite(gesture.scale)) { | |
| return; | |
| } | |
| event.preventDefault(); | |
| safariPinchScaleRef.current = gesture.scale; | |
| const rect = container.getBoundingClientRect(); | |
| const client = { | |
| x: Number.isFinite(gesture.clientX) ? gesture.clientX : rect.left + (rect.width / 2), | |
| y: Number.isFinite(gesture.clientY) ? gesture.clientY : rect.top + (rect.height / 2), | |
| }; | |
| setAnimateTransform(false); | |
| setViewport((current) => applyCanvasWheelGesture(current, { | |
| client, | |
| kind: 'zoom', | |
| scale: Math.pow(gesture.scale / previousScale, 0.4), | |
| }, rect, MIN_ZOOM, MAX_ZOOM)); | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/diagram-canvas.tsx` around lines 1572 - 1605, Update
handleSafariGestureStart and handleSafariGestureChange to reject gesture.scale
values that are not finite or are less than or equal to zero. Apply the
positive-scale validation in both handlers before storing the scale or
calculating the scale ratio, while preserving the existing guard behavior for
invalid gestures.
Comment on lines
+495
to
+533
| async function expectWheelGestureCameraControls(page: Page, label: string, renderer: 'flowchart' | 'generic'): Promise<void> { | ||
| const canvas = page.getByTestId('diagram-canvas'); | ||
| await canvas.waitFor({ state: 'visible', timeout: 15_000 }); | ||
| await verifiedClick(page, page.getByRole('button', { name: 'Zoom out', exact: true }), `${label} wheel headroom`); | ||
| await waitForStableCanvasTransform(page, `${label} wheel headroom`); | ||
| const beforePan = await readCanvasCameraSnapshot(page, `${label} wheel-pan baseline`); | ||
| const normalWheel = await dispatchTrustedCanvasWheel(page, label, renderer, { ctrlKey: false, deltaX: 18, deltaY: 32 }); | ||
| const afterPan = await readCanvasCameraSnapshot(page, `${label} wheel-pan result`); | ||
| assert(normalWheel.isTrusted && normalWheel.defaultPrevented, `${label} normal trusted wheel was not owned by the canvas.`); | ||
| assert(afterPan.zoom === beforePan.zoom && (afterPan.panX !== beforePan.panX || afterPan.panY !== beforePan.panY), | ||
| `${label} ordinary two-finger scroll did not pan without zooming: ${JSON.stringify({ beforePan, afterPan })}.`); | ||
|
|
||
| const beforeZoom = afterPan; | ||
| const browserBeforeZoom = await page.evaluate(() => ({ | ||
| browserScale: window.visualViewport?.scale ?? 1, | ||
| devicePixelRatio: window.devicePixelRatio, | ||
| })); | ||
| const pinch = await dispatchTrustedCanvasWheel(page, label, renderer, { ctrlKey: true, deltaX: 0, deltaY: -20 }); | ||
| const afterZoom = await readCanvasCameraSnapshot(page, `${label} ctrl-wheel result`); | ||
| const canvasBounds = await canvas.boundingBox(); | ||
| assert(canvasBounds, `${label} lost its canvas bounds during ctrl-wheel zoom.`); | ||
| const anchoredCanvasPoint = { | ||
| x: (pinch.point.x - canvasBounds.x - beforeZoom.panX) / beforeZoom.zoom, | ||
| y: (pinch.point.y - canvasBounds.y - beforeZoom.panY) / beforeZoom.zoom, | ||
| }; | ||
| const anchoredScreenPoint = { | ||
| x: afterZoom.panX + (anchoredCanvasPoint.x * afterZoom.zoom), | ||
| y: afterZoom.panY + (anchoredCanvasPoint.y * afterZoom.zoom), | ||
| }; | ||
| assert(pinch.isTrusted && pinch.defaultPrevented, `${label} trusted ctrl-wheel pinch was not cancelled before browser zoom.`); | ||
| assert(afterZoom.zoom > beforeZoom.zoom && afterZoom.zoom < beforeZoom.zoom * 1.1, | ||
| `${label} ctrl-wheel zoom was missing or too sensitive: ${JSON.stringify({ beforeZoom, afterZoom })}.`); | ||
| assert(pinch.browserScale === 1 && browserBeforeZoom.browserScale === 1 | ||
| && pinch.devicePixelRatio === browserBeforeZoom.devicePixelRatio, | ||
| `${label} ctrl-wheel changed the browser viewport scale: ${JSON.stringify({ browserBeforeZoom, pinch })}.`); | ||
| assert(Math.abs(anchoredScreenPoint.x - (pinch.point.x - canvasBounds.x)) < 0.1 | ||
| && Math.abs(anchoredScreenPoint.y - (pinch.point.y - canvasBounds.y)) < 0.1, | ||
| `${label} ctrl-wheel zoom did not keep its blank-pane cursor point anchored: ${JSON.stringify({ afterZoom, anchoredScreenPoint, pinch })}.`); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Restore the camera before the next check.
expectWheelGestureCameraControls clicks "Zoom out", then pans and zooms with wheel input. It does not restore the camera. The nearby helper expectCanvasCameraControlHitTargets ends with a "Fit diagram" click for this reason (Line 392). Later checks then run against a panned and scaled camera. For example, expectBlankCanvasClickClearsSelection clicks .mermaid-flow-node first, which can sit outside the visible canvas after the pan, and allowedCanvasGesturePoints samples ratio-based points that may no longer be blank. Add a "Fit diagram" click plus waitForStableCanvasTransform at the end of the helper.
🛡️ Proposed fix
assert(Math.abs(anchoredScreenPoint.x - (pinch.point.x - canvasBounds.x)) < 0.1
&& Math.abs(anchoredScreenPoint.y - (pinch.point.y - canvasBounds.y)) < 0.1,
`${label} ctrl-wheel zoom did not keep its blank-pane cursor point anchored: ${JSON.stringify({ afterZoom, anchoredScreenPoint, pinch })}.`);
+ await verifiedClick(page, page.getByRole('button', { name: 'Fit diagram', exact: true }), `${label} wheel camera reset`);
+ await waitForStableCanvasTransform(page, `${label} wheel camera reset`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function expectWheelGestureCameraControls(page: Page, label: string, renderer: 'flowchart' | 'generic'): Promise<void> { | |
| const canvas = page.getByTestId('diagram-canvas'); | |
| await canvas.waitFor({ state: 'visible', timeout: 15_000 }); | |
| await verifiedClick(page, page.getByRole('button', { name: 'Zoom out', exact: true }), `${label} wheel headroom`); | |
| await waitForStableCanvasTransform(page, `${label} wheel headroom`); | |
| const beforePan = await readCanvasCameraSnapshot(page, `${label} wheel-pan baseline`); | |
| const normalWheel = await dispatchTrustedCanvasWheel(page, label, renderer, { ctrlKey: false, deltaX: 18, deltaY: 32 }); | |
| const afterPan = await readCanvasCameraSnapshot(page, `${label} wheel-pan result`); | |
| assert(normalWheel.isTrusted && normalWheel.defaultPrevented, `${label} normal trusted wheel was not owned by the canvas.`); | |
| assert(afterPan.zoom === beforePan.zoom && (afterPan.panX !== beforePan.panX || afterPan.panY !== beforePan.panY), | |
| `${label} ordinary two-finger scroll did not pan without zooming: ${JSON.stringify({ beforePan, afterPan })}.`); | |
| const beforeZoom = afterPan; | |
| const browserBeforeZoom = await page.evaluate(() => ({ | |
| browserScale: window.visualViewport?.scale ?? 1, | |
| devicePixelRatio: window.devicePixelRatio, | |
| })); | |
| const pinch = await dispatchTrustedCanvasWheel(page, label, renderer, { ctrlKey: true, deltaX: 0, deltaY: -20 }); | |
| const afterZoom = await readCanvasCameraSnapshot(page, `${label} ctrl-wheel result`); | |
| const canvasBounds = await canvas.boundingBox(); | |
| assert(canvasBounds, `${label} lost its canvas bounds during ctrl-wheel zoom.`); | |
| const anchoredCanvasPoint = { | |
| x: (pinch.point.x - canvasBounds.x - beforeZoom.panX) / beforeZoom.zoom, | |
| y: (pinch.point.y - canvasBounds.y - beforeZoom.panY) / beforeZoom.zoom, | |
| }; | |
| const anchoredScreenPoint = { | |
| x: afterZoom.panX + (anchoredCanvasPoint.x * afterZoom.zoom), | |
| y: afterZoom.panY + (anchoredCanvasPoint.y * afterZoom.zoom), | |
| }; | |
| assert(pinch.isTrusted && pinch.defaultPrevented, `${label} trusted ctrl-wheel pinch was not cancelled before browser zoom.`); | |
| assert(afterZoom.zoom > beforeZoom.zoom && afterZoom.zoom < beforeZoom.zoom * 1.1, | |
| `${label} ctrl-wheel zoom was missing or too sensitive: ${JSON.stringify({ beforeZoom, afterZoom })}.`); | |
| assert(pinch.browserScale === 1 && browserBeforeZoom.browserScale === 1 | |
| && pinch.devicePixelRatio === browserBeforeZoom.devicePixelRatio, | |
| `${label} ctrl-wheel changed the browser viewport scale: ${JSON.stringify({ browserBeforeZoom, pinch })}.`); | |
| assert(Math.abs(anchoredScreenPoint.x - (pinch.point.x - canvasBounds.x)) < 0.1 | |
| && Math.abs(anchoredScreenPoint.y - (pinch.point.y - canvasBounds.y)) < 0.1, | |
| `${label} ctrl-wheel zoom did not keep its blank-pane cursor point anchored: ${JSON.stringify({ afterZoom, anchoredScreenPoint, pinch })}.`); | |
| } | |
| async function expectWheelGestureCameraControls(page: Page, label: string, renderer: 'flowchart' | 'generic'): Promise<void> { | |
| const canvas = page.getByTestId('diagram-canvas'); | |
| await canvas.waitFor({ state: 'visible', timeout: 15_000 }); | |
| await verifiedClick(page, page.getByRole('button', { name: 'Zoom out', exact: true }), `${label} wheel headroom`); | |
| await waitForStableCanvasTransform(page, `${label} wheel headroom`); | |
| const beforePan = await readCanvasCameraSnapshot(page, `${label} wheel-pan baseline`); | |
| const normalWheel = await dispatchTrustedCanvasWheel(page, label, renderer, { ctrlKey: false, deltaX: 18, deltaY: 32 }); | |
| const afterPan = await readCanvasCameraSnapshot(page, `${label} wheel-pan result`); | |
| assert(normalWheel.isTrusted && normalWheel.defaultPrevented, `${label} normal trusted wheel was not owned by the canvas.`); | |
| assert(afterPan.zoom === beforePan.zoom && (afterPan.panX !== beforePan.panX || afterPan.panY !== beforePan.panY), | |
| `${label} ordinary two-finger scroll did not pan without zooming: ${JSON.stringify({ beforePan, afterPan })}.`); | |
| const beforeZoom = afterPan; | |
| const browserBeforeZoom = await page.evaluate(() => ({ | |
| browserScale: window.visualViewport?.scale ?? 1, | |
| devicePixelRatio: window.devicePixelRatio, | |
| })); | |
| const pinch = await dispatchTrustedCanvasWheel(page, label, renderer, { ctrlKey: true, deltaX: 0, deltaY: -20 }); | |
| const afterZoom = await readCanvasCameraSnapshot(page, `${label} ctrl-wheel result`); | |
| const canvasBounds = await canvas.boundingBox(); | |
| assert(canvasBounds, `${label} lost its canvas bounds during ctrl-wheel zoom.`); | |
| const anchoredCanvasPoint = { | |
| x: (pinch.point.x - canvasBounds.x - beforeZoom.panX) / beforeZoom.zoom, | |
| y: (pinch.point.y - canvasBounds.y - beforeZoom.panY) / beforeZoom.zoom, | |
| }; | |
| const anchoredScreenPoint = { | |
| x: afterZoom.panX + (anchoredCanvasPoint.x * afterZoom.zoom), | |
| y: afterZoom.panY + (anchoredCanvasPoint.y * afterZoom.zoom), | |
| }; | |
| assert(pinch.isTrusted && pinch.defaultPrevented, `${label} trusted ctrl-wheel pinch was not cancelled before browser zoom.`); | |
| assert(afterZoom.zoom > beforeZoom.zoom && afterZoom.zoom < beforeZoom.zoom * 1.1, | |
| `${label} ctrl-wheel zoom was missing or too sensitive: ${JSON.stringify({ beforeZoom, afterZoom })}.`); | |
| assert(pinch.browserScale === 1 && browserBeforeZoom.browserScale === 1 | |
| && pinch.devicePixelRatio === browserBeforeZoom.devicePixelRatio, | |
| `${label} ctrl-wheel changed the browser viewport scale: ${JSON.stringify({ browserBeforeZoom, pinch })}.`); | |
| assert(Math.abs(anchoredScreenPoint.x - (pinch.point.x - canvasBounds.x)) < 0.1 | |
| && Math.abs(anchoredScreenPoint.y - (pinch.point.y - canvasBounds.y)) < 0.1, | |
| `${label} ctrl-wheel zoom did not keep its blank-pane cursor point anchored: ${JSON.stringify({ afterZoom, anchoredScreenPoint, pinch })}.`); | |
| await verifiedClick(page, page.getByRole('button', { name: 'Fit diagram', exact: true }), `${label} wheel camera reset`); | |
| await waitForStableCanvasTransform(page, `${label} wheel camera reset`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e-workspace-ux-validate.ts` around lines 495 - 533, Add camera restoration
to the end of expectWheelGestureCameraControls: click the exact “Fit diagram”
button using the existing verifiedClick helper, then call
waitForStableCanvasTransform with the label. Keep this cleanup after all wheel
assertions so subsequent checks start from the fitted, stable camera state.
donovan-yohan
force-pushed
the
codex/canvas-trackpad-gestures
branch
from
August 10, 2026 16:09
bf3cd96 to
9879664
Compare
This was referenced Aug 10, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Root cause
The canvas treated every wheel event as a fixed 10% zoom step. Its roving keyboard focus target was also painted like selection, so clearing selected IDs still left a default node looking active.
Impact
MacBook trackpad scrolling now behaves like a canvas pan, pinch zoom is controlled, blank clicks clear selection, and keyboard focus remains accessible without the panning outline artifact.
Exact-head evidence
Head:
bf3cd96pnpm --filter @arielcharts/shared buildpnpm lintpnpm typecheckpnpm test— 302 tests passedpnpm buildnpx tsx e2e-workspace-ux-validate.tsnpx tsx e2e-validate.tspnpm test:e2e-sequencegit diff --checkThe workspace UX gate uses trusted
page.mouse.wheelinput at verified React Flow and generic Mermaid canvas targets, checksdefaultPrevented, pan-only scroll, gentle cursor-anchored pinch zoom, browser viewport-scale stability, blank-pane deselection, and focus styling.Residual platform note
macOS Accessibility Zoom may intercept gestures before the browser. The app suppresses cancellable browser-level gesture defaults; a physical Mac/Safari pass remains the final platform-specific confirmation.
Summary by CodeRabbit
New Features
Bug Fixes