-
Notifications
You must be signed in to change notification settings - Fork 310
feat: box model overlay for inspect mode #285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 10 commits
f1296f1
ba9f4c3
9d88d82
e26b0d7
87af2fc
20b2642
a481586
8be7680
1baf7f0
9ad96a1
6d82e2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,3 +17,4 @@ playwright-report | |
| meta.json | ||
| .agents/skills | ||
| .claude.expect | ||
| .mcp.json | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import { createEffect, onCleanup, onMount, on } from "solid-js"; | ||
| import type { Component } from "solid-js"; | ||
| import type { OverlayBounds, SelectionLabelInstance } from "../types.js"; | ||
| import type { BoxModelBounds, OverlayBounds, SelectionLabelInstance } from "../types.js"; | ||
| import { lerp } from "../utils/lerp.js"; | ||
| import { | ||
| SELECTION_LERP_FACTOR, | ||
|
|
@@ -15,8 +15,15 @@ | |
| OPACITY_CONVERGENCE_THRESHOLD, | ||
| OVERLAY_BORDER_COLOR_DEFAULT, | ||
| OVERLAY_FILL_COLOR_DEFAULT, | ||
| OVERLAY_BORDER_COLOR_INSPECT, | ||
| OVERLAY_FILL_COLOR_INSPECT, | ||
| BOX_MODEL_MARGIN_HATCH_COLOR, | ||
| BOX_MODEL_PADDING_FILL_COLOR, | ||
| BOX_MODEL_CONTENT_FILL_COLOR, | ||
| BOX_MODEL_GAP_HATCH_COLOR, | ||
| HATCH_PATTERN_WIDTH_PX, | ||
| HATCH_DASH_LENGTH_PX, | ||
| HATCH_DASH_GAP_PX, | ||
| HATCH_LINE_WIDTH_PX, | ||
| HATCH_ROTATION_DEG, | ||
| } from "../constants.js"; | ||
| import { nativeCancelAnimationFrame, nativeRequestAnimationFrame } from "../utils/native-raf.js"; | ||
| import { supportsDisplayP3 } from "../utils/supports-display-p3.js"; | ||
|
|
@@ -27,12 +34,6 @@ | |
| lerpFactor: SELECTION_LERP_FACTOR, | ||
| } as const; | ||
|
|
||
| const INSPECT_LAYER_STYLE = { | ||
| borderColor: OVERLAY_BORDER_COLOR_INSPECT, | ||
| fillColor: OVERLAY_FILL_COLOR_INSPECT, | ||
| lerpFactor: SELECTION_LERP_FACTOR, | ||
| } as const; | ||
|
|
||
| const LAYER_STYLES = { | ||
| drag: { | ||
| borderColor: OVERLAY_BORDER_COLOR_DRAG, | ||
|
|
@@ -41,10 +42,10 @@ | |
| }, | ||
| selection: DEFAULT_LAYER_STYLE, | ||
| grabbed: DEFAULT_LAYER_STYLE, | ||
| inspect: INSPECT_LAYER_STYLE, | ||
| } as const; | ||
|
|
||
| type LayerName = "drag" | "selection" | "grabbed" | "inspect"; | ||
| type BoxModelLayerName = "margin" | "border" | "padding" | "content"; | ||
|
|
||
| interface OffscreenLayer { | ||
| canvas: OffscreenCanvas | null; | ||
|
|
@@ -55,7 +56,7 @@ | |
| id: string; | ||
| current: { x: number; y: number; width: number; height: number }; | ||
| target: { x: number; y: number; width: number; height: number }; | ||
| borderRadius: number; | ||
| borderRadii: number[]; | ||
| opacity: number; | ||
| targetOpacity: number; | ||
| createdAt?: number; | ||
|
|
@@ -71,6 +72,7 @@ | |
|
|
||
| inspectVisible?: boolean; | ||
| inspectBounds?: OverlayBounds[]; | ||
| inspectBoxModel?: BoxModelBounds; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| dragVisible?: boolean; | ||
| dragBounds?: OverlayBounds; | ||
|
|
@@ -85,7 +87,7 @@ | |
| } | ||
|
|
||
| export const OverlayCanvas: Component<OverlayCanvasProps> = (props) => { | ||
| let canvasRef: HTMLCanvasElement | undefined; | ||
| let mainContext: CanvasRenderingContext2D | null = null; | ||
| let canvasWidth = 0; | ||
| let canvasHeight = 0; | ||
|
|
@@ -102,7 +104,7 @@ | |
| let selectionAnimations: AnimatedBounds[] = []; | ||
| let dragAnimation: AnimatedBounds | null = null; | ||
| let grabbedAnimations: AnimatedBounds[] = []; | ||
| let inspectAnimations: AnimatedBounds[] = []; | ||
| let boxModelAnimations: Partial<Record<BoxModelLayerName, AnimatedBounds>> = {}; | ||
|
|
||
| const canvasColorSpace: PredefinedColorSpace = supportsDisplayP3() ? "display-p3" : "srgb"; | ||
|
|
||
|
|
@@ -141,10 +143,12 @@ | |
| } | ||
| }; | ||
|
|
||
| const parseBorderRadiusValue = (borderRadius: string): number => { | ||
| if (!borderRadius) return 0; | ||
| const match = borderRadius.match(/^(\d+(?:\.\d+)?)/); | ||
| return match ? parseFloat(match[1]) : 0; | ||
| const parseBorderRadii = (borderRadius: string): number[] => { | ||
| if (!borderRadius) return [0, 0, 0, 0]; | ||
| const radiusString = borderRadius.split("/")[0].trim(); | ||
| const values = radiusString.split(/\s+/).map((value) => parseFloat(value) || 0); | ||
| const [topLeft = 0, topRight = topLeft, bottomRight = topLeft, bottomLeft = topRight] = values; | ||
| return [topLeft, topRight, bottomRight, bottomLeft]; | ||
| }; | ||
|
|
||
| const createAnimatedBounds = ( | ||
|
|
@@ -165,7 +169,7 @@ | |
| width: bounds.width, | ||
| height: bounds.height, | ||
| }, | ||
| borderRadius: parseBorderRadiusValue(bounds.borderRadius), | ||
| borderRadii: parseBorderRadii(bounds.borderRadius), | ||
| opacity: options?.opacity ?? 1, | ||
| targetOpacity: options?.targetOpacity ?? options?.opacity ?? 1, | ||
| createdAt: options?.createdAt, | ||
|
|
@@ -183,7 +187,7 @@ | |
| width: bounds.width, | ||
| height: bounds.height, | ||
| }; | ||
| animation.borderRadius = parseBorderRadiusValue(bounds.borderRadius); | ||
| animation.borderRadii = parseBorderRadii(bounds.borderRadius); | ||
| if (targetOpacity !== undefined) { | ||
| animation.targetOpacity = targetOpacity; | ||
| } | ||
|
|
@@ -192,29 +196,27 @@ | |
| const resolveBoundsArray = (instance: SelectionLabelInstance): OverlayBounds[] => | ||
| instance.boundsMultiple ?? [instance.bounds]; | ||
|
|
||
| const clampRadii = (radii: number[], halfWidth: number, halfHeight: number): number[] => | ||
| radii.map((radius) => Math.min(radius, halfWidth, halfHeight)); | ||
|
|
||
| const drawRoundedRectangle = ( | ||
| context: OffscreenCanvasRenderingContext2D, | ||
| rectX: number, | ||
| rectY: number, | ||
| rectWidth: number, | ||
| rectHeight: number, | ||
| cornerRadius: number, | ||
| cornerRadii: number[], | ||
| fillColor: string, | ||
| strokeColor: string, | ||
| opacity: number = 1, | ||
| ) => { | ||
| if (rectWidth <= 0 || rectHeight <= 0) return; | ||
|
|
||
| const maxCornerRadius = Math.min(rectWidth / 2, rectHeight / 2); | ||
| const clampedCornerRadius = Math.min(cornerRadius, maxCornerRadius); | ||
| const clamped = clampRadii(cornerRadii, rectWidth / 2, rectHeight / 2); | ||
|
|
||
| context.globalAlpha = opacity; | ||
| context.beginPath(); | ||
| if (clampedCornerRadius > 0) { | ||
| context.roundRect(rectX, rectY, rectWidth, rectHeight, clampedCornerRadius); | ||
| } else { | ||
| context.rect(rectX, rectY, rectWidth, rectHeight); | ||
| } | ||
| context.roundRect(rectX, rectY, rectWidth, rectHeight, clamped); | ||
| context.fillStyle = fillColor; | ||
| context.fill(); | ||
| context.strokeStyle = strokeColor; | ||
|
|
@@ -239,7 +241,7 @@ | |
| dragAnimation.current.y, | ||
| dragAnimation.current.width, | ||
| dragAnimation.current.height, | ||
| dragAnimation.borderRadius, | ||
| dragAnimation.borderRadii, | ||
| style.fillColor, | ||
| style.borderColor, | ||
| ); | ||
|
|
@@ -264,7 +266,7 @@ | |
| animation.current.y, | ||
| animation.current.width, | ||
| animation.current.height, | ||
| animation.borderRadius, | ||
| animation.borderRadii, | ||
| style.fillColor, | ||
| style.borderColor, | ||
| effectiveOpacity, | ||
|
|
@@ -291,14 +293,110 @@ | |
| animation.current.y, | ||
| animation.current.width, | ||
| animation.current.height, | ||
| animation.borderRadius, | ||
| animation.borderRadii, | ||
| style.fillColor, | ||
| style.borderColor, | ||
| animation.opacity, | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| const hatchPatternCache = new Map<string, CanvasPattern>(); | ||
|
|
||
| const getOrCreateHatchPattern = ( | ||
| context: OffscreenCanvasRenderingContext2D, | ||
| color: string, | ||
| ): CanvasPattern | null => { | ||
| const cached = hatchPatternCache.get(color); | ||
| if (cached) return cached; | ||
|
|
||
| const patternCanvas = new OffscreenCanvas( | ||
| HATCH_PATTERN_WIDTH_PX, | ||
| HATCH_DASH_LENGTH_PX + HATCH_DASH_GAP_PX, | ||
| ); | ||
| const patternContext = patternCanvas.getContext("2d"); | ||
| if (!patternContext) return null; | ||
|
|
||
| patternContext.clearRect(0, 0, patternCanvas.width, patternCanvas.height); | ||
| patternContext.fillStyle = color; | ||
| patternContext.fillRect(0, 0, HATCH_LINE_WIDTH_PX, HATCH_DASH_LENGTH_PX); | ||
|
|
||
| const pattern = context.createPattern(patternCanvas, "repeat"); | ||
| if (pattern) { | ||
| pattern.setTransform(new DOMMatrix().rotate(HATCH_ROTATION_DEG)); | ||
vercel[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| hatchPatternCache.set(color, pattern); | ||
| } | ||
| return pattern; | ||
| }; | ||
|
|
||
| // Chromium bug: mixing roundRect and rect sub-paths on the same Path2D | ||
| // breaks the "evenodd" fill rule, clipping the top-left of the ring. | ||
| // Always use roundRect (even with [0,0,0,0] radii) to keep both | ||
| // sub-paths using the same drawing primitive. | ||
| const appendBoundsToPath = (path: Path2D, animation: AnimatedBounds) => { | ||
| const { x, y, width, height } = animation.current; | ||
| if (width <= 0 || height <= 0) return; | ||
| const clamped = clampRadii(animation.borderRadii, width / 2, height / 2); | ||
| path.roundRect(x, y, width, height, clamped); | ||
| }; | ||
|
|
||
| const buildBoundsPath = (animation: AnimatedBounds): Path2D => { | ||
| const path = new Path2D(); | ||
| appendBoundsToPath(path, animation); | ||
| return path; | ||
| }; | ||
|
|
||
| const buildRingPath = (outer: AnimatedBounds, inner: AnimatedBounds): Path2D => { | ||
| const path = new Path2D(); | ||
| appendBoundsToPath(path, outer); | ||
| appendBoundsToPath(path, inner); | ||
| return path; | ||
| }; | ||
|
|
||
| const fillWithHatch = ( | ||
| context: OffscreenCanvasRenderingContext2D, | ||
| path: Path2D, | ||
| color: string, | ||
| ) => { | ||
| const pattern = getOrCreateHatchPattern(context, color); | ||
| if (!pattern) return; | ||
| context.fillStyle = pattern; | ||
| context.fill(path, "evenodd"); | ||
vercel[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }; | ||
|
|
||
| const renderInspectLayer = () => { | ||
| const layer = layers.inspect; | ||
| if (!layer.context) return; | ||
|
|
||
| const context = layer.context; | ||
| context.clearRect(0, 0, canvasWidth, canvasHeight); | ||
|
|
||
| if (!props.inspectVisible) return; | ||
|
|
||
| const { margin, border, padding, content } = boxModelAnimations; | ||
| if (!margin || !border || !padding || !content) return; | ||
|
|
||
| fillWithHatch(context, buildRingPath(margin, border), BOX_MODEL_MARGIN_HATCH_COLOR); | ||
|
|
||
| context.fillStyle = BOX_MODEL_PADDING_FILL_COLOR; | ||
| context.fill(buildRingPath(padding, content), "evenodd"); | ||
|
|
||
| const contentPath = buildBoundsPath(content); | ||
| context.fillStyle = BOX_MODEL_CONTENT_FILL_COLOR; | ||
| context.fill(contentPath); | ||
|
|
||
| const gapRects = props.inspectBoxModel?.gaps; | ||
| if (gapRects && gapRects.length > 0) { | ||
| const gapPath = new Path2D(); | ||
| for (const gapRect of gapRects) { | ||
| if (gapRect.width > 0 && gapRect.height > 0) { | ||
| gapPath.rect(gapRect.x, gapRect.y, gapRect.width, gapRect.height); | ||
| } | ||
| } | ||
| fillWithHatch(context, gapPath, BOX_MODEL_GAP_HATCH_COLOR); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gap rects not animated, visually desync during transitionsMedium Severity Gap rects in Additional Locations (1)Reviewed by Cursor Bugbot for commit e7ea6dc. Configure here.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Grid gap hatching creates holes at intersectionsMedium Severity For grid containers, Additional Locations (2)Reviewed by Cursor Bugbot for commit 17c5eae. Configure here. |
||
| }; | ||
|
|
||
| const compositeAllLayers = () => { | ||
| if (!mainContext || !canvasRef) return; | ||
|
|
||
|
|
@@ -309,7 +407,7 @@ | |
| renderDragLayer(); | ||
| renderSelectionLayer(); | ||
| renderBoundsLayer("grabbed", grabbedAnimations); | ||
| renderBoundsLayer("inspect", inspectAnimations); | ||
| renderInspectLayer(); | ||
|
|
||
| const layerRenderOrder: LayerName[] = ["inspect", "drag", "selection", "grabbed"]; | ||
| for (const layerName of layerRenderOrder) { | ||
|
|
@@ -411,9 +509,9 @@ | |
| return animation.opacity > 0; | ||
| }); | ||
|
|
||
| for (const animation of inspectAnimations) { | ||
| for (const animation of Object.values(boxModelAnimations)) { | ||
| if (animation.isInitialized) { | ||
| if (interpolateBounds(animation, LAYER_STYLES.inspect.lerpFactor)) { | ||
| if (interpolateBounds(animation, SELECTION_LERP_FACTOR)) { | ||
| shouldContinueAnimating = true; | ||
| } | ||
| } | ||
|
|
@@ -580,27 +678,24 @@ | |
|
|
||
| createEffect( | ||
| on( | ||
| () => [props.inspectVisible, props.inspectBounds] as const, | ||
| ([isVisible, bounds]) => { | ||
| if (!isVisible || !bounds || bounds.length === 0) { | ||
| inspectAnimations = []; | ||
| () => [props.inspectVisible, props.inspectBoxModel] as const, | ||
| ([isVisible, boxModel]) => { | ||
| if (!isVisible || !boxModel) { | ||
| boxModelAnimations = {}; | ||
| scheduleAnimationFrame(); | ||
| return; | ||
| } | ||
|
|
||
| inspectAnimations = bounds.map((ancestorBounds, index) => { | ||
| const animationId = `inspect-${index}`; | ||
| const existingAnimation = inspectAnimations.find( | ||
| (animation) => animation.id === animationId, | ||
| ); | ||
|
|
||
| if (existingAnimation) { | ||
| updateAnimationTarget(existingAnimation, ancestorBounds); | ||
| return existingAnimation; | ||
| const layers = ["margin", "border", "padding", "content"] as const; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Variable shadows outer offscreen layers recordLow Severity The Reviewed by Cursor Bugbot for commit 17c5eae. Configure here. |
||
| for (const layer of layers) { | ||
|
Comment on lines
+689
to
+690
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| const bounds = boxModel[layer]; | ||
| const existing = boxModelAnimations[layer]; | ||
| if (existing) { | ||
| updateAnimationTarget(existing, bounds); | ||
| } else { | ||
| boxModelAnimations[layer] = createAnimatedBounds(`boxmodel-${layer}`, bounds); | ||
| } | ||
|
|
||
| return createAnimatedBounds(animationId, ancestorBounds); | ||
| }); | ||
| } | ||
|
|
||
| scheduleAnimationFrame(); | ||
| }, | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused
inspectBoundsprop after refactor to box modelLow Severity
The
inspectBoundsprop inOverlayCanvasPropsis declared and passed through the component hierarchy but never consumed. ThecreateEffectthat previously readprops.inspectBoundswas replaced with one that readsprops.inspectBoxModel, making the prop dead code. TheinspectBounds()memo incore/index.tsxstill runscreateElementBoundson every ancestor each time the viewport changes, solely to power theinspectBounds().length > 0visibility check — which could use the already-existinginspectAncestorElements().length > 0instead, avoiding that unnecessary computation.Additional Locations (2)
packages/react-grab/src/components/renderer.tsx#L30-L31packages/react-grab/src/core/index.tsx#L3651-L3652Reviewed by Cursor Bugbot for commit b9a52c1. Configure here.