[Grida Canvas] Daily RC #358
Conversation
…-bug Fix image drop size
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
|
Warning Rate limit exceeded@softmarshmallow has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 38 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
""" WalkthroughThis update introduces a new, fully styled Select dropdown UI component suite based on Radix UI, updates imports to use this new component, and refines UI behaviors in comboboxes and context menus. The editor's configuration now supports a customizable rotation quantization step, with corresponding reducer and transform logic adjustments. Scene duplication is enhanced to deeply clone node hierarchies. Additionally, the canvas UI is wrapped with a global editor context provider. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GridaCanvas
participant WindowCurrentEditorProvider
participant EditorContext
User->>GridaCanvas: Render Canvas Page
GridaCanvas->>WindowCurrentEditorProvider: Wrap canvas UI
WindowCurrentEditorProvider->>EditorContext: Provide global editor context
GridaCanvas->>User: Display canvas with context
sequenceDiagram
participant User
participant VirtualizedCombobox
participant NativeButton
User->>VirtualizedCombobox: Interact with combobox
VirtualizedCombobox->>NativeButton: Render trigger with new styling
NativeButton->>User: Show combobox options
sequenceDiagram
participant User
participant Editor
participant Reducer
participant Scene
participant Node
User->>Editor: Duplicate Scene
Editor->>Reducer: Handle duplicate action
Reducer->>Scene: Create new scene
loop For each child node
Scene->>Node: Clone node hierarchy recursively
Node->>Scene: Insert cloned sub-document
end
Reducer->>Editor: Update state with duplicated scene and nodes
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…y-nodes-recursively Fix scene duplication to deep copy nodes
…on-quantization Add rotation quantization step
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
editor/components/ui/virtualized-combobox.tsx (1)
181-190: UI component refactoring looks good with minor suggestion.The refactoring from
Buttonto nativebuttonelement with utility classes aligns well with the broader standardization effort mentioned in the summary. The accessibility attributes (role="combobox",aria-expanded) are properly maintained.However, consider adding explicit
type="button"to prevent potential form submission issues:<button + type="button" role="combobox" aria-expanded={open} className={cn("flex w-full justify-between items-center", className)} >editor/components/ui-editor/select.tsx (1)
31-55: SelectTrigger implementation with good flexibility.The
SelectTriggercomponent provides good customization with size variants and properly forwards all props. The extensive className string handles various states and styling concerns comprehensively.Consider extracting the complex className into a separate function or using a more readable approach for better maintainability:
+const triggerClasses = (size: "sm" | "default") => cn( + "border-input line-clamp-1 data-[placeholder]:text-muted-foreground", + "[&_svg:not([class*='text-'])]:text-muted-foreground", + "focus-visible:border-ring focus-visible:ring-ring/50", + // ... rest of classes + size === "default" ? "h-9" : "h-8" +); className={cn( - "border-input line-clamp-1 data-[placeholder]:text-muted-foreground [&_svg:not([class*='text-'])]:text-muted-foreground focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 dark:hover:bg-input/50 flex w-fit items-center justify-between gap-2 rounded-md border bg-transparent px-3 py-2 text-sm whitespace-nowrap shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50 data-[size=default]:h-9 data-[size=sm]:h-8 *:data-[slot=select-value]:line-clamp-1 *:data-[slot=select-value]:flex *:data-[slot=select-value]:items-center *:data-[slot=select-value]:gap-2 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", + triggerClasses(size), className )}editor/grida-canvas/features.md (1)
108-113: Fix markdown formatting for the section heading.The static analysis tool correctly identified a formatting issue. "Edge Scrolling" should be a proper heading instead of using emphasis.
Apply this diff to fix the formatting:
-**Edge Scrolling** +### Edge Scrolling🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
108-108: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
editor/grida-canvas-react/provider.tsx (1)
1075-1076: Address the TODO for uploader implementation.The TODO comment indicates missing uploader functionality. Consider implementing this or creating an issue to track it.
Would you like me to help design the uploader interface or create an issue to track this enhancement?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
editor/app/(workbench)/[org]/[proj]/[id]/canvas/page.tsx(2 hunks)editor/components/ui-editor/select.tsx(1 hunks)editor/components/ui/virtualized-combobox.tsx(1 hunks)editor/grida-canvas-react/devtools/global-api-host.tsx(1 hunks)editor/grida-canvas-react/provider.tsx(1 hunks)editor/grida-canvas-react/viewport/surface-context-menu.tsx(2 hunks)editor/grida-canvas/features.md(2 hunks)editor/grida-canvas/index.ts(5 hunks)editor/grida-canvas/reducers/index.ts(2 hunks)editor/grida-canvas/reducers/methods/transform.ts(2 hunks)editor/scaffolds/playground-canvas/playground.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/cursor.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/font-size.tsx(2 hunks)editor/scaffolds/sidecontrol/ui/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
editor/app/(workbench)/[org]/[proj]/[id]/canvas/page.tsx (1)
editor/grida-canvas-react/devtools/global-api-host.tsx (1)
WindowCurrentEditorProvider(16-19)
editor/components/ui/virtualized-combobox.tsx (1)
editor/components/lib/utils/index.ts (1)
cn(4-6)
editor/components/ui-editor/select.tsx (1)
editor/components/lib/utils/index.ts (1)
cn(4-6)
editor/grida-canvas/index.ts (1)
packages/grida-canvas-schema/grida.ts (1)
IDocumentTemplatesRepository(247-252)
🪛 markdownlint-cli2 (0.17.2)
editor/grida-canvas/features.md
108-108: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
🔇 Additional comments (25)
editor/grida-canvas-react/devtools/global-api-host.tsx (1)
1-1: LGTM! Good import optimization.The change to import only the specific
useEffecthook instead of the default React import is a good optimization that improves tree-shaking and reduces bundle size without affecting functionality.editor/components/ui/virtualized-combobox.tsx (1)
186-186: Verify text truncation behavior change.The class change from
"text-ellipsis overflow-hidden"to"line-clamp-1 text-left"modifies how text truncation works. Please ensure this change maintains the intended visual behavior, especially for long option text.editor/grida-canvas/index.ts (3)
59-64: Well-documented configuration addition.The new
rotation_quantize_stepconfiguration property is properly documented with clear JSDoc comments describing its purpose and default value. Good implementation.
98-101: Consistent naming and default value.The
DEFAULT_ROTATION_QUANTIZE_STEPconstant follows naming conventions and the default value of 1 degree is sensible for fine-grained rotation control.
562-562:Details
✅ Verification successful
Verify integration with transform logic.
The rotation quantization step is properly added to the state interfaces and initialization. Please ensure this new configuration is actually used in the rotation gesture transform logic mentioned in the summary.
Also applies to: 672-674
🏁 Script executed:
#!/bin/bash # Description: Verify that rotation_quantize_step is used in transform.ts # Test: Search for usage of rotation_quantize_step in transform logic rg -A 5 -B 5 "rotation_quantize_step" --type tsLength of output: 4507
rotation_quantize_step integration verified
Verified that the
rotation_quantize_stepconfig is pulled from state and applied in the rotation gesture logic:
- In editor/grida-canvas/index.ts (initialization):
rotation_quantize_step: init.rotation_quantize_step ?? editor.config.DEFAULT_ROTATION_QUANTIZE_STEP- In editor/grida-canvas/reducers/methods/transform.ts:
const { rotation_quantize_step } = draft; const _user_q = typeof rotate_with_quantize === "number" ? rotate_with_quantize : rotation_quantize_step; const q = Math.max(0.01, _user_q); const angle = cmath.quantize(_angle, q);No further changes required.
editor/components/ui-editor/select.tsx (4)
1-11: Well-structured imports and setup.The component uses proper "use client" directive for Next.js, imports necessary Radix primitives and icons, and includes the utility function for conditional styling. Good foundation.
57-90: SelectContent with proper portal usage and animations.The
SelectContentcomponent properly uses Radix's Portal for rendering and includes comprehensive animation classes. The conditional positioning logic for "popper" mode is well-implemented.
105-127: SelectItem with good accessibility and visual feedback.The
SelectItemcomponent includes proper accessibility attributes and visual feedback with the check icon indicator. The styling handles various states appropriately.
178-189: Complete and well-organized exports.All necessary components are properly exported, providing a complete Select component API that can replace the previous Select implementation across the codebase.
editor/scaffolds/playground-canvas/playground.tsx (1)
126-126:Details
✅ Verification successful
Verify the new import path exists and exports the correct component.
The import path for
WindowCurrentEditorProviderhas been updated. Please ensure the new module exports the same component interface.
🏁 Script executed:
#!/bin/bash # Description: Verify that the new import path exists and exports WindowCurrentEditorProvider # Check if the new file exists if [ -f "editor/grida-canvas-react/devtools/global-api-host.tsx" ] || [ -f "editor/grida-canvas-react/devtools/global-api-host/index.tsx" ]; then echo "✓ New import path file exists" else echo "✗ New import path file not found" fi # Search for WindowCurrentEditorProvider export in the new location fd -e tsx -e ts -e js "global-api-host" --exec grep -l "WindowCurrentEditorProvider" # Check if the old path still exists (potential inconsistency) if [ -f "editor/grida-canvas-react/devtools/api-host.tsx" ] || [ -f "editor/grida-canvas-react/devtools/api-host/index.tsx" ]; then echo "⚠ Old import path still exists - verify migration is complete" fd -e tsx -e ts -e js "api-host" --exec grep -l "WindowCurrentEditorProvider" fiLength of output: 404
Import Path Verified for WindowCurrentEditorProvider
- ✓
editor/grida-canvas-react/devtools/global-api-host.tsxexists- ✓ Exports
WindowCurrentEditorProvider- ⚡ Old
api-hostmodule no longer presentNo further changes needed.
editor/scaffolds/sidecontrol/controls/cursor.tsx (1)
7-7: LGTM! Consistent migration to the new Select component module.This change aligns with the broader migration to the new
ui-editor/selectmodule seen across other files. The component usage remains unchanged, indicating good API compatibility.editor/grida-canvas-react/viewport/surface-context-menu.tsx (2)
64-64: LGTM! Improved context menu width consistency.Changing from
min-w-52tow-52provides a more consistent fixed width for the context menu, improving the overall UI layout.
158-158: LGTM! Added text truncation for better overflow handling.Adding the
truncateclass prevents long ID lists from overflowing the container, improving the visual presentation when multiple items are selected.editor/grida-canvas/features.md (1)
94-94: LGTM! Good documentation of the new default rotation quantization.This addition properly documents the new default 1-degree rotation quantization behavior that aligns with the implementation changes mentioned in the AI summary.
editor/app/(workbench)/[org]/[proj]/[id]/canvas/page.tsx (2)
21-21: LGTM! Clean import of the global API host provider.The import path correctly references the devtools module for global editor context management.
40-40: LGTM! Proper placement of the WindowCurrentEditorProvider.The component is correctly placed at the top level of the GridaCanvas component, allowing it to establish global editor context before other components render. Since it returns
null, it won't affect the UI structure.editor/grida-canvas-react/provider.tsx (1)
1077-1099: Excellent enhancement to image dimension detection!The async approach to determine natural image dimensions before node creation is a significant improvement over the previous "auto" sizing. The implementation correctly:
- Uses
Image.onloadto get natural dimensions when successful- Provides sensible 100x100 fallback dimensions on error
- Maintains proper error handling flow
This ensures more accurate initial sizing of inserted images.
editor/scaffolds/sidecontrol/controls/font-size.tsx (3)
1-5: LGTM! Clean migration to the new Select component.The import update correctly uses the new ui-editor Select components that provide enhanced styling and composability based on Radix UI primitives.
8-8: LGTM! Consistent icon choice.The switch from
CaretDownIcontoChevronDownIconaligns with the new UI component standards.
44-46:Details
❓ Verification inconclusive
Verify accessibility of the native button element.
The refactor from
Buttoncomponent to nativebuttonelement is more lightweight, but ensure that accessibility features (focus indicators, screen reader support) are maintained through the applied CSS classes.
🏁 Script executed:
#!/bin/bash # Description: Check if the new Select UI components maintain proper accessibility attributes # Expected: Verify that the new select components provide proper ARIA attributes and keyboard navigation # Search for accessibility attributes in the new select component ast-grep --pattern $'export function Select($_) { $$$ }' # Check for ARIA attributes in select-related components rg -A 10 -B 5 'aria-|role=|tabIndex' editor/components/ui-editor/select.tsxLength of output: 2389
Ensure Accessible Native Button
Please confirm that this native
<button>ineditor/scaffolds/sidecontrol/controls/font-size.tsx(lines 44–46) retains a clear focus indicator and full screen-reader support. The previousButtoncomponent may have injected focus-ring styles and aria attributes that are now missing.• File: editor/scaffolds/sidecontrol/controls/font-size.tsx (lines 44–46)
• Add or verify focus-visible styles, e.g.
focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-primary
• Ensure the button has an accessible name (either descriptive inner text oraria-label)
• Test keyboard navigation (Tab focus) and screen-reader announcement of the controleditor/grida-canvas/reducers/methods/transform.ts (2)
598-601: LGTM! The rotation quantization logic is well-implemented.The conditional logic correctly uses the configurable
rotation_quantize_stepas a fallback whenrotate_with_quantizeis not a number, providing better user control over rotation behavior.
589-589:Details
✅ Verification successful
Verify that
rotation_quantize_stepis properly initialized in the editor state.The destructuring looks correct, but ensure that
rotation_quantize_stepis always available in the draft state to prevent runtime errors.Let me verify that this property is properly defined in the editor state interface:
🏁 Script executed:
#!/bin/bash # Description: Check if rotation_quantize_step is defined in editor state interfaces # Expected: Find the property definition with default value rg -A 3 -B 3 "rotation_quantize_step" --type tsLength of output: 2921
rotation_quantize_step is properly initialized in editor state
rotation_quantize_stepis declared onIEditorStateand, ineditor/grida-canvas/index.ts, set toinit.rotation_quantize_step ?? editor.config.DEFAULT_ROTATION_QUANTIZE_STEP(default 1). It’s always available ondraft, so the destructuring and fallback logic intransform.tsis safe.editor/grida-canvas/reducers/index.ts (3)
6-6: LGTM! Import addition is correct.The import of
self_insertSubDocumentis properly added to support the new scene cloning functionality.
115-115: Correct initialization of empty children array.Starting with an empty children array is the right approach since nodes will be cloned and added subsequently.
126-138: Excellent implementation of deep scene cloning.The recursive node cloning logic is well-structured and follows established patterns:
- Uses factory methods for creating prototypes and packed documents
- Properly generates new IDs with
nid()- Correctly inserts sub-documents with
self_insertSubDocumentThis provides proper scene duplication with independent node hierarchies.
Consider adding error handling for edge cases:
+ try { for (const child_id of origin.children) { const prototype = grida.program.nodes.factory.createPrototypeFromSnapshot( state.document, child_id ); const sub = grida.program.nodes.factory.create_packed_scene_document_from_prototype( prototype, nid ); self_insertSubDocument(draft, null, sub); } + } catch (error) { + console.error('Failed to clone scene nodes:', error); + // Could revert to empty scene or show user notification + }
| SelectTrigger, | ||
| SelectValue, | ||
| } from "@/components/ui/select"; | ||
| } from "@/components/ui-editor/select"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify compatibility of the new Select component module.
The import has been migrated to the new ui-editor/select module. Please ensure the new Select components maintain API compatibility.
🏁 Script executed:
#!/bin/bash
# Description: Verify the new ui-editor/select module exists and exports the required components
# Check if the new Select module exists
if [ -f "editor/components/ui-editor/select.tsx" ] || [ -f "editor/components/ui-editor/select/index.tsx" ]; then
echo "✓ New ui-editor/select module exists"
# Check for required exports
rg -A 3 "export.*Select" editor/components/ui-editor/select.tsx 2>/dev/null || \
rg -A 3 "export.*Select" editor/components/ui-editor/select/index.tsx 2>/dev/null
else
echo "✗ New ui-editor/select module not found"
fi
# Search for any remaining imports of the old ui/select module
echo "Checking for remaining old imports..."
rg '@/components/ui/select' --type tsx --type ts | head -5Length of output: 444
🏁 Script executed:
#!/bin/bash
echo "Imports in editor/scaffolds/sidecontrol/ui/index.tsx:"
rg -n "from \"@/components/ui-editor/select\"" -C2 editor/scaffolds/sidecontrol/ui/index.tsx
echo
echo "Exports in editor/components/ui-editor/select.tsx:"
rg -n "^export " -C2 editor/components/ui-editor/select.tsx
echo
echo "Searching for any remaining imports of the old ui/select:"
rg "@/components/ui/select" | head -n5Length of output: 1226
🏁 Script executed:
#!/bin/bash
# Show the full export block from ui-editor/select.tsx to confirm all exported components
rg -A10 -n '^export {' editor/components/ui-editor/select.tsxLength of output: 145
🏁 Script executed:
#!/bin/bash
# Display the complete export block from ui-editor/select.tsx for verification
rg -n "export \\{" -C20 editor/components/ui-editor/select.tsx || \
grep -n "export {" -C20 editor/components/ui-editor/select.tsxLength of output: 825
New Select module exports all required components – migration in this file is correct.
We confirmed that editor/components/ui-editor/select.tsx exports:
- Select
- SelectContent
- SelectGroup
- SelectItem
- SelectLabel
- SelectScrollDownButton
- SelectScrollUpButton
- SelectSeparator
- SelectTrigger
- SelectValue
The imports in editor/scaffolds/sidecontrol/ui/index.tsx (SelectTrigger, SelectValue) are present in the new module. No further changes are needed in this file.
However, other scaffold files still reference the old module. Please update these imports to the new path @/components/ui-editor/select:
- editor/scaffolds/sidebar/sidebar-mode-data.tsx
- editor/scaffolds/template-editor/template-editor.tsx
- editor/scaffolds/sidecontrol/sidecontrol-global.tsx
- editor/scaffolds/analytics/stats/index.tsx
- editor/scaffolds/settings/form-method-preference.tsx
🤖 Prompt for AI Agents
In editor/scaffolds/sidecontrol/ui/index.tsx at line 9, the import from the new
module "@/components/ui-editor/select" is correct and requires no changes.
However, update the imports in the following scaffold files to replace any
references to the old "@/components/ui/select" module with the new
"@/components/ui-editor/select" path:
editor/scaffolds/sidebar/sidebar-mode-data.tsx,
editor/scaffolds/template-editor/template-editor.tsx,
editor/scaffolds/sidecontrol/sidecontrol-global.tsx,
editor/scaffolds/analytics/stats/index.tsx, and
editor/scaffolds/settings/form-method-preference.tsx.
1 participant
Summary by CodeRabbit
New Features
Improvements
UI/Style
Documentation