-
Notifications
You must be signed in to change notification settings - Fork 119
Daily RC #465
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
Daily RC #465
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughIntroduces a FileDropzone for drag-and-drop imports, rewires Figma/Grida import flows to use selectedFile, adds .grida drop handling, refactors surface resize handles with z-index/gesture and double-click auto-size, adds eager_canvas_input checks, and extends autoLayout for direct container application. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
…out layout, enhancing user experience by preserving container identity. Update documentation to reflect new behavior and parameters.
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on January 5. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
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.
Actionable comments posted: 1
🧹 Nitpick comments (5)
editor/scaffolds/sidecontrol/controls/font-family.tsx (1)
187-210: Tighten category typing and avoid repeatedfonts.filterworkThe NativeSelect block works as intended, but there are a couple of easy polish points:
- Avoid
as anyonsetCategoryto keep the union type enforced.- Avoid recomputing the three
fonts.filter(...).lengthcounts on every render; the same filters are already conceptually encoded incategoryFilteredFontFamilies.Example refactor (one option):
- const [category, setCategory] = React.useState< - "all-fonts" | "with-axes" | "non-variable" | "used-in-document" - >("all-fonts"); + type FontCategory = + | "all-fonts" + | "with-axes" + | "non-variable" + | "used-in-document"; + + const [category, setCategory] = React.useState<FontCategory>("all-fonts");- <NativeSelect - size="xs" - value={category} - onChange={(e) => setCategory(e.target.value as any)} - className="w-full" - > + <NativeSelect + size="xs" + value={category} + onChange={(e) => setCategory(e.target.value as FontCategory)} + className="w-full" + >And, if you want to avoid re-running
filterthree times per render, you could memoize the counts:const variableCount = React.useMemo( () => fonts.filter((f) => f.axes && f.axes.length > 0).length, [fonts] ); const nonVariableCount = React.useMemo( () => fonts.filter((f) => !f.axes || f.axes.length === 0).length, [fonts] ); const usedInDocCount = React.useMemo( () => fonts.filter((f) => usedFonts.includes(f.family)).length, [fonts, usedFonts] );…and then use those in the JSX.
editor/grida-canvas-react-starter-kit/starterkit-import/from-grida.tsx (1)
1-47: FileDropzone integration and.grida/.jsonvalidation look solidUsing
selectedFile+validateFilewith the sharedFileDropzoneand gatinghandleFileImportonselectedFileresults in a clear, robust flow and fixes “accept filedrop from import grida”/“reroute user message” objectives. ClearingselectedFileon successful import is a nice touch to avoid stale state.Minor optional UX tweak: the label “Select a .grida file” (Line 61) could be updated to mention
.jsonas well to match the supported formats.Also applies to: 62-85
editor/grida-canvas-react-starter-kit/starterkit-import/from-figma.tsx (1)
313-323:.figFileDropzone integration and confirmation flow align wellThe new
.figFileDropzone (withvalidateFile, disabled whileparsingis true, and a “Selected:” +Progressdisplay) is wired cleanly, and the confirm step correctly reflectsselectedFile?.name. Once the auto‑parse loop is fixed as above, this flow should feel smooth.Also applies to: 324-332, 335-342
editor/grida-canvas-react/viewport/surface.tsx (2)
1015-1057: Group resize handles: z‑index logic and gesture wiring match the NSWE priority requirementComputing
handleSizeInViewportand switchingnsweZIndexbased onMIN_RESIZE_HANDLE_SIZE_FOR_DIAGONAL_PRIORITY_UI_SIZEcorrectly prioritizes the NSWE (side) handles over diagonals when the overlay is small, and vice‑versa when large. The sideLayerOverlayResizeSidecomponents now own the drag behavior while the NSWELayerOverlayResizeHandles are rendered but disabled (visual only), which is a sensible split for groups.Also applies to: 1066-1153
1519-1565: Resize handle components: zIndex, disabled, and double‑click behavior are well-factored
LayerOverlayResizeHandleandLayerOverlayResizeSidenow take explicitzIndex,onDragStart,onDoubleClick, anddisabledprops, with gesture options guarding drag enablement via!disabled && !!onDragStart. The useSurfaceGesture bindings prevent default on pointer/drag and stop propagation for double‑clicks, which should reduce accidental underlying interactions and keep behavior predictable.Also applies to: 1567-1581, 1582-1603
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
editor/grida-canvas-react-starter-kit/starterkit-import/file-dropzone.tsx(1 hunks)editor/grida-canvas-react-starter-kit/starterkit-import/from-figma.tsx(6 hunks)editor/grida-canvas-react-starter-kit/starterkit-import/from-grida.tsx(4 hunks)editor/grida-canvas-react/ui-config.ts(1 hunks)editor/grida-canvas-react/use-data-transfer.ts(1 hunks)editor/grida-canvas-react/viewport/hooks/use-surface-gesture.ts(2 hunks)editor/grida-canvas-react/viewport/surface.tsx(16 hunks)editor/grida-canvas-react/viewport/ui/rule.tsx(0 hunks)editor/grida-canvas/ASSERTIONS.md(1 hunks)editor/grida-canvas/editor.i.ts(5 hunks)editor/grida-canvas/editor.ts(1 hunks)editor/grida-canvas/reducers/surface.reducer.ts(2 hunks)editor/scaffolds/sidecontrol/controls/font-family.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- editor/grida-canvas-react/viewport/ui/rule.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/ui-config.tseditor/grida-canvas-react-starter-kit/starterkit-import/file-dropzone.tsxeditor/grida-canvas-react/viewport/hooks/use-surface-gesture.tseditor/grida-canvas/reducers/surface.reducer.tseditor/grida-canvas-react-starter-kit/starterkit-import/from-figma.tsxeditor/grida-canvas/editor.tseditor/grida-canvas-react/use-data-transfer.tseditor/grida-canvas-react-starter-kit/starterkit-import/from-grida.tsxeditor/scaffolds/sidecontrol/controls/font-family.tsxeditor/grida-canvas/editor.i.tseditor/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/ui-config.tseditor/grida-canvas-react-starter-kit/starterkit-import/file-dropzone.tsxeditor/grida-canvas-react/viewport/hooks/use-surface-gesture.tseditor/grida-canvas/reducers/surface.reducer.tseditor/grida-canvas-react-starter-kit/starterkit-import/from-figma.tsxeditor/grida-canvas/editor.tseditor/grida-canvas-react/use-data-transfer.tseditor/grida-canvas-react-starter-kit/starterkit-import/from-grida.tsxeditor/scaffolds/sidecontrol/controls/font-family.tsxeditor/grida-canvas/editor.i.tseditor/grida-canvas-react/viewport/surface.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use React.js 19 for web UI development
Files:
editor/grida-canvas-react-starter-kit/starterkit-import/file-dropzone.tsxeditor/grida-canvas-react-starter-kit/starterkit-import/from-figma.tsxeditor/grida-canvas-react-starter-kit/starterkit-import/from-grida.tsxeditor/scaffolds/sidecontrol/controls/font-family.tsxeditor/grida-canvas-react/viewport/surface.tsx
🧠 Learnings (10)
📚 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/ui-config.tseditor/grida-canvas-react-starter-kit/starterkit-import/from-grida.tsxeditor/grida-canvas/editor.i.tseditor/grida-canvas-react/viewport/surface.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/ui-config.tseditor/grida-canvas-react/use-data-transfer.tseditor/grida-canvas-react-starter-kit/starterkit-import/from-grida.tsxeditor/grida-canvas-react/viewport/surface.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/ui-config.tseditor/grida-canvas/reducers/surface.reducer.tseditor/scaffolds/sidecontrol/controls/font-family.tsxeditor/grida-canvas-react/viewport/surface.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 Canvas 2D API with path commands for rendering geometric shapes (circle, square, triangle, etc.)
Applied to files:
editor/grida-canvas/reducers/surface.reducer.tseditor/grida-canvas-react/viewport/surface.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 : 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/reducers/surface.reducer.tseditor/grida-canvas-react/viewport/surface.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-starter-kit/starterkit-import/from-figma.tsxeditor/grida-canvas-react-starter-kit/starterkit-import/from-grida.tsxeditor/grida-canvas-react/viewport/surface.tsx
📚 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/grida-canvas-react-starter-kit/starterkit-import/from-figma.tsxeditor/grida-canvas-react-starter-kit/starterkit-import/from-grida.tsxeditor/scaffolds/sidecontrol/controls/font-family.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/**/*.{js,ts,jsx,tsx} : Use Web Workers for heavy graphics operations to improve performance and responsiveness
Applied to files:
editor/grida-canvas-react/use-data-transfer.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-import/from-grida.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 : Cache ImageData and dimensions in refs (imageDataRef, sizeRef) for efficient exports
Applied to files:
editor/grida-canvas-react/viewport/surface.tsx
🧬 Code graph analysis (7)
editor/grida-canvas/reducers/surface.reducer.ts (1)
packages/grida-canvas-schema/grida.ts (1)
PropsPaintValue(1391-1391)
editor/grida-canvas-react-starter-kit/starterkit-import/from-figma.tsx (1)
editor/grida-canvas-react-starter-kit/starterkit-import/file-dropzone.tsx (1)
FileDropzone(20-114)
editor/grida-canvas/editor.ts (2)
editor/grida-canvas/editor.i.ts (1)
NodeID(136-136)packages/grida-canvas-schema/grida.ts (2)
NodeID(1101-1101)ContainerNode(2165-2180)
editor/grida-canvas-react-starter-kit/starterkit-import/from-grida.tsx (1)
editor/grida-canvas-react-starter-kit/starterkit-import/file-dropzone.tsx (1)
FileDropzone(20-114)
editor/scaffolds/sidecontrol/controls/font-family.tsx (1)
editor/grida-canvas/query/index.ts (1)
fonts(606-614)
editor/grida-canvas/editor.i.ts (2)
editor/grida-canvas/editor.ts (3)
state(327-329)state(2445-2447)state(3511-3513)packages/grida-canvas-schema/grida.ts (1)
NodeID(1101-1101)
editor/grida-canvas-react/viewport/surface.tsx (6)
editor/grida-canvas-react/ui-config.ts (4)
MIN_NODE_OVERLAY_GAP_VISIBLE_UI_SIZE(26-26)MIN_NODE_OVERLAY_PADDING_VISIBLE_UI_SIZE(33-33)MIN_RESIZE_HANDLE_SIZE_FOR_DIAGONAL_PRIORITY_UI_SIZE(48-48)MIN_NODE_OVERLAY_CORNER_RADIUS_VISIBLE_UI_SIZE(19-19)editor/grida-canvas-react/provider.tsx (1)
useTransformState(369-387)editor/grida-canvas-react/viewport/hooks/use-surface-gesture.ts (1)
useSurfaceGesture(4-49)editor/grida-canvas-react/viewport/ui/knob.tsx (1)
Knob(7-70)editor/grida-canvas-react/viewport/context.ts (1)
useViewport(11-19)editor/grida-canvas-react/viewport/surface-hooks.ts (1)
useSurfaceSelectionGroups(128-136)
🔇 Additional comments (14)
editor/scaffolds/sidecontrol/controls/font-family.tsx (2)
11-13: NativeSelect integration looks consistent with the new UI patternImporting
NativeSelectandNativeSelectOptionfrom the sharedui-editor/native-selectmodule is appropriate here and keeps this control aligned with the rest of the scaffold UI.
212-217: Search result banner behavior is correct and UX-friendlyConditionally showing “X font(s) found” only when there is a query and at least one result avoids noise, and the singular/plural handling is correct.
editor/grida-canvas-react/ui-config.ts (1)
42-48: Resize handle threshold constant looks goodThe naming, comment, and value make the intent clear and align with the documented diagonal vs. side handle priority behavior. No issues from a code perspective.
editor/grida-canvas-react/use-data-transfer.ts (1)
621-625: .grida drop handling matches intended flowThe additional guard for
.gridafiles is consistent with the existing.fighandling, and thecontinueensures they are excluded from the generic insert pipeline while giving clear guidance to the user.editor/grida-canvas-react/viewport/hooks/use-surface-gesture.ts (1)
12-15: Enabled flag integration intouseGestureis reasonableDeriving
enabledfromconfig?.enabled !== falseand overriding it in the config object gives a clear, opt-out toggle while preserving existing behavior for callers that don’t passenabled. This should work as long as@use-gesture/reactin this repo supports theenabledconfig option.Please confirm your installed
@use-gesture/reactversion documentsenabled?: booleanon the config object to avoid subtle runtime or typing mismatches. You can verify this via the package’s docs or type definitions.Also applies to: 44-47
editor/grida-canvas/ASSERTIONS.md (1)
33-43: New UX assertions are clear and align with implementationThe added entries document the intended behaviors for handle z-order, direct auto-layout application, and guideline input gating in a way that matches the new constants and helper APIs. This is a good reference for manual UX verification.
editor/grida-canvas/editor.ts (1)
1189-1223: Direct auto-layout application path looks consistent but changes semanticsThe new
autoLayoutimplementation cleanly adds a short-circuit for"selection"when a single non-flex container is selected, dispatching{ type: "autolayout", contain: false, target: node_id }and otherwise preserving the previous wrapping behavior (contain: true). This matches the documented API and UX intent for Shift+A.Because this subtly changes the default behavior of
autoLayout("selection")for single containers, it’s worth double-checking that:
- The
autolayoutreducer correctly handlescontain: falsewith a containertarget, and- All call sites that rely on wrapping semantics either pass
prefersDirectApplication = falseor are happy with the new default.editor/grida-canvas/editor.i.ts (2)
1076-1089:eager_canvas_inputhelper cleanly centralizes input gating logicThe helper’s condition (
content_edit_mode !== undefined || tool.type === "insert") matches the described “eager canvas input” semantics and uses a minimalPick<IEditorState, "content_edit_mode" | "tool">type, which keeps it easy to call from reducers or UI.
2805-2874: Auto-layout API/docs now accurately describe direct-application behaviorThe extended JSDoc and updated
autoLayout(target, prefersDirectApplication?)signature align with the concrete implementation inEditorDocumentStore.autoLayout, including theprefersDirectApplicationdefault and its effect on single-container selections versus wrapping. This keeps the public API contract in sync with behavior.editor/grida-canvas/reducers/surface.reducer.ts (1)
452-460: Guide gestures correctly gated behindeager_canvas_inputEarly-returning when
editor.state.eager_canvas_input({ content_edit_mode: draft.content_edit_mode, tool: draft.tool })is true prevents starting guide gestures during content edit or insert modes, which matches the desired UX for avoiding accidental guide interactions. Looks correct and self‑contained.editor/grida-canvas-react-starter-kit/starterkit-import/file-dropzone.tsx (1)
1-113: Reusable FileDropzone is well-structured and consistentDrag‑and‑drop + picker flows are wired correctly, validation is centralized via
validateFile, and the disabled/loading state is consistently honored (isProcessing). The API surface (accept, onFileSelected, customizable texts/classes) is clean and should work well for other importers too.editor/grida-canvas-react/viewport/surface.tsx (3)
887-917: Using UI-space size for gap/padding overlay visibility is a good callDeriving
rect_ui_width/heightfromsize * scaleand using them to gateGapOverlayandPaddingOverlayavoids noisy UI when the selection is tiny or heavily zoomed out. This should make overlays feel much more usable at different zoom levels.
1196-1293: Single-node resize: NSWE vs diagonal z‑order and text auto‑size double‑clicks are implemented cleanly
- Using
rect_ui_width/heightandhandleSizeInViewportto computensweZIndexvsdiagonalZIndexachieves the intended NSWE‑over‑diagonal priority when zoomed out, while still letting diagonals take precedence at larger sizes.- Side double‑click handlers correctly route to
editor.autoSizeTextNode(node_id, "width" | "height")only for text nodes, and the NE/SE/SW diagonal double‑click handler resizing both width and height matches the PR requirement.- The line‑node special case (east/west only) remains intact and now also benefits from horizontal double‑click auto‑size.
This all looks consistent with the UX goals.
Also applies to: 1300-1430, 1450-1453
1758-1785: Ruler and guide interactions correctly gated byeager_canvas_inputUsing
useEditorState(editorInstance, state => editor.state.eager_canvas_input(state))to:
- disable drag gestures on ruler bars and guides (
enabled: !eager_canvas_input), and- apply
pointerEvents: "none"styles when eager input is activeensures users can’t accidentally create or move guides while inserting or in content edit modes. Combined with the reducer guard on
"guide"gestures, this gives a coherent, centralized gating model for guide UX.Also applies to: 1922-1932, 1936-1971, 1973-1980
NE,SE,SWknobs be able to resize-to-fit for bothwidthandheightSummary by CodeRabbit
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.