Grida Canvas - Parametric Scale (K)#471
Merged
softmarshmallow merged 23 commits intomainfrom Dec 14, 2025
Merged
Conversation
WalkthroughAdds a new parametric Scale tool (hotkey K) with editor API, reducer handlers, gesture orchestration, schema-based node scaling, UI controls, cursor assets, and tests to perform non-destructive uniform scaling of selected nodes with optional subtree and space options. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Scale Tool UI
participant API as Editor API (applyScale)
participant Reducer as Document Reducer / Methods
participant Schema as parametric_scale
participant State as Editor State
User->>UI: choose targets, origin, factor
UI->>API: applyScale(targets, factor, { origin, include_subtree, space })
API->>Reducer: dispatch({ type: "apply-scale", ... })
Reducer->>Reducer: self_apply_scale_by_factor(targets, factor, origin, include_subtree, space)
Reducer->>Schema: schema.parametric_scale.apply_node(node, factor)
Schema->>Schema: scale numeric fields, preserve non-numeric
Schema->>Reducer: return updated authored properties
Reducer->>State: commit node updates
State->>UI: reflect changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas to focus review on:
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/wg/feat-authoring/parametric-scaling.md (1)
86-141: Docs: fix “no anchor” vs “relative to anchor” inconsistency + remove accidental escaping
The text says “no anchor”, but the table still references anchor-based scaling; also\"auto\"will render with backslashes.-| `left`, `top`, `right`, `bottom` | layout | Y | Absolute/offset lengths; scale relative to anchor. | +| `left`, `top`, `right`, `bottom` | layout | Y | Absolute/offset lengths; scale stored numeric values (no reflow / no anchor rewrite). | -| `width`, `height` | layout | Y\* | Scale only numeric/px-like lengths. Do not scale `%`, viewport units, or `\"auto\"`. | +| `width`, `height` | layout | Y\* | Scale only numeric/px-like lengths. Do not scale `%`, viewport units, or `"auto"`. |(Style hint around Line 98 feels optional; I’d prioritize consistency over wording tweaks.)
editor/grida-canvas-react/viewport/ui/floating-bar.tsx (1)
11-32: Addtop-0to anchor the absolute title bar positioning
Without an explicittopvalue, the element defaults totop: auto, making its position layout-dependent. Addingtop-0ensures predictable positioning above the parent.- <div className="absolute left-0 right-0 -translate-y-full"> + <div className="absolute top-0 left-0 right-0 -translate-y-full"> <div {...porps} className={cn("max-w-full overflow-hidden", className)}> {children} </div> </div>
🧹 Nitpick comments (10)
packages/grida-canvas-vn/tsconfig.json (1)
7-8: Remove redundantnoImplicitAnywhenstrictis enabled.The
strictcompiler option subsumesnoImplicitAny, making the explicit declaration redundant.Apply this diff to clean up the configuration:
"compilerOptions": { "esModuleInterop": true, "module": "ESNext", "moduleResolution": "Bundler", "target": "ES2020", - "noImplicitAny": true, "strict": true },packages/grida-canvas-vn/jest.config.ts (1)
3-15: Fix coverage exclude glob + verify ESM preset works with your Jest/ts-jest versions.
"!dist/**"likely won’t excludepackages/**/dist/**; consider!**/dist/**.Also please verify
preset: "ts-jest/presets/default-esm"+extensionsToTreatAsEsmis sufficient for your current Jest + ts-jest versions (and that transforms behave as intended with ESM-only deps likesvg-pathdata).collectCoverageFrom: [ "**/*.ts", "!**/*.d.ts", "!**/node_modules/**", - "!dist/**", + "!**/dist/**", ],editor/components/cursor/cursor-data.ts (1)
45-53: Use directional fallback keywords for scale cursors too (notauto).For
resize_handle_scale_cursor_map, falling back to"auto"may regress UX if the PNG can’t be loaded; consider using the directional resize keywords (like you do inresize_handle_cursor_map) so behavior stays correct even on asset load failure.export const resize_handle_scale_cursor_map = { - nw: pngsetcss(_nwse_resize_scale_png_url, 32, 32, "auto"), - n: pngsetcss(_ns_resize_scale_png_url, 32, 32, "auto"), - ne: pngsetcss(_nesw_resize_scale_png_url, 32, 32, "auto"), - e: pngsetcss(_ew_resize_scale_png_url, 32, 32, "auto"), - se: pngsetcss(_nwse_resize_scale_png_url, 32, 32, "auto"), - s: pngsetcss(_ns_resize_scale_png_url, 32, 32, "auto"), - sw: pngsetcss(_nesw_resize_scale_png_url, 32, 32, "auto"), - w: pngsetcss(_ew_resize_scale_png_url, 32, 32, "auto"), + nw: pngsetcss(_nwse_resize_scale_png_url, 32, 32, "nwse-resize"), + n: pngsetcss(_ns_resize_scale_png_url, 32, 32, "ns-resize"), + ne: pngsetcss(_nesw_resize_scale_png_url, 32, 32, "nesw-resize"), + e: pngsetcss(_ew_resize_scale_png_url, 32, 32, "ew-resize"), + se: pngsetcss(_nwse_resize_scale_png_url, 32, 32, "nwse-resize"), + s: pngsetcss(_ns_resize_scale_png_url, 32, 32, "ns-resize"), + sw: pngsetcss(_nesw_resize_scale_png_url, 32, 32, "nesw-resize"), + w: pngsetcss(_ew_resize_scale_png_url, 32, 32, "ew-resize"), } as const;Also applies to: 89-123
editor/scaffolds/sidecontrol/controls/alignment9.tsx (1)
12-25: MakeALIGNMENTSimmutable and type-checked.Helps avoid accidental mutation and keeps the literal union intact.
-const ALIGNMENTS: Array<{ +const ALIGNMENTS = [ + { key: "top-left", label: "Top left" }, + { key: "top-center", label: "Top" }, + { key: "top-right", label: "Top right" }, + { key: "center-left", label: "Left" }, + { key: "center", label: "Center" }, + { key: "center-right", label: "Right" }, + { key: "bottom-left", label: "Bottom left" }, + { key: "bottom-center", label: "Bottom" }, + { key: "bottom-right", label: "Bottom right" }, +] as const satisfies ReadonlyArray<{ key: cmath.Alignment9; label: string }>; -}> = [ - { key: "top-left", label: "Top left" }, - { key: "top-center", label: "Top" }, - { key: "top-right", label: "Top right" }, - { key: "center-left", label: "Left" }, - { key: "center", label: "Center" }, - { key: "center-right", label: "Right" }, - { key: "bottom-left", label: "Bottom left" }, - { key: "bottom-center", label: "Bottom" }, - { key: "bottom-right", label: "Bottom right" }, -];editor/scaffolds/sidecontrol/controls/scale-factor.tsx (1)
41-44: Potentially redundant spinner-hiding CSS.The
[appearance:textfield]and webkit spinner overrides may be redundant here sinceInputPropertyNumberalready applies these styles internally whenappearance="none"(the default). Consider removing these classes to reduce duplication.className={cn( "overflow-hidden", - "[appearance:textfield] [&::-webkit-outer-spin-button]:appearance-none [&::-webkit-inner-spin-button]:appearance-none" )}editor/grida-canvas/editor.ts (1)
933-957:applyScaleAPI looks solid; clarify/validate allowed factor domain (<=0 / negative) and whether targets should be deduped.
Right now it silently no-ops forfactor === 1and non-finite values, but will pass through0/ negative values—if those are unsupported (or cause downstream “locked constraint / zero dimension” edge cases), consider guarding here or explicitly documenting the behavior.public applyScale( target: ReadonlyArray<editor.NodeID> | "selection", factor: number, options?: { origin?: "center" | cmath.CardinalDirection; include_subtree?: boolean; space?: "auto" | "global"; } ) { @@ if (!targets || targets.length === 0) return; if (!Number.isFinite(factor) || factor === 1) return; + // Decide policy: allow negative (flip) / zero (collapse) or treat as invalid. + // if (factor <= 0) return; this.dispatch({ type: "apply-scale", - targets: Array.from(targets), + targets: Array.from(new Set(targets)), factor, origin: options?.origin ?? "center", include_subtree: options?.include_subtree ?? true, space: options?.space ?? "auto", }); }editor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsx (1)
17-17: Toolbar “scale” option wiring is consistent; confirm customUpscaleIconis acceptable vs Lucide/Radix.
If the repo policy is “Lucide/Radix only” for icons, consider swappingUpscaleIconto an existing icon (or moving this SVG under the same icon system). Based on learnings, prefer Lucide/Radix.Also applies to: 150-151, 261-262, 286-288
editor/scaffolds/sidecontrol/chunks/scale-tool.tsx (1)
144-173: Division by zero guard missing forsessionScale.Line 151 computes
factorToApply = nextScale / sessionScale. IfsessionScaleis 0 (though unlikely given theMath.max(0.01, next)guard), this could produceInfinity. Consider adding a guard:const nextScale = Math.max(0.01, next); + if (sessionScale === 0) return; const factorToApply = nextScale / sessionScale;editor/grida-canvas/reducers/methods/scale.ts (2)
48-52: Missing null check for bounding rects.Line 51 uses non-null assertion (
!) when mapping node IDs to rects. IfgetNodeAbsoluteBoundingRectreturnsnullfor any node, this could cause issues downstream.const rects = selection.map( - (node_id) => context.geometry.getNodeAbsoluteBoundingRect(node_id)! + (node_id) => context.geometry.getNodeAbsoluteBoundingRect(node_id) + ).filter((r): r is cmath.Rectangle => r !== null); + + if (rects.length !== selection.length) { + // Some nodes don't have bounding rects, skip gesture start + return; );
304-308: Assertion message could be more helpful.The assertion at Line 305-307 provides useful context, but consider including the selection context:
assert( parent_rect, - "Parent rect must be defined : " + parent_id + "/" + node_id + `Parent rect must be defined for parent ${parent_id} (child: ${node_id}, selection: ${selection.join(",")})` );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
editor/public/assets/css-cursors-grida/ew-resize-64-x32y32-000000.pngis excluded by!**/*.pngeditor/public/assets/css-cursors-grida/ew-resize-64-x32y32-000000.svgis excluded by!**/*.svgeditor/public/assets/css-cursors-grida/ew-resize-scale-64-x32y32-000000.pngis excluded by!**/*.pngeditor/public/assets/css-cursors-grida/ew-resize-scale-64-x32y32-000000.svgis excluded by!**/*.svgeditor/public/assets/css-cursors-grida/nesw-resize-64-x32y32-000000.pngis excluded by!**/*.pngeditor/public/assets/css-cursors-grida/nesw-resize-64-x32y32-000000.svgis excluded by!**/*.svgeditor/public/assets/css-cursors-grida/nesw-resize-scale-64-x32y32-000000.pngis excluded by!**/*.pngeditor/public/assets/css-cursors-grida/nesw-resize-scale-64-x32y32-000000.svgis excluded by!**/*.svgeditor/public/assets/css-cursors-grida/ns-resize-64-x32y32-000000.pngis excluded by!**/*.pngeditor/public/assets/css-cursors-grida/ns-resize-64-x32y32-000000.svgis excluded by!**/*.svgeditor/public/assets/css-cursors-grida/ns-resize-scale-64-x32y32-000000.pngis excluded by!**/*.pngeditor/public/assets/css-cursors-grida/ns-resize-scale-64-x32y32-000000.svgis excluded by!**/*.svgeditor/public/assets/css-cursors-grida/nwse-resize-64-x32y32-000000.pngis excluded by!**/*.pngeditor/public/assets/css-cursors-grida/nwse-resize-64-x32y32-000000.svgis excluded by!**/*.svgeditor/public/assets/css-cursors-grida/nwse-resize-scale-64-x32y32-000000.pngis excluded by!**/*.pngeditor/public/assets/css-cursors-grida/nwse-resize-scale-64-x32y32-000000.svgis excluded by!**/*.svg
📒 Files selected for processing (52)
AGENTS.md(1 hunks)docs/AGENTS.md(1 hunks)docs/wg/feat-authoring/parametric-scaling.md(3 hunks)editor/components/cursor/cursor-data.ts(2 hunks)editor/grida-canvas-hosted/playground/uxhost-toolbar.tsx(1 hunks)editor/grida-canvas-react-starter-kit/starterkit-icons/upscale.tsx(1 hunks)editor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsx(4 hunks)editor/grida-canvas-react-starter-kit/starterkit-toolbar/utils.ts(3 hunks)editor/grida-canvas-react/provider.tsx(1 hunks)editor/grida-canvas-react/viewport/hotkeys.tsx(2 hunks)editor/grida-canvas-react/viewport/surface.tsx(8 hunks)editor/grida-canvas-react/viewport/ui/floating-bar.tsx(1 hunks)editor/grida-canvas-react/viewport/ui/surface-vector-editor.tsx(1 hunks)editor/grida-canvas/action.ts(2 hunks)editor/grida-canvas/editor.i.ts(3 hunks)editor/grida-canvas/editor.ts(1 hunks)editor/grida-canvas/reducers/__tests__/apply-scale.roundtrip.test.ts(1 hunks)editor/grida-canvas/reducers/__tests__/history.test.ts(0 hunks)editor/grida-canvas/reducers/__tests__/image-paint-clipboard.test.ts(0 hunks)editor/grida-canvas/reducers/__tests__/vector-cut.test.ts(0 hunks)editor/grida-canvas/reducers/__tests__/vector-self-remove.test.ts(0 hunks)editor/grida-canvas/reducers/document.reducer.ts(2 hunks)editor/grida-canvas/reducers/event-target.reducer.ts(7 hunks)editor/grida-canvas/reducers/methods/index.ts(1 hunks)editor/grida-canvas/reducers/methods/scale.ts(1 hunks)editor/grida-canvas/reducers/methods/tool.ts(2 hunks)editor/grida-canvas/reducers/methods/transform.ts(2 hunks)editor/grida-canvas/reducers/schema/__tests__/schema.parametric-scale.test.ts(1 hunks)editor/grida-canvas/reducers/schema/index.ts(1 hunks)editor/grida-canvas/reducers/schema/schema.ts(1 hunks)editor/grida-canvas/reducers/surface.reducer.ts(2 hunks)editor/jest.config.js(1 hunks)editor/jest.setup.ts(1 hunks)editor/public/assets/css-cursors-grida/README.md(1 hunks)editor/scaffolds/sidecontrol/chunks/scale-tool.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/alignment9.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/fe.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/font-size.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/scale-factor.tsx(1 hunks)editor/scaffolds/sidecontrol/sidecontrol-node-selection.tsx(7 hunks)editor/scaffolds/sidecontrol/ui/index.tsx(1 hunks)editor/scaffolds/sidecontrol/ui/number.tsx(3 hunks)fixtures/test-grida/README.md(1 hunks)packages/grida-canvas-vn/__mocks__/svg-pathdata.ts(0 hunks)packages/grida-canvas-vn/__tests__/vn.test.ts(3 hunks)packages/grida-canvas-vn/jest.config.ts(1 hunks)packages/grida-canvas-vn/package.json(1 hunks)packages/grida-canvas-vn/tsconfig.json(1 hunks)packages/grida-canvas-vn/vn.ts(4 hunks)packages/grida-cmath/index.ts(2 hunks)packages/grida-number-input/src/n.test.ts(1 hunks)packages/grida-number-input/src/n.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/grida-canvas-vn/mocks/svg-pathdata.ts
- editor/grida-canvas/reducers/tests/vector-self-remove.test.ts
- editor/grida-canvas/reducers/tests/image-paint-clipboard.test.ts
- editor/grida-canvas/reducers/tests/vector-cut.test.ts
- editor/grida-canvas/reducers/tests/history.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript 5 as the main language for most apps
Use Lucide or Radix Icons for icons
Files:
editor/grida-canvas-react/viewport/ui/surface-vector-editor.tsxeditor/scaffolds/sidecontrol/controls/font-size.tsxeditor/grida-canvas-react/provider.tsxeditor/grida-canvas/reducers/methods/index.tseditor/grida-canvas-react-starter-kit/starterkit-icons/upscale.tsxeditor/grida-canvas-hosted/playground/uxhost-toolbar.tsxeditor/grida-canvas/reducers/schema/__tests__/schema.parametric-scale.test.tseditor/scaffolds/sidecontrol/controls/scale-factor.tsxeditor/scaffolds/sidecontrol/ui/index.tsxeditor/scaffolds/sidecontrol/ui/number.tsxeditor/grida-canvas-react/viewport/ui/floating-bar.tsxeditor/scaffolds/sidecontrol/controls/alignment9.tsxeditor/grida-canvas/reducers/schema/index.tseditor/components/cursor/cursor-data.tseditor/grida-canvas-react-starter-kit/starterkit-toolbar/utils.tseditor/grida-canvas/reducers/surface.reducer.tspackages/grida-canvas-vn/vn.tspackages/grida-number-input/src/n.test.tseditor/scaffolds/sidecontrol/controls/fe.tsxeditor/grida-canvas/reducers/__tests__/apply-scale.roundtrip.test.tseditor/grida-canvas/editor.tseditor/grida-canvas/reducers/document.reducer.tseditor/grida-canvas/reducers/methods/tool.tseditor/scaffolds/sidecontrol/chunks/scale-tool.tsxpackages/grida-cmath/index.tseditor/grida-canvas/reducers/event-target.reducer.tspackages/grida-canvas-vn/jest.config.tseditor/grida-canvas/reducers/methods/transform.tseditor/jest.setup.tseditor/grida-canvas/editor.i.tseditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsxeditor/grida-canvas-react/viewport/hotkeys.tsxeditor/grida-canvas/action.tseditor/grida-canvas-react/viewport/surface.tsxpackages/grida-canvas-vn/__tests__/vn.test.tseditor/grida-canvas/reducers/methods/scale.tseditor/grida-canvas/reducers/schema/schema.tspackages/grida-number-input/src/n.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use React.js 19 for web UI development
Files:
editor/grida-canvas-react/viewport/ui/surface-vector-editor.tsxeditor/scaffolds/sidecontrol/controls/font-size.tsxeditor/grida-canvas-react/provider.tsxeditor/grida-canvas-react-starter-kit/starterkit-icons/upscale.tsxeditor/grida-canvas-hosted/playground/uxhost-toolbar.tsxeditor/scaffolds/sidecontrol/controls/scale-factor.tsxeditor/scaffolds/sidecontrol/ui/index.tsxeditor/scaffolds/sidecontrol/ui/number.tsxeditor/grida-canvas-react/viewport/ui/floating-bar.tsxeditor/scaffolds/sidecontrol/controls/alignment9.tsxeditor/scaffolds/sidecontrol/controls/fe.tsxeditor/scaffolds/sidecontrol/chunks/scale-tool.tsxeditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsxeditor/grida-canvas-react/viewport/hotkeys.tsxeditor/grida-canvas-react/viewport/surface.tsx
**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS 4 for styling
Files:
editor/grida-canvas-react/viewport/ui/surface-vector-editor.tsxeditor/scaffolds/sidecontrol/controls/font-size.tsxeditor/grida-canvas-react/provider.tsxeditor/grida-canvas/reducers/methods/index.tseditor/grida-canvas-react-starter-kit/starterkit-icons/upscale.tsxeditor/grida-canvas-hosted/playground/uxhost-toolbar.tsxeditor/grida-canvas/reducers/schema/__tests__/schema.parametric-scale.test.tseditor/scaffolds/sidecontrol/controls/scale-factor.tsxeditor/scaffolds/sidecontrol/ui/index.tsxeditor/scaffolds/sidecontrol/ui/number.tsxeditor/grida-canvas-react/viewport/ui/floating-bar.tsxeditor/scaffolds/sidecontrol/controls/alignment9.tsxeditor/grida-canvas/reducers/schema/index.tseditor/components/cursor/cursor-data.tseditor/grida-canvas-react-starter-kit/starterkit-toolbar/utils.tseditor/grida-canvas/reducers/surface.reducer.tspackages/grida-canvas-vn/vn.tspackages/grida-number-input/src/n.test.tseditor/scaffolds/sidecontrol/controls/fe.tsxeditor/grida-canvas/reducers/__tests__/apply-scale.roundtrip.test.tseditor/grida-canvas/editor.tseditor/grida-canvas/reducers/document.reducer.tseditor/grida-canvas/reducers/methods/tool.tseditor/scaffolds/sidecontrol/chunks/scale-tool.tsxpackages/grida-cmath/index.tseditor/grida-canvas/reducers/event-target.reducer.tspackages/grida-canvas-vn/jest.config.tseditor/grida-canvas/reducers/methods/transform.tseditor/jest.setup.tseditor/grida-canvas/editor.i.tseditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsxeditor/grida-canvas-react/viewport/hotkeys.tsxeditor/grida-canvas/action.tseditor/grida-canvas-react/viewport/surface.tsxpackages/grida-canvas-vn/__tests__/vn.test.tseditor/grida-canvas/reducers/methods/scale.tseditor/grida-canvas/reducers/schema/schema.tspackages/grida-number-input/src/n.ts
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Place documentation files in the ./docs directory, which is the source of truth
Files:
docs/wg/feat-authoring/parametric-scaling.mddocs/AGENTS.md
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Shadcn UI for reusable UI components
Files:
editor/components/cursor/cursor-data.ts
🧠 Learnings (22)
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Use Canvas 2D API with path commands for rendering geometric shapes (circle, square, triangle, etc.)
Applied to files:
editor/grida-canvas-react/viewport/ui/surface-vector-editor.tsxeditor/grida-canvas-react/provider.tsxeditor/grida-canvas/reducers/schema/__tests__/schema.parametric-scale.test.tspackages/grida-canvas-vn/vn.tseditor/grida-canvas/reducers/__tests__/apply-scale.roundtrip.test.tseditor/grida-canvas/reducers/methods/tool.tseditor/grida-canvas/reducers/event-target.reducer.tseditor/jest.setup.tseditor/grida-canvas/editor.i.tseditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react/viewport/surface.tsxpackages/grida-canvas-vn/__tests__/vn.test.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : For SVG export, convert circles to <circle> elements, rectangles to <rect> elements, and polygons to <polygon> elements with calculated points
Applied to files:
editor/grida-canvas-react/viewport/ui/surface-vector-editor.tsxeditor/grida-canvas-react-starter-kit/starterkit-icons/upscale.tsxeditor/components/cursor/cursor-data.tspackages/grida-canvas-vn/vn.tseditor/grida-canvas/reducers/__tests__/apply-scale.roundtrip.test.tseditor/grida-canvas/reducers/methods/tool.tseditor/grida-canvas/reducers/event-target.reducer.tseditor/jest.setup.tseditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsx
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : When adding new shape types, update the Shape type union, add cases in drawShape() function, add cases in shapeToSVG() function, and add SelectItem in UI
Applied to files:
editor/grida-canvas-react/viewport/ui/surface-vector-editor.tsxeditor/grida-canvas-react/provider.tsxeditor/grida-canvas-react-starter-kit/starterkit-icons/upscale.tsxeditor/grida-canvas-hosted/playground/uxhost-toolbar.tsxeditor/grida-canvas/reducers/schema/__tests__/schema.parametric-scale.test.tseditor/scaffolds/sidecontrol/controls/alignment9.tsxeditor/grida-canvas-react-starter-kit/starterkit-toolbar/utils.tspackages/grida-canvas-vn/vn.tseditor/grida-canvas/reducers/__tests__/apply-scale.roundtrip.test.tseditor/grida-canvas/reducers/document.reducer.tseditor/grida-canvas/reducers/methods/tool.tseditor/scaffolds/sidecontrol/chunks/scale-tool.tsxeditor/grida-canvas/reducers/event-target.reducer.tseditor/jest.setup.tseditor/grida-canvas/editor.i.tseditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsxeditor/grida-canvas/action.tseditor/grida-canvas-react/viewport/surface.tsxeditor/grida-canvas/reducers/schema/schema.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : When adding new parameters to the halftone tool, add state with useState, include in useEffect dependency array, pass to renderHalftone() function, use in rendering logic, and add UI control
Applied to files:
editor/grida-canvas-react/viewport/ui/surface-vector-editor.tsxeditor/grida-canvas-hosted/playground/uxhost-toolbar.tsxeditor/scaffolds/sidecontrol/controls/scale-factor.tsxeditor/scaffolds/sidecontrol/controls/alignment9.tsxeditor/grida-canvas-react-starter-kit/starterkit-toolbar/utils.tseditor/scaffolds/sidecontrol/controls/fe.tsxeditor/grida-canvas/reducers/methods/tool.tseditor/scaffolds/sidecontrol/chunks/scale-tool.tsxeditor/grida-canvas/editor.i.tseditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsx
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Use React hooks for state management (imageSrc, shape, grid, maxRadius, gamma, jitter, opacity, color, customShapeImage, imageDataRef, sizeRef)
Applied to files:
editor/grida-canvas-react/viewport/ui/surface-vector-editor.tsxeditor/scaffolds/sidecontrol/chunks/scale-tool.tsxeditor/scaffolds/sidecontrol/sidecontrol-node-selection.tsxeditor/grida-canvas-react/viewport/surface.tsx
📚 Learning: 2025-12-01T00:22:41.045Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:41.045Z
Learning: Applies to editor/**/next-env.d.ts : Include `next-env.d.ts` file in git for the project to avoid requiring dev/build runs for typechecking in CI environments
Applied to files:
editor/jest.config.jspackages/grida-canvas-vn/tsconfig.jsonpackages/grida-canvas-vn/jest.config.ts
📚 Learning: 2025-12-01T00:21:48.564Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T00:21:48.564Z
Learning: Applies to **/components/**/*.{ts,tsx} : Use Shadcn UI for reusable UI components
Applied to files:
editor/scaffolds/sidecontrol/controls/font-size.tsxeditor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsx
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/main.rs : Update `grida-canvas-wasm.d.ts` TypeScript definitions file when new APIs are introduced via `main.rs`
Applied to files:
editor/grida-canvas-react/provider.tsxeditor/grida-canvas/reducers/methods/index.tsfixtures/test-grida/README.mdeditor/grida-canvas/reducers/schema/__tests__/schema.parametric-scale.test.tspackages/grida-canvas-vn/tsconfig.jsoneditor/grida-canvas/reducers/schema/index.tseditor/components/cursor/cursor-data.tspackages/grida-canvas-vn/vn.tseditor/grida-canvas/reducers/__tests__/apply-scale.roundtrip.test.tspackages/grida-canvas-vn/package.jsonpackages/grida-canvas-vn/jest.config.tseditor/grida-canvas-react/viewport/surface.tsxeditor/grida-canvas/reducers/schema/schema.ts
📚 Learning: 2025-12-01T00:21:48.564Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T00:21:48.564Z
Learning: Applies to **/*.{tsx,jsx} : Use React.js 19 for web UI development
Applied to files:
editor/grida-canvas-react-starter-kit/starterkit-icons/upscale.tsx
📚 Learning: 2025-12-01T00:22:28.164Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:28.164Z
Learning: Validate `.grida` files using the `tool_io_grida` CLI tool for debugging parsing issues and structure verification
Applied to files:
fixtures/test-grida/README.md
📚 Learning: 2025-12-01T00:22:06.800Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-fonts/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:06.800Z
Learning: Applies to crates/grida-canvas-fonts/**/scenario_*.rs : Scenario-specific comprehensive tests should be organized in files matching `scenario_*.rs`
Applied to files:
fixtures/test-grida/README.mdAGENTS.md
📚 Learning: 2025-12-01T00:22:06.800Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-fonts/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:06.800Z
Learning: Applies to crates/grida-canvas-fonts/**/scenario_level2_*.rs : Level 2+ placeholder and limitation tests should be organized in files matching `scenario_level2_*.rs`
Applied to files:
fixtures/test-grida/README.md
📚 Learning: 2025-12-01T00:21:48.564Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T00:21:48.564Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript 5 as the main language for most apps
Applied to files:
packages/grida-canvas-vn/tsconfig.json
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/+(grida-canvas-wasm.js|grida-canvas-wasm.wasm) : Include WASM artifacts (`grida-canvas-wasm.js` and `grida-canvas-wasm.wasm`) in git for faster CI builds
Applied to files:
packages/grida-canvas-vn/tsconfig.jsonAGENTS.mdpackages/grida-canvas-vn/package.json
📚 Learning: 2025-12-01T00:21:48.564Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T00:21:48.564Z
Learning: Applies to {editor,apps}/**/app/**/*.{ts,tsx} : Use Next.js 15 as the web framework
Applied to files:
packages/grida-canvas-vn/tsconfig.json
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Custom images used as halftone shapes should be loaded as HTMLImageElement for efficient canvas rendering and preserve original image colors in as-is mode
Applied to files:
editor/components/cursor/cursor-data.ts
📚 Learning: 2025-12-01T00:21:55.314Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: apps/docs/AGENTS.md:0-0
Timestamp: 2025-12-01T00:21:55.314Z
Learning: Applies to apps/docs/docs/** : Edit documentation files in `/docs/` (root directory) as the source of truth for all documentation content
Applied to files:
docs/AGENTS.mdAGENTS.md
📚 Learning: 2025-12-01T00:21:55.314Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: apps/docs/AGENTS.md:0-0
Timestamp: 2025-12-01T00:21:55.314Z
Learning: Applies to apps/docs/docs/** : DO NOT edit files under `apps/docs/docs/` directory directly - this directory is auto-generated and will be overwritten on build
Applied to files:
docs/AGENTS.mdAGENTS.md
📚 Learning: 2025-12-01T00:21:48.564Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T00:21:48.564Z
Learning: Applies to docs/**/*.md : Place documentation files in the ./docs directory, which is the source of truth
Applied to files:
docs/AGENTS.mdAGENTS.md
📚 Learning: 2025-12-01T00:21:48.564Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T00:21:48.564Z
Learning: Place Rust crates in the /crates directory as a monorepo
Applied to files:
AGENTS.md
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Cache ImageData and dimensions in refs (imageDataRef, sizeRef) for efficient exports
Applied to files:
editor/grida-canvas/reducers/__tests__/apply-scale.roundtrip.test.ts
📚 Learning: 2025-12-01T00:21:48.564Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T00:21:48.564Z
Learning: Applies to **/*.{ts,tsx} : Use Lucide or Radix Icons for icons
Applied to files:
editor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsx
🧬 Code graph analysis (13)
editor/scaffolds/sidecontrol/controls/scale-factor.tsx (1)
editor/scaffolds/sidecontrol/ui/number.tsx (1)
InputPropertyNumber(127-246)
editor/scaffolds/sidecontrol/controls/alignment9.tsx (1)
packages/grida-cmath/index.ts (1)
Alignment9(176-185)
editor/grida-canvas/reducers/__tests__/apply-scale.roundtrip.test.ts (1)
editor/grida-canvas/reducers/index.ts (1)
ReducerContext(20-31)
editor/grida-canvas/editor.ts (2)
editor/grida-canvas/editor.i.ts (1)
NodeID(136-136)packages/grida-cmath/index.ts (1)
CardinalDirection(164-172)
editor/grida-canvas/reducers/document.reducer.ts (2)
editor/grida-canvas/reducers/utils/immer.ts (1)
updateState(3-13)editor/grida-canvas/reducers/methods/scale.ts (1)
self_apply_scale_by_factor(697-759)
editor/grida-canvas/reducers/event-target.reducer.ts (3)
editor/grida-canvas/reducers/methods/hover.ts (1)
self_updateSurfaceHoverState(5-48)editor/grida-canvas/reducers/methods/selection.ts (2)
self_clearSelection(93-115)self_selectNode(33-91)editor/grida-canvas/reducers/tools/target.ts (1)
getMarqueeSelection(240-295)
editor/grida-canvas/reducers/methods/transform.ts (1)
editor/grida-canvas/reducers/methods/scale.ts (1)
self_update_gesture_scale(167-187)
editor/grida-canvas/editor.i.ts (2)
packages/grida-cmath/index.ts (2)
Rectangle(147-158)CardinalDirection(164-172)packages/grida-canvas-schema/grida.ts (1)
NodeID(1101-1101)
editor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsx (1)
editor/grida-canvas-react-starter-kit/starterkit-icons/upscale.tsx (1)
UpscaleIcon(3-22)
editor/grida-canvas/action.ts (3)
editor/grida-canvas/editor.i.ts (1)
NodeID(136-136)packages/grida-canvas-schema/grida.ts (1)
NodeID(1101-1101)packages/grida-cmath/index.ts (1)
CardinalDirection(164-172)
editor/grida-canvas-react/viewport/surface.tsx (1)
editor/grida-canvas-react/provider.tsx (1)
useToolState(497-500)
editor/grida-canvas/reducers/methods/scale.ts (6)
editor/grida-canvas/editor.ts (2)
id(4483-4485)scale(139-184)editor/grida-canvas/reducers/tools/snap.ts (2)
getSnapTargets(184-217)threshold(20-23)editor/grida-canvas/reducers/tools/snap-resize.ts (1)
snapObjectsResize(651-975)editor/grida-canvas/reducers/node-transform.reducer.ts (1)
updateNodeTransform(70-202)editor/grida-canvas/reducers/schema/index.ts (1)
schema(1-1)editor/app/(dev)/canvas/experimental/grid/core/state.ts (1)
initial(6-9)
editor/grida-canvas/reducers/schema/schema.ts (2)
packages/grida-canvas-schema/grida.ts (9)
ICornerRadius(1471-1473)IRectangularCornerRadius(1478-1484)IPositioning(1778-1786)ICSSDimension(1768-1771)IPadding(1489-1498)IFlexContainer(1508-1563)IStroke(1622-1673)IRectangularStrokeWidth(1678-1695)IEffects(1586-1592)packages/grida-canvas-cg/lib.ts (6)
VariableWidthProfile(1365-1367)FeShadow(1073-1076)FeLayerBlur(1080-1088)FeBackdropBlur(1090-1098)FeLiquidGlass(1106-1172)FeNoise(1219-1294)
🪛 LanguageTool
docs/wg/feat-authoring/parametric-scaling.md
[style] ~98-~98: Consider replacing this word to strengthen your wording.
Context: ...y rewriting is an implementation detail and is not required for the core parameter-...
(AND_THAT)
docs/AGENTS.md
[style] ~27-~27: In American English, abbreviations like “etc.” require a period.
Context: ...nts, architecture documents, todo list, etc | yes | | [/docs/reference](....
(ETC_PERIOD)
[style] ~34-~34: In American English, abbreviations like “etc.” require a period.
Context: ... | Contributing, Support, Community, etc | yes ...
(ETC_PERIOD)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
editor/grida-canvas/reducers/schema/schema.ts (3)
1-33: Namespace approach is fine, but consider whethernamespaceis intentional for bundling/tree-shaking.
If consumers only need a few helpers, aconst schema = { parametric_scale: ... }style module often tree-shakes better thannamespace(depending on TS config/output).
86-97:scale_rect_about_anchormath looks correct; consider clamping/validatingsat the boundary.
If callers ever pass unclamped/NaN scale, this can propagate invalid geometry. (Might already be guaranteed upstream—just worth verifying.)export function scale_rect_about_anchor( rect: cmath.Rectangle, anchor: cmath.Vector2, s: number ): cmath.Rectangle { + s = _clamp_scale(s); return { x: anchor[0] + (rect.x - anchor[0]) * s, y: anchor[1] + (rect.y - anchor[1]) * s, width: rect.width * s, height: rect.height * s, }; }
99-128: Helpers are pragmatic; consider tightening typing to avoid accidental mutation of readonly shapes.
Right nowscale_number_in_place<T extends object>(obj: T, key: keyof T, s: number)will happily accept structurally readonly objects at compile-time depending on how it’s called, then mutate anyway. If these are only used on draft/mutable nodes, fine—otherwise consider constrainingTto a mutable record shape.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
editor/components/cursor/cursor-data.ts(2 hunks)editor/grida-canvas/reducers/schema/schema.ts(1 hunks)editor/scaffolds/sidecontrol/controls/alignment9.tsx(1 hunks)editor/scaffolds/sidecontrol/sidecontrol-node-selection.tsx(6 hunks)packages/grida-canvas-vn/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/grida-canvas-vn/tsconfig.json
- editor/components/cursor/cursor-data.ts
- editor/scaffolds/sidecontrol/sidecontrol-node-selection.tsx
- editor/scaffolds/sidecontrol/controls/alignment9.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript 5 as the main language for most apps
Use Lucide or Radix Icons for icons
Files:
editor/grida-canvas/reducers/schema/schema.ts
**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS 4 for styling
Files:
editor/grida-canvas/reducers/schema/schema.ts
🧠 Learnings (3)
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : When adding new shape types, update the Shape type union, add cases in drawShape() function, add cases in shapeToSVG() function, and add SelectItem in UI
Applied to files:
editor/grida-canvas/reducers/schema/schema.ts
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/main.rs : Update `grida-canvas-wasm.d.ts` TypeScript definitions file when new APIs are introduced via `main.rs`
Applied to files:
editor/grida-canvas/reducers/schema/schema.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Use Canvas 2D API with path commands for rendering geometric shapes (circle, square, triangle, etc.)
Applied to files:
editor/grida-canvas/reducers/schema/schema.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cargo test
- GitHub Check: test
🔇 Additional comments (2)
editor/grida-canvas/reducers/schema/schema.ts (2)
220-227: The code correctly assumesvector_networkis present for vector nodes. According to the VectorNode type definition in the schema (packages/grida-canvas-schema/grida.ts),vector_networkis a required property without an optional marker. The unconditional call inapply_nodeis appropriate—if a node hastype === "vector", the type system guaranteesvector_networkexists. No defensive guard is necessary.
229-310:apply_nodecorrectly preserves"auto"values and handles zero-dimensions, but does not respect node lock constraints.The implementation validates:
- "preserve-auto":
width/heightonly scale when numeric (lines 238-239), preventing conversion of"auto"to fixed values- zero-dimension handling: numeric
0remains0after scalingHowever, the function unconditionally scales all numeric properties without checking for lock flags. Evidence in the codebase shows Grida nodes support a
lockedfield (seen in editor state and UI controls), butapply_nodedoes not reference or respect any lock constraints. If a node or property is marked as locked, it will still be scaled by this function.Recommend verifying whether:
- Locked properties should skip scaling (e.g., locked
width/height, aspect-ratio locks)- Lock-checking should happen in
apply_nodeor at the call site inscale.ts- This behavior is intentional (scaling ignores locks) or a missing feature
Comment on lines
+35
to
+84
| export function _clamp_scale(s: number) { | ||
| if (!Number.isFinite(s)) return 1; | ||
| return Math.max(0.01, s); | ||
| } | ||
|
|
||
| /** | ||
| * Calculates a uniform similarity scale factor from a movement delta. | ||
| * | ||
| * - Always returns a clamped scale \(s \ge 0.01\) | ||
| * - If `q` is provided, the return value is quantized to `q` precision | ||
| * | ||
| * Intended usage: | ||
| * - **Interactive gestures / UI**: pass `q = 0.01` to keep stable `0.00x` precision. | ||
| * - **Programmatic commands / API** (e.g. `applyScale(factor)`): use the factor as-is | ||
| * (developer intent) and do not route it through this helper. | ||
| */ | ||
| export function _uniform_scale_factor( | ||
| initial_bounds: cmath.Rectangle, | ||
| movement: cmath.Vector2, | ||
| q?: number | ||
| ) { | ||
| const w = initial_bounds.width; | ||
| const h = initial_bounds.height; | ||
|
|
||
| if (w === 0 && h === 0) return 1; | ||
|
|
||
| const dominantAxis = | ||
| Math.abs(movement[0]) > Math.abs(movement[1]) ? "x" : "y"; | ||
|
|
||
| let s: number; | ||
| if (dominantAxis === "x") { | ||
| if (w === 0) { | ||
| s = h !== 0 ? (h + movement[1]) / h : 1; | ||
| } else { | ||
| s = (w + movement[0]) / w; | ||
| } | ||
| } else { | ||
| if (h === 0) { | ||
| s = w !== 0 ? (w + movement[0]) / w : 1; | ||
| } else { | ||
| s = (h + movement[1]) / h; | ||
| } | ||
| } | ||
|
|
||
| const clamped = _clamp_scale(s); | ||
| if (typeof q === "number" && Number.isFinite(q) && q > 0) { | ||
| return cmath.quantize(clamped, q); | ||
| } | ||
| return clamped; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all call sites of _uniform_scale_factor
rg "_uniform_scale_factor" --type ts --type tsx -nRepository: gridaco/grida
Length of output: 84
🏁 Script executed:
# Also search for any references to quantization in the codebase
rg "quantize" --type ts --type tsx -B2 -A2Repository: gridaco/grida
Length of output: 84
🏁 Script executed:
# Check the git context to understand the PR's requirements
git log --oneline -20Repository: gridaco/grida
Length of output: 75
🏁 Script executed:
# Look for any PR descriptions or comments about no-quantization
git show --stat | head -50Repository: gridaco/grida
Length of output: 2921
🏁 Script executed:
# Search for _uniform_scale_factor call sites using just ts type
rg "_uniform_scale_factor" -n --type tsRepository: gridaco/grida
Length of output: 255
🏁 Script executed:
# Also search without type restriction to catch all files
rg "_uniform_scale_factor" -nRepository: gridaco/grida
Length of output: 255
🏁 Script executed:
# Look at the specific file to understand its context and usage
head -150 editor/grida-canvas/reducers/schema/schema.ts | tail -100Repository: gridaco/grida
Length of output: 2394
🏁 Script executed:
# Search for apply_node function mentioned in the review
rg "apply_node" -n --type ts -A3Repository: gridaco/grida
Length of output: 2450
🏁 Script executed:
# Read the call site context in scale.ts around line 642
sed -n '630,680p' editor/grida-canvas/reducers/methods/scale.tsRepository: gridaco/grida
Length of output: 1555
🏁 Script executed:
# Get more context about this function
sed -n '600,750p' editor/grida-canvas/reducers/methods/scale.tsRepository: gridaco/grida
Length of output: 4426
🏁 Script executed:
# Check what happens with the returned value - look for where `s` is used
sed -n '640,670p' editor/grida-canvas/reducers/methods/scale.tsRepository: gridaco/grida
Length of output: 915
🏁 Script executed:
# Check the git diff to understand the PR's actual changes and intent
git diff HEAD~1 HEAD --stat | head -20Repository: gridaco/grida
Length of output: 243
🏁 Script executed:
# Look for any PR description or commit message mentioning quantization
git log --all --oneline | head -30Repository: gridaco/grida
Length of output: 136
🏁 Script executed:
# Check if there are any other usages of the q parameter or quantization context
rg "no-quantization\|no quantization" -iRepository: gridaco/grida
Length of output: 39
🏁 Script executed:
# Search for recent changes to schema.ts or scale.ts
git log -p --follow -S "_uniform_scale_factor" -- editor/grida-canvas/reducers/schema/schema.ts | head -100Repository: gridaco/grida
Length of output: 39
🏁 Script executed:
# Check the apply_node function to confirm it directly modifies node geometry
sed -n '229,280p' editor/grida-canvas/reducers/schema/schema.tsRepository: gridaco/grida
Length of output: 2144
🏁 Script executed:
# Look at the test file to understand expected behavior
cat editor/grida-canvas/reducers/schema/__tests__/schema.parametric-scale.test.ts | head -200Repository: gridaco/grida
Length of output: 3203
Remove quantization from _uniform_scale_factor to prevent baking quantized geometry.
The helper currently applies cmath.quantize(clamped, q) before returning (lines 80-82), and this quantized value is directly passed to apply_node(node, s) at scale.ts:668, which persists all geometry properties (width, height, corner_radius, stroke_width, etc.) with the quantized scale factor. Instead, return the unquantized scale and let UI code apply quantization only for display purposes.
export function _uniform_scale_factor(
initial_bounds: cmath.Rectangle,
movement: cmath.Vector2,
- q?: number
) {
const w = initial_bounds.width;
const h = initial_bounds.height;
@@
const clamped = _clamp_scale(s);
- if (typeof q === "number" && Number.isFinite(q) && q > 0) {
- return cmath.quantize(clamped, q);
- }
return clamped;
}Update the docstring and call site at scale.ts:642 accordingly.
Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+152
to
+200
| export function _fe_blur(initial: cg.FeLayerBlur, s: number): cg.FeLayerBlur { | ||
| const blur = initial.blur; | ||
| switch (blur.type) { | ||
| case "blur": | ||
| return { | ||
| ...initial, | ||
| blur: { | ||
| ...blur, | ||
| radius: blur.radius * s, | ||
| }, | ||
| }; | ||
| case "progressive-blur": | ||
| return { | ||
| ...initial, | ||
| blur: { | ||
| ...blur, | ||
| // x1/y1/x2/y2 remain unchanged (normalized) | ||
| radius: blur.radius * s, | ||
| radius2: blur.radius2 * s, | ||
| }, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| export function _fe_backdrop_blur( | ||
| initial: cg.FeBackdropBlur, | ||
| s: number | ||
| ): cg.FeBackdropBlur { | ||
| const blur = initial.blur; | ||
| switch (blur.type) { | ||
| case "blur": | ||
| return { | ||
| ...initial, | ||
| blur: { | ||
| ...blur, | ||
| radius: blur.radius * s, | ||
| }, | ||
| }; | ||
| case "progressive-blur": | ||
| return { | ||
| ...initial, | ||
| blur: { | ||
| ...blur, | ||
| radius: blur.radius * s, | ||
| radius2: blur.radius2 * s, | ||
| }, | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the @grida/cg package and its type definitions
find . -name "*.ts" -o -name "*.d.ts" | grep -E "(grida|cg)" | head -20Repository: gridaco/grida
Length of output: 1146
🏁 Script executed:
# Search for the type definitions of FeLayerBlur and FeBackdropBlur
rg "FeLayerBlur|FeBackdropBlur" --type ts --type tsx -B 2 -A 5 | head -100Repository: gridaco/grida
Length of output: 84
🏁 Script executed:
# Look for the blur type definition
rg "blur.*:.*{.*type" --type ts -A 3 | head -50Repository: gridaco/grida
Length of output: 165
🏁 Script executed:
# Check the imports in the target file to understand where cg comes from
head -30 editor/grida-canvas/reducers/schema/schema.tsRepository: gridaco/grida
Length of output: 1132
🏁 Script executed:
# Find @grida/cg package location
find . -name "package.json" -type f | xargs grep -l "@grida/cg" | head -5Repository: gridaco/grida
Length of output: 248
🏁 Script executed:
# Search for @grida/cg type definitions directory
find . -type d -name "cg" | grep -E "packages|src" | head -10Repository: gridaco/grida
Length of output: 85
🏁 Script executed:
# Search for FeLayerBlur type definition
rg "FeLayerBlur" --type ts -B 3 -A 10 | head -100Repository: gridaco/grida
Length of output: 5932
🏁 Script executed:
# Search for blur type definitions more broadly
rg "type.*blur" --type ts -i | head -30Repository: gridaco/grida
Length of output: 2465
🏁 Script executed:
# Check if there's a .d.ts file for cg module
find . -name "*.d.ts" | xargs grep -l "FeLayerBlur\|FeBackdropBlur" 2>/dev/nullRepository: gridaco/grida
Length of output: 41
🏁 Script executed:
# Get the complete FeBlur and related type definitions
sed -n '1,300p' packages/grida-canvas-cg/lib.ts | grep -A 15 "export type FeBlur\|export type FeGaussian\|export type FeProgressive"Repository: gridaco/grida
Length of output: 39
🏁 Script executed:
# Get more complete context around the blur type definitions
sed -n '1,500p' packages/grida-canvas-cg/lib.ts | tail -200Repository: gridaco/grida
Length of output: 2656
🏁 Script executed:
# Check tsconfig.json for TypeScript strict settings
find . -name "tsconfig.json" -type f | head -3 | xargs catRepository: gridaco/grida
Length of output: 1604
🏁 Script executed:
# Verify the exact blur type union by finding the complete definition
rg "export type FeBlur.*=.*\|" packages/grida-canvas-cg/ -A 1Repository: gridaco/grida
Length of output: 180
🏁 Script executed:
# Get the full blur type definitions
rg "export type FeGaussianBlur|export type FeProgressiveBlur|export interface IFeGaussian|export interface IFeProgressive" packages/grida-canvas-cg/ -A 10Repository: gridaco/grida
Length of output: 2383
🏁 Script executed:
# Check if blur property could be undefined
rg "blur\?" packages/grida-canvas-cg/lib.ts | head -20Repository: gridaco/grida
Length of output: 39
🏁 Script executed:
# Verify the complete FeLayerBlur and FeBackdropBlur definitions
rg "export type FeLayerBlur|export type FeBackdropBlur" packages/grida-canvas-cg/lib.ts -A 8Repository: gridaco/grida
Length of output: 419
🏁 Script executed:
# Check actual usage in the codebase - do these functions get called with potentially invalid blur objects?
rg "_fe_blur|_fe_backdrop_blur" editor/grida-canvas/ -B 2 -A 5 | head -50Repository: gridaco/grida
Length of output: 3664
🏁 Script executed:
# Verify if there's any code that creates FeLayerBlur/FeBackdropBlur with undefined blur
rg "FeLayerBlur|FeBackdropBlur" editor/grida-canvas/ -B 3 -A 3 | grep -A 3 -B 3 "blur.*undefined\|undefined.*blur"Repository: gridaco/grida
Length of output: 39
🏁 Script executed:
# Check if there are any other switch statements on blur.type to see if they also lack defaults
rg "switch.*blur\.type\|switch.*blur\[" editor/grida-canvas/ -B 2 -A 15 | head -80Repository: gridaco/grida
Length of output: 39
Add default case to _fe_blur and _fe_backdrop_blur for defensive programming.
Both functions implicitly return undefined if blur.type doesn't match either case. While the type system ensures blur.type is strictly "blur" | "progressive-blur" (see FeBlur = FeGaussianBlur | FeProgressiveBlur in packages/grida-canvas-cg/lib.ts), TypeScript strict mode is disabled in your config, so exhaustiveness checking isn't enforced. Adding a default fallback that returns initial is good defensive practice:
export function _fe_blur(initial: cg.FeLayerBlur, s: number): cg.FeLayerBlur {
const blur = initial.blur;
switch (blur.type) {
case "blur":
return { ... };
case "progressive-blur":
return { ... };
+ default:
+ return initial;
}
}Apply the same pattern to _fe_backdrop_blur.
📝 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
| export function _fe_blur(initial: cg.FeLayerBlur, s: number): cg.FeLayerBlur { | |
| const blur = initial.blur; | |
| switch (blur.type) { | |
| case "blur": | |
| return { | |
| ...initial, | |
| blur: { | |
| ...blur, | |
| radius: blur.radius * s, | |
| }, | |
| }; | |
| case "progressive-blur": | |
| return { | |
| ...initial, | |
| blur: { | |
| ...blur, | |
| // x1/y1/x2/y2 remain unchanged (normalized) | |
| radius: blur.radius * s, | |
| radius2: blur.radius2 * s, | |
| }, | |
| }; | |
| } | |
| } | |
| export function _fe_backdrop_blur( | |
| initial: cg.FeBackdropBlur, | |
| s: number | |
| ): cg.FeBackdropBlur { | |
| const blur = initial.blur; | |
| switch (blur.type) { | |
| case "blur": | |
| return { | |
| ...initial, | |
| blur: { | |
| ...blur, | |
| radius: blur.radius * s, | |
| }, | |
| }; | |
| case "progressive-blur": | |
| return { | |
| ...initial, | |
| blur: { | |
| ...blur, | |
| radius: blur.radius * s, | |
| radius2: blur.radius2 * s, | |
| }, | |
| }; | |
| } | |
| } | |
| export function _fe_blur(initial: cg.FeLayerBlur, s: number): cg.FeLayerBlur { | |
| const blur = initial.blur; | |
| switch (blur.type) { | |
| case "blur": | |
| return { | |
| ...initial, | |
| blur: { | |
| ...blur, | |
| radius: blur.radius * s, | |
| }, | |
| }; | |
| case "progressive-blur": | |
| return { | |
| ...initial, | |
| blur: { | |
| ...blur, | |
| // x1/y1/x2/y2 remain unchanged (normalized) | |
| radius: blur.radius * s, | |
| radius2: blur.radius2 * s, | |
| }, | |
| }; | |
| default: | |
| return initial; | |
| } | |
| } | |
| export function _fe_backdrop_blur( | |
| initial: cg.FeBackdropBlur, | |
| s: number | |
| ): cg.FeBackdropBlur { | |
| const blur = initial.blur; | |
| switch (blur.type) { | |
| case "blur": | |
| return { | |
| ...initial, | |
| blur: { | |
| ...blur, | |
| radius: blur.radius * s, | |
| }, | |
| }; | |
| case "progressive-blur": | |
| return { | |
| ...initial, | |
| blur: { | |
| ...blur, | |
| radius: blur.radius * s, | |
| radius2: blur.radius2 * s, | |
| }, | |
| }; | |
| default: | |
| return initial; | |
| } | |
| } |
🤖 Prompt for AI Agents
In editor/grida-canvas/reducers/schema/schema.ts around lines 152 to 200, the
switch statements in _fe_blur and _fe_backdrop_blur do not have a default branch
and can implicitly return undefined if blur.type is unexpected; add a default
case to each switch that returns the unchanged initial object (e.g., return
initial) so the functions always return a valid cg.FeLayerBlur /
cg.FeBackdropBlur and are defensive against unexpected values.
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.
assertions
lineheight should be kept 0.Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.