[Grida Canvas] Editor API (Scripting interface V2) #355
Conversation
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
|
WalkthroughThis update refactors the editor's architecture and API, consolidating numerous hooks and state accessors into a unified Changes
Sequence Diagram(s)sequenceDiagram
participant UI_Component
participant useCurrentEditor
participant Editor_Instance
participant Reducer
UI_Component->>useCurrentEditor: Get editor instance
useCurrentEditor-->>UI_Component: Return Editor_Instance
UI_Component->>Editor_Instance: Call action (e.g., insertNode, changeNodeText)
Editor_Instance->>Reducer: Dispatch action (e.g., "node/change/*")
Reducer->>Reducer: Apply property update via property handler
Reducer-->>Editor_Instance: Return updated state
Editor_Instance-->>UI_Component: State updates via subscription/hooks
Possibly related PRs
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 (
|
![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
🧹 Nitpick comments (13)
editor/grida-canvas/backends/dom.ts (1)
6-7: Enhance deprecation notice with migration guidanceThe
@deprecatedtag is appropriate, but it would be more helpful to consumers if you point them to the new API and indicate a removal timeline. For example:/** * @deprecated Use the `Editor` class via the `useCurrentEditor` hook instead. * Scheduled for removal in v3.0. */This makes it clear what to use going forward and when
domapiwill be removed.editor/grida-canvas/animation.ts (1)
3-38: Consider enhancing the animation utility for production use.The implementation correctly interpolates between transform matrices, but could benefit from these improvements:
- Cancellation mechanism: Add ability to cancel ongoing animations
- Configurable duration: Make the 200ms duration a parameter
- Easing functions: Consider non-linear interpolation options
- Input validation: Validate that transforms are valid 2x3 matrices
export function animateTransformTo( from: cmath.Transform, to: cmath.Transform, - update: (t: cmath.Transform) => void + update: (t: cmath.Transform) => void, + options: { duration?: number; easing?: (t: number) => number } = {} ) { - const duration = 200; // ms + const duration = options.duration ?? 200; + const easing = options.easing ?? ((t) => t); // linear by default const start = performance.now(); + let cancelled = false; function step(now: number) { + if (cancelled) return; const elapsed = now - start; - const progress = Math.min(elapsed / duration, 1); + const progress = easing(Math.min(elapsed / duration, 1)); // ... rest of implementation } + return () => { cancelled = true; }; // Return cancellation function }editor/grida-canvas-react/renderer.tsx (1)
140-140: Fix the banned{}type usage.The static analysis tool correctly identifies that
{}should not be used as a type, as it means "any non-nullable value" rather than an empty object.-export function Transformer({ children }: React.PropsWithChildren<{}>) { +export function Transformer({ children }: React.PropsWithChildren) { -export function AutoInitialFitTransformer({ - children, -}: React.PropsWithChildren<{}>) { +export function AutoInitialFitTransformer({ + children, +}: React.PropsWithChildren) {
React.PropsWithChildrenalready has a default generic parameter ofunknown, so no explicit type parameter is needed.Also applies to: 165-165
🧰 Tools
🪛 Biome (1.9.4)
[error] 140-140: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
editor/grida-canvas-react/devtools/api-host.tsx (1)
4-14: Good devtools integration, but optimize the cleanup.The global editor exposure for devtools is well-implemented with proper cleanup. However, the
deleteoperator can impact performance.Apply this diff to improve performance:
- delete globalThis["grida"]; + globalThis["grida"] = undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 11-11: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/grida-canvas-schema/grida.ts (1)
1190-1191: Clarify the nullable behavior intention.The change from
fill?: T;tofill?: T | undefined;is somewhat redundant since optional properties already includeundefined. The FIXME comment suggests this may need to supportnullvalues as well. Consider clarifying whether the property should supportnull,undefined, or both, and update the type accordingly.If
nullsupport is needed:- // FIXME: make it nullable - fill?: T | undefined; + fill?: T | null | undefined;Or if explicit
undefinedisn't needed:- // FIXME: make it nullable - fill?: T | undefined; + fill?: T | null;editor/grida-canvas-react-starter-kit/starterkit-hierarchy/index.tsx (1)
317-323: Consider consistency in state access patterns.The data loader uses direct state access (
editor.state.document.nodesandeditor.state.document_ctx.__ctx_nid_to_children_ids). While this might be intentional for performance in the synchronous data loader, consider if there should be a consistent pattern for accessing state throughout the codebase.If performance is not critical here, consider using state selectors for consistency:
dataLoader: { getItem(itemId) { return useEditorState(editor, (state) => state.document.nodes[itemId]); }, getChildren: (itemId) => { if (itemId === "<root>") { return children; } return useEditorState(editor, (state) => state.document_ctx.__ctx_nid_to_children_ids[itemId]); }, },editor/scaffolds/sidecontrol/sidecontrol-node-selection.tsx (2)
231-231: Remove commented-out code.This commented line appears to be leftover from refactoring and should be removed to keep the code clean.
const sid = ids.join(","); - // const is_root = ids.length === 0 && scene.children.includes(ids[0]); // assuming when root is selected, only root is selected const types = new Set(nodes.map((n) => n.type));
688-735: Consider optimizing the useNodeState selector for better performance.The current selector returns many properties, which could cause unnecessary re-renders when any property changes. Consider splitting this into multiple selectors or using more granular hooks for different sections.
For example, group related properties:
// Text-related properties const textProps = useNodeState(node_id, (node) => ({ text: node.text, fontFamily: node.fontFamily, fontWeight: node.fontWeight, fontSize: node.fontSize, // ... other text properties })); // Layout-related properties const layoutProps = useNodeState(node_id, (node) => ({ layout: node.layout, direction: node.direction, mainAxisAlignment: node.mainAxisAlignment, // ... other layout properties }));editor/grida-canvas/index.ts (4)
1859-1866: Consider consistent change type usage.The
changeTextNodeLineHeightandchangeTextNodeLetterSpacingmethods useTChange<grida.program.nodes.TextNode["lineHeight"]>while other numeric properties useNumberChange. For consistency, consider usingNumberChangeif these properties are always numeric.If
lineHeightandletterSpacingare always numeric values, consider:- changeTextNodeLineHeight( - node_id: NodeID, - lineHeight: TChange<grida.program.nodes.TextNode["lineHeight"]> - ): void; - changeTextNodeLetterSpacing( - node_id: NodeID, - letterSpacing: TChange<grida.program.nodes.TextNode["letterSpacing"]> - ): void; + changeTextNodeLineHeight( + node_id: NodeID, + lineHeight: NumberChange + ): void; + changeTextNodeLetterSpacing( + node_id: NodeID, + letterSpacing: NumberChange + ): void;
1722-1736: Consider using a generic type parameter for the context property.The
contextproperty is typed asany, which reduces type safety. Consider making the class generic to provide better type information.-export class EditorConsumerVerboseError extends Error { - context: any; - constructor(message: string, context: any) { +export class EditorConsumerVerboseError<TContext = unknown> extends Error { + context: TContext; + constructor(message: string, context: TContext) { super(message); // Pass message to the parent Error class this.name = this.constructor.name; // Set the error name this.context = context; // Attach the context object if (Error.captureStackTrace) { Error.captureStackTrace(this, this.constructor); } } toString(): string { return `${this.name}: ${this.message} - Context: ${JSON.stringify(this.context)}`; } }
1839-1893: Consider applying the change type pattern consistently across all numeric parameters.While some methods like
changeNodeStrokeWidth,changeNodeOpacity, andchangeNodeRotationnow useeditor.api.NumberChange, other methods that accept numeric values still use raw types. For consistency, consider using change types for all numeric parameters.For example, methods like:
changeNodeMaxlength(line 1867-1870)changeFlexContainerNodeGap(line 1889-1892)Could benefit from using
TChange<number>orNumberChangeto support both absolute and delta changes uniformly across the API.
1955-1955: Consider using a more explicit type instead of -1 as a special value.The
startGuideGesturemethod acceptsidx: number | -1, where -1 appears to be a special case. This is a code smell that can lead to confusion.-startGuideGesture(axis: cmath.Axis, idx: number | -1): void; +startGuideGesture(axis: cmath.Axis, idx: number | "new"): void;Or alternatively:
-startGuideGesture(axis: cmath.Axis, idx: number | -1): void; +startGuideGesture(axis: cmath.Axis, idx?: number): void;editor/grida-canvas/editor.ts (1)
120-124: Address TODO: Implement instance-wise ID generation.The current implementation could potentially generate duplicate IDs across different editor instances. Consider implementing an instance-specific ID generator.
Would you like me to implement an instance-wise ID generator that ensures unique IDs across editor instances? I can open a new issue to track this improvement.
Example approach:
private __nodeIdCounter = 0; private __instanceId = Math.random().toString(36).substr(2, 9); private __createNodeId(): editor.NodeID { return `${this.__instanceId}-${++this.__nodeIdCounter}`; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
editor/app/(tools)/(playground)/playground/image/_page.tsx(2 hunks)editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/design/template-duo-001-viewer.tsx(2 hunks)editor/app/(workbench)/[org]/[proj]/[id]/form/start/page.tsx(0 hunks)editor/grida-canvas-react-starter-kit/starterkit-artboard-list/index.tsx(2 hunks)editor/grida-canvas-react-starter-kit/starterkit-hierarchy/index.tsx(17 hunks)editor/grida-canvas-react-starter-kit/starterkit-preview/index.tsx(3 hunks)editor/grida-canvas-react-starter-kit/starterkit-toolbar/brush-toolbar.tsx(6 hunks)editor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsx(5 hunks)editor/grida-canvas-react/devtools/__unsafe-console.tsx(1 hunks)editor/grida-canvas-react/devtools/api-host.tsx(1 hunks)editor/grida-canvas-react/devtools/index.tsx(1 hunks)editor/grida-canvas-react/index.ts(1 hunks)editor/grida-canvas-react/plugins/use-recorder.ts(1 hunks)editor/grida-canvas-react/renderer.tsx(4 hunks)editor/grida-canvas-react/use-editor.tsx(1 hunks)editor/grida-canvas-react/viewport/hooks/use-edge-scrolling.ts(3 hunks)editor/grida-canvas-react/viewport/hotkeys.tsx(16 hunks)editor/grida-canvas-react/viewport/surface-context-menu.tsx(7 hunks)editor/grida-canvas-react/viewport/surface-hooks.ts(5 hunks)editor/grida-canvas-react/viewport/surface.tsx(30 hunks)editor/grida-canvas-react/viewport/ui/measurement.tsx(2 hunks)editor/grida-canvas-react/viewport/ui/path-editor.tsx(2 hunks)editor/grida-canvas-react/viewport/ui/snap.tsx(1 hunks)editor/grida-canvas-react/viewport/ui/text-editor.tsx(2 hunks)editor/grida-canvas/action.ts(2 hunks)editor/grida-canvas/animation.ts(1 hunks)editor/grida-canvas/backends/dom.ts(1 hunks)editor/grida-canvas/editor.ts(4 hunks)editor/grida-canvas/index.ts(3 hunks)editor/grida-canvas/reducers/document.reducer.ts(2 hunks)editor/grida-canvas/reducers/event-target.reducer.ts(2 hunks)editor/grida-canvas/reducers/methods/transform.ts(1 hunks)editor/grida-canvas/reducers/node.reducer.ts(1 hunks)editor/grida-canvas/reducers/surface.reducer.ts(3 hunks)editor/grida-canvas/reducers/tools/id.ts(1 hunks)editor/scaffolds/playground-canvas/playground.tsx(8 hunks)editor/scaffolds/playground-canvas/toolbar.tsx(14 hunks)editor/scaffolds/sidebar/sidebar-node-hierarchy-list.tsx(3 hunks)editor/scaffolds/sidecontrol/controls/ext-zoom.tsx(4 hunks)editor/scaffolds/sidecontrol/controls/font-size.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/letter-spacing.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/line-height.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/opacity.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/rotate.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/stroke-width.tsx(1 hunks)editor/scaffolds/sidecontrol/controls/utils/types.ts(0 hunks)editor/scaffolds/sidecontrol/sidecontrol-doctype-form.tsx(2 hunks)editor/scaffolds/sidecontrol/sidecontrol-doctype-site.tsx(3 hunks)editor/scaffolds/sidecontrol/sidecontrol-document-properties.tsx(3 hunks)editor/scaffolds/sidecontrol/sidecontrol-global.tsx(4 hunks)editor/scaffolds/sidecontrol/sidecontrol-node-selection.tsx(13 hunks)editor/scaffolds/sidecontrol/ui/index.tsx(4 hunks)editor/theme/templates/formcollection/page.tsx(1 hunks)packages/grida-canvas-schema/grida.ts(2 hunks)
💤 Files with no reviewable changes (2)
- editor/app/(workbench)/[org]/[proj]/[id]/form/start/page.tsx
- editor/scaffolds/sidecontrol/controls/utils/types.ts
🧰 Additional context used
🧬 Code Graph Analysis (27)
editor/theme/templates/formcollection/page.tsx (1)
editor/grida-canvas-react/provider.tsx (1)
useDocumentState(722-732)
editor/scaffolds/sidecontrol/sidecontrol-doctype-form.tsx (1)
editor/grida-canvas-react/provider.tsx (1)
useSelectionState(709-715)
editor/grida-canvas-react-starter-kit/starterkit-preview/index.tsx (1)
editor/grida-canvas-react/provider.tsx (2)
useDocumentState(722-732)useCurrentSceneState(754-760)
editor/grida-canvas-react/viewport/ui/path-editor.tsx (1)
editor/grida-canvas-react/provider.tsx (3)
useEditorFlagsState(691-702)useToolState(827-841)useTransformState(762-780)
editor/scaffolds/sidecontrol/controls/font-size.tsx (2)
editor/scaffolds/sidecontrol/controls/utils/types.ts (1)
TMixed(3-3)editor/grida-canvas/index.ts (1)
NumberChange(1751-1751)
editor/grida-canvas/animation.ts (1)
packages/grida-cmath/index.ts (1)
Transform(130-130)
editor/grida-canvas-react/viewport/ui/measurement.tsx (1)
editor/grida-canvas-react/provider.tsx (1)
useTransformState(762-780)
editor/scaffolds/sidebar/sidebar-node-hierarchy-list.tsx (2)
editor/grida-canvas-react/index.ts (1)
useCurrentEditor(1-1)editor/components/ui/sidebar.tsx (1)
SidebarGroupAction(706-706)
editor/scaffolds/sidecontrol/controls/stroke-width.tsx (2)
editor/scaffolds/sidecontrol/controls/utils/types.ts (1)
TMixed(3-3)editor/grida-canvas/index.ts (1)
NumberChange(1751-1751)
editor/scaffolds/sidecontrol/controls/letter-spacing.tsx (2)
editor/scaffolds/sidecontrol/controls/utils/types.ts (1)
TMixed(3-3)editor/grida-canvas/index.ts (1)
NumberChange(1751-1751)
editor/scaffolds/sidecontrol/sidecontrol-global.tsx (1)
editor/grida-canvas-react/index.ts (1)
useCurrentEditor(1-1)
editor/grida-canvas-react/renderer.tsx (2)
editor/grida-canvas-react/provider.tsx (2)
useCurrentSceneState(754-760)useTransformState(762-780)editor/grida-canvas-react/index.ts (2)
useTransformState(9-9)AutoInitialFitTransformer(19-19)
editor/scaffolds/sidecontrol/controls/line-height.tsx (2)
editor/scaffolds/sidecontrol/controls/utils/types.ts (1)
TMixed(3-3)editor/grida-canvas/index.ts (1)
NumberChange(1751-1751)
editor/scaffolds/sidecontrol/sidecontrol-doctype-site.tsx (1)
editor/grida-canvas-react/index.ts (1)
useCurrentEditor(1-1)
editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/design/template-duo-001-viewer.tsx (1)
editor/grida-canvas-react/index.ts (1)
useCurrentEditor(1-1)
editor/scaffolds/sidecontrol/controls/rotate.tsx (2)
editor/scaffolds/sidecontrol/controls/utils/types.ts (1)
TMixed(3-3)editor/grida-canvas/index.ts (1)
NumberChange(1751-1751)
editor/grida-canvas-react/viewport/surface-context-menu.tsx (2)
editor/grida-canvas-react/index.ts (1)
useCurrentEditor(1-1)editor/grida-canvas-react/provider.tsx (2)
useSelectionState(709-715)useCurrentSelection(255-613)
editor/scaffolds/sidecontrol/controls/opacity.tsx (2)
editor/scaffolds/sidecontrol/controls/utils/types.ts (1)
TMixed(3-3)editor/grida-canvas/index.ts (1)
NumberChange(1751-1751)
editor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsx (3)
editor/grida-canvas-react/index.ts (3)
useCurrentEditor(1-1)useToolState(10-10)useEditorFlagsState(11-11)editor/grida-canvas-react/provider.tsx (2)
useToolState(827-841)useEditorFlagsState(691-702)editor/grida-canvas-react-starter-kit/starterkit-toolbar/utils.ts (2)
toolbar_value_to_cursormode(42-70)ToolbarToolType(3-16)
editor/scaffolds/sidecontrol/ui/index.tsx (1)
editor/grida-canvas/index.ts (1)
NumberChange(1751-1751)
editor/scaffolds/playground-canvas/playground.tsx (4)
editor/grida-canvas-react/devtools/api-host.tsx (1)
WindowCurrentEditorProvider(16-19)editor/grida-canvas-react/index.ts (2)
useToolState(10-10)useCurrentEditor(1-1)editor/grida-canvas-react/provider.tsx (2)
useToolState(827-841)useCurrentSceneState(754-760)editor/grida-canvas-react/use-editor.tsx (1)
useCurrentEditor(25-33)
editor/grida-canvas-react/viewport/ui/text-editor.tsx (2)
editor/grida-canvas-react/index.ts (2)
useCurrentEditor(1-1)useTransformState(9-9)editor/grida-canvas-react/provider.tsx (1)
useTransformState(762-780)
editor/grida-canvas-react/viewport/hooks/use-edge-scrolling.ts (2)
editor/grida-canvas-react/index.ts (4)
useCurrentEditor(1-1)useEditorState(1-1)useToolState(10-10)useTransformState(9-9)editor/grida-canvas-react/provider.tsx (3)
useToolState(827-841)useTransformState(762-780)usePointerState(817-820)
editor/grida-canvas-react-starter-kit/starterkit-toolbar/brush-toolbar.tsx (3)
editor/grida-canvas-react/index.ts (2)
useCurrentEditor(1-1)useBrushState(6-6)packages/grida-canvas-bitmap/index.ts (1)
brush(422-483)editor/grida-canvas-react/provider.tsx (1)
useBrushState(843-846)
editor/grida-canvas-react/viewport/hotkeys.tsx (2)
editor/grida-canvas-react/index.ts (2)
useCurrentEditor(1-1)useToolState(10-10)editor/grida-canvas-react/provider.tsx (3)
useToolState(827-841)useA11yActions(903-988)useCurrentSelection(255-613)
editor/grida-canvas-react/viewport/surface.tsx (8)
editor/grida-canvas-react/use-editor.tsx (2)
useCurrentEditor(25-33)useEditorState(35-47)editor/grida-canvas-react/provider.tsx (9)
useTransformState(762-780)useGestureState(997-1016)useSelectionState(709-715)useToolState(827-841)useBrushState(843-846)useDocumentState(722-732)useCurrentSceneState(754-760)usePointerState(817-820)useMultipleSelectionOverlayClick(855-887)packages/grida-cmath/index.ts (4)
transform(523-533)transform(860-868)transform(1308-1344)Transform(130-130)editor/grida-canvas-react/viewport/ui/marquee.tsx (1)
MarqueeArea(15-29)editor/grida-canvas/index.ts (1)
DropzoneIndication(144-152)editor/grida-canvas-react/viewport/ui/knob.tsx (1)
Knob(6-85)editor/grida-canvas-react/viewport/context.ts (1)
useViewport(11-19)editor/grida-canvas-react/viewport/surface-hooks.ts (1)
useSurfaceSelectionGroups(129-137)
editor/grida-canvas/reducers/node.reducer.ts (3)
packages/grida-canvas-schema/grida.ts (3)
UnknwonNode(851-871)UnknownNodeProperties(873-873)Node(789-804)packages/grida-canvas-cg/lib.ts (2)
LinearGradientPaint(317-322)RadialGradientPaint(324-329)editor/grida-canvas/action.ts (1)
NodeChangeAction(594-606)
🪛 Biome (1.9.4)
editor/grida-canvas-react/devtools/api-host.tsx
[error] 11-11: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
editor/grida-canvas-react/renderer.tsx
[error] 167-167: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
editor/grida-canvas/reducers/node.reducer.ts
[error] 367-367: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
editor/grida-canvas/editor.ts
[error] 1551-1569: The constructor should not return a value.
The constructor is here:
Returning a value from a constructor is ignored.
(lint/correctness/noConstructorReturn)
🔇 Additional comments (108)
editor/scaffolds/sidecontrol/sidecontrol-doctype-form.tsx (1)
25-25: LGTM! Clean migration to granular state hook.The change from
useDocumenttouseSelectionStateis consistent with the broader refactor to use more specific state hooks. The new hook provides the sameselectionproperty that the component needs.Also applies to: 56-56
editor/grida-canvas/reducers/tools/id.ts (1)
7-7: Good deprecation practice.The deprecation notice clearly indicates the new recommended approach (
editor.id()) while maintaining backward compatibility. This supports the gradual migration to the centralized editor instance.editor/grida-canvas-react/viewport/ui/measurement.tsx (1)
8-8: LGTM! Consistent with the state management refactor.The migration from
useTransformtouseTransformStatemaintains the same functionality while using the new centralized state pattern. The{ transform }destructuring correctly accesses the transform matrix from the returned object.Also applies to: 67-67
editor/theme/templates/formcollection/page.tsx (1)
20-20: LGTM! Proper migration to state-specific hook.The change from
useDocumenttouseDocumentStatefollows the established refactor pattern of using more granular state hooks. The{ document }destructuring correctly accesses the document object from the new hook's return value.Also applies to: 26-26
editor/grida-canvas-react/devtools/__unsafe-console.tsx (1)
6-6: LGTM: Import reordering aligns with refactoring.The import reordering is clean and the removal of
useEditorApiusage (mentioned in the summary) aligns with the broader migration to the unifieduseCurrentEditorinterface.editor/grida-canvas-react/devtools/index.tsx (2)
23-23: LGTM: Import path reflects component reorganization.The updated import path for
__UNSAFE_CONSOLEcorrectly reflects the reorganization of devtools components.
27-27: LGTM: Hook extraction to plugin location.Moving
useRecorderto a dedicated plugin location (../plugins/use-recorder) improves modularity and separation of concerns.editor/grida-canvas-react-starter-kit/starterkit-preview/index.tsx (3)
5-5: LGTM: Hook replacement aligns with editor state consolidation.Replacing
useDocumentwithuseDocumentStatealigns with the architectural migration to centralize state access throughuseCurrentEditor.
25-25: LGTM: Consistent state hook migration.The replacement of
useCurrentScenewithuseCurrentSceneStatefollows the same pattern as other components and maintains the unified editor interface.
47-48: LGTM: Hook usage properly updated.The destructuring assignments have been correctly updated to use the new state hooks while maintaining the same interface and functionality.
editor/grida-canvas-react/viewport/ui/path-editor.tsx (2)
3-6: LGTM: Systematic hook migration to state-based variants.The replacement of direct hooks (
useEditorFlags,useTool,useTransform) with their state-based variants follows the systematic migration pattern and maintains the same interfaces while using the unifieduseCurrentEditorinternally.
15-17: LGTM: Hook usage correctly updated.The destructuring assignments have been properly updated to use the new state hooks. Based on the relevant code snippets, these hooks provide the same data structure with proper memoization for performance optimization.
editor/grida-canvas-react-starter-kit/starterkit-artboard-list/index.tsx (2)
9-9: LGTM: Clean migration to unified editor hook.The import change from
useDocumenttouseCurrentEditoraligns perfectly with the PR objective of consolidating editor state access.
18-21: LGTM: Proper method access through editor object.The refactor correctly updates the hook usage pattern - accessing
insertNodethrough the editor object instead of destructuring it directly. The node insertion logic remains unchanged and properly typed.editor/scaffolds/sidebar/sidebar-node-hierarchy-list.tsx (1)
4-4: LGTM: Consistent migration pattern implemented.The changes follow the same pattern as other files in this refactor:
- Import updated to
useCurrentEditor- Method access through the editor object (
editor.createScene())- Functionality preserved without any logic changes
This maintains consistency across the codebase migration.
Also applies to: 18-18, 27-27
editor/scaffolds/sidecontrol/controls/font-size.tsx (2)
7-8: LGTM: Clean import organization for type standardization.The type-only import of
editorand simplified import ofTMixedimproves import clarity and aligns with the API standardization effort.
16-16: LGTM: Type standardization improves API consistency.The change from
TChange<number>toeditor.api.NumberChangestandardizes the numeric change type across the editor API. This maintains the same functionality while improving type consistency throughout the codebase.editor/grida-canvas-react/use-editor.tsx (1)
5-5: LGTM: Improved modularity through hook extraction.The removal of
useRecorderand its related imports is part of a beneficial architectural change. Moving recording functionality to a dedicated plugin file (editor/grida-canvas-react/plugins/use-recorder.ts) improves:
- Separation of concerns: Core editor state management is now distinct from recording features
- Modularity: Recording functionality can be maintained independently
- Bundle optimization: Applications not using recording won't include this code
This aligns well with the PR objective of creating a cleaner, more modular editor architecture.
editor/scaffolds/sidecontrol/controls/letter-spacing.tsx (1)
1-2: LGTM! Clean type migration to the new editor API.The changes properly migrate from the local
TChangetype to the standardizededitor.api.NumberChangetype, aligning with the broader architectural refactor. The use of type-only imports is appropriate and the type compatibility should be maintained.Also applies to: 10-10
editor/scaffolds/sidecontrol/sidecontrol-global.tsx (1)
75-75: Excellent migration to the unified editor interface!The changes successfully migrate from the hook-based approach to the centralized
useCurrentEditorpattern:
useDocumentStatenow provides just the state datauseCurrentEditorprovides the editor instance with action methods- The method call
editor.changeNodeProps("page", k, v)properly replaces the previous hook-based approachThis aligns perfectly with the PR's goal of consolidating editor interactions into a unified interface.
Also applies to: 89-89, 158-158, 188-188
editor/scaffolds/sidecontrol/controls/opacity.tsx (1)
5-6: Consistent type migration - LGTM!The changes follow the same pattern as other control components, migrating from
TChange<number>toeditor.api.NumberChange. The type-only import usage is correct and maintains compatibility.Also applies to: 13-13
editor/scaffolds/sidecontrol/controls/rotate.tsx (1)
1-2: Perfect consistency with the type refactor!This file completes the consistent pattern seen across all control components, successfully migrating from
TChange<number>toeditor.api.NumberChange. The changes maintain functionality while adopting the standardized type system.Also applies to: 10-10
editor/scaffolds/sidecontrol/controls/stroke-width.tsx (1)
1-2: LGTM! Type refactoring aligns with centralization pattern.The change from local
TChange<number>toeditor.api.NumberChangeis correct and consistent with the broader refactoring to centralize types in the editor API. SinceNumberChangeis defined asTChange<number>in the editor module, this change maintains semantic equivalence while improving consistency.Also applies to: 10-10
editor/scaffolds/sidecontrol/controls/line-height.tsx (1)
2-3: LGTM! Consistent with type centralization pattern.The changes follow the same pattern as other control components, replacing local
TChange<number>with the centralizededitor.api.NumberChangetype. This improves consistency across the codebase.Also applies to: 11-11
editor/grida-canvas-react/renderer.tsx (1)
4-4: LGTM! Hook replacements align with editor API refactoring.The changes correctly replace the old hooks with their new equivalents:
useCurrentScene()→useCurrentSceneState()useTransform()→useTransformState()fit("*")→editor.fit("*")These changes are consistent with the broader refactoring to centralize editor functionality in the
Editorclass instance.Also applies to: 53-53, 141-141, 161-161, 168-168
editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/design/template-duo-001-viewer.tsx (1)
15-15: LGTM! Clean migration to unified editor API.The migration from
useDocumentanduseTransformhooks touseCurrentEditoris correctly implemented. The functionality remains the same - selecting and fitting nodes - but now uses the unified editor interface as intended by the architectural refactor.Also applies to: 278-278, 283-284
editor/grida-canvas/reducers/methods/transform.ts (1)
607-610: LGTM! Correctly implements the new generic node change action pattern.The migration from the specific
"node/change/rotation"action to the generic"node/change/*"action with simplified payload aligns with the broader architectural refactor. This change centralizes node property updates through a unified action interface while maintaining the same rotation functionality.editor/scaffolds/sidecontrol/sidecontrol-doctype-site.tsx (1)
14-14: LGTM! Consistent migration to unified editor interface.The changes correctly migrate from
useDocumenttouseCurrentEditorand update theschemaPutPropertycall to use the editor instance method. This maintains the same functionality while following the new unified editor API pattern.Also applies to: 68-68, 139-139
editor/app/(tools)/(playground)/playground/image/_page.tsx (1)
46-46: LGTM! Excellent API improvement with the unified editor interface.The migration to
useCurrentEditoris correctly implemented, and the newinsertNodemethod that returns the node ID is a significant improvement over the previous approach. This eliminates the need for manual ID management and makes the code cleaner and more intuitive.The flow is well-structured:
editor.insertNode()creates the node and returns its ID- The returned ID is used in the promise chain for
editor.changeNodeSrc()Also applies to: 127-127, 133-149
editor/grida-canvas-react/index.ts (1)
1-14: Excellent API consolidation supporting the editor refactor.The export restructuring effectively supports the migration to a unified editor instance pattern. The addition of
useCurrentEditorand granular state hooks (useDocumentState,useBrushState,useNodeActions, etc.) creates a more modular and consistent API surface. MovinguseRecorderto a dedicated plugin module also improves separation of concerns.editor/grida-canvas-react/devtools/api-host.tsx (1)
16-19: Clean component pattern for the devtools provider.The provider component follows React patterns correctly by using the hook and rendering null.
editor/grida-canvas-react/plugins/use-recorder.ts (1)
7-16: Excellent concurrent-safe implementation.The hook correctly uses
useSyncExternalStoreWithSelectorwith deep equality checking for optimal React 18+ compatibility and performance. The state synchronization pattern is well-implemented.editor/grida-canvas-react/viewport/ui/text-editor.tsx (3)
3-5: Perfect alignment with the unified editor architecture.The import updates correctly reflect the new architecture, replacing separate transform hooks with the unified
useCurrentEditorpattern and the granularuseTransformStatehook.
14-15: Clean integration of the new editor instance pattern.The unified editor instance and transform state access properly implements the architectural refactor, consolidating multiple hooks into a cohesive interface.
59-68: Excellent consolidation of editor actions.The event handlers now properly use the unified editor instance methods (
editor.tryExitContentEditMode,editor.changeNodeText) instead of separate hook-based actions, creating a more cohesive and maintainable API surface.editor/scaffolds/playground-canvas/playground.tsx (4)
32-35: LGTM! Import updates align with the new editor architecture.The replacement of
useToolanduseCurrentScenewith their "State" variants (useToolState,useCurrentSceneState) is consistent with the refactoring to centralize editor state management through theuseCurrentEditorhook.
126-126: LGTM! WindowCurrentEditorProvider addition supports development tooling.The addition of
WindowCurrentEditorProviderinside theStandaloneDocumentEditorcomponent tree enables external access to the current editor instance, which aligns with the scripting interface objectives mentioned in the PR.Also applies to: 274-274
510-511: LGTM! Hook usage updates maintain compatibility with new architecture.The replacement of
useTool()withuseToolState()anduseCurrentScene()withuseCurrentSceneState()maintains the same destructuring patterns and data access while routing through the centralized editor state management.Also applies to: 571-571
710-710: LGTM! Method calls updated to use centralized editor API.The changes from direct
insertNode()calls toeditor.insertNode()after obtaining the editor instance viauseCurrentEditor()correctly implements the new editor-centric API pattern. This centralizes document manipulation through the unified Editor class interface.Also applies to: 736-736, 753-753
editor/grida-canvas/reducers/surface.reducer.ts (2)
3-3: LGTM! Import statement consolidation improves code organization.Consolidating the imports into a single line is a good cleanup with no functional impact.
199-199: LGTM! Type parameter updates align with systematic type refinement.The change from
TChange<number>toeditor.api.NumberChangeis part of the broader type system refactoring to use more specific and centralized type definitions. The function logic remains unchanged, indicating the types have compatible structures.Also applies to: 218-218
packages/grida-canvas-schema/grida.ts (1)
873-873: LGTM! Generic type parameter adds flexibility while maintaining compatibility.Making
UnknownNodePropertiesgeneric with a default parameterT = unknownallows for more specific typing when needed while maintaining backward compatibility with existing usage.editor/grida-canvas/reducers/event-target.reducer.ts (2)
934-940: LGTM: Action type consolidation aligns with the broader refactor.The change from specific action type
"node/change/cornerRadius"to the generic"node/change/*"is consistent with the broader refactor to consolidate node change actions mentioned in the AI summary.
994-999: LGTM: Improved payload structure for flex gap updates.The changes consolidate the action type to the generic
"node/change/*"and improve the payload structure by setting bothmainAxisGapandcrossAxisGapinstead of a singlegapproperty. This provides more granular control over flex layout properties.editor/grida-canvas-react/viewport/ui/snap.tsx (1)
2-20: LGTM: Improved modularity with granular state hooks.The refactor successfully migrates from combined hooks (
useEventTarget,useTransform) to more granular state management withuseCurrentEditor,useGestureState,useTransformState, anduseEditorState. This centralization through the editor instance improves consistency and modularity across the codebase.The surface_snapping state access through
useEditorState(editor, (state) => state.surface_snapping)provides better type safety and encapsulation.editor/scaffolds/sidecontrol/ui/index.tsx (2)
17-17: LGTM: Type import standardization.The change from local
TChangetype toeditor.api.NumberChangestandardizes the change event types across the editor UI components, aligning with the broader API refactor.
187-187: LGTM: Consistent type annotations for NumberChange.The type annotations and casts are correctly updated to use
editor.api.NumberChangeinstead of the previousTChange<number>. This provides better type consistency across the editor API and maintains the same functionality for both "delta" and "set" change types.Also applies to: 225-235, 247-250
editor/grida-canvas-react/viewport/surface-hooks.ts (4)
2-2: LGTM: Updated imports for centralized state management.The import changes replace direct document and transform hooks with their state counterparts (
useDocumentState,useTransformState) and introduceuseCurrentEditorfor centralized editor access. This aligns with the broader refactor to improve state management consistency.Also applies to: 6-6
246-247: LGTM: Consistent state access pattern.The hook usage has been properly updated to use the new state hooks pattern:
useDocumentState()for document and document_ctx accessuseTransformState()for transform accessuseCurrentEditor()for editor instance accessThis centralizes state management through the editor instance as intended by the refactor.
Also applies to: 298-300
346-346: LGTM: Centralized rotation calculation.Using
instance.getNodeAbsoluteRotation(node_id)instead of direct document access centralizes the rotation calculation logic through the editor instance, improving encapsulation and consistency.
408-408: LGTM: Properly updated dependency array.The dependency array correctly reflects the new state access pattern, removing
document_ctxsince it's now accessed throughuseDocumentState()hook rather than being a separate dependency.editor/grida-canvas-react-starter-kit/starterkit-toolbar/index.tsx (3)
29-33: Migration to centralized editor API looks good.The import changes correctly migrate from individual provider hooks to the unified editor interface. This aligns well with the architectural refactoring objective.
55-57: Proper hook usage for editor state access.The migration to
useCurrentEditor(),useToolState(), anduseEditorFlagsState()follows the new centralized pattern correctly and maintains the same functionality.
82-87: Consistent editor method calls for tool updates.All tool state updates now properly use
editor.setTool()instead of direct setters, which centralizes the tool management through the editor instance. The conversion logic usingtoolbar_value_to_cursormodeis preserved correctly.Also applies to: 99-100, 118-119, 125-126
editor/grida-canvas-react/viewport/surface-context-menu.tsx (4)
11-12: Import updates align with editor centralization.The migration to
useCurrentSelection,useSelectionState, anduseCurrentEditorcorrectly consolidates the editor state access pattern.Also applies to: 16-16
22-25: Proper hook usage for centralized editor state.The editor instance and selection state are correctly accessed through the new centralized hooks, maintaining the same functionality while improving the architectural consistency.
53-53: Editor paste method integration.The fallback to
editor.paste()when clipboard API fails is correctly implemented, ensuring consistent paste behavior through the centralized editor interface.
84-84: Consistent editor method usage for document operations.All document manipulation operations (ordering, grouping, auto-layout, deletion) now properly use the centralized editor methods (
editor.order(),editor.contain(),editor.autoLayout(),editor.deleteNode()), maintaining functionality while improving architectural consistency.Also applies to: 94-94, 105-105, 115-115, 126-126
editor/scaffolds/sidecontrol/controls/ext-zoom.tsx (4)
14-14: Appropriate mixed import strategy for state hooks.The imports correctly use
useTransformStatefrom the provider for specific state slicing while usinguseCurrentEditoranduseEditorStatefor centralized editor access. This mixed approach is logical for the migration.Also applies to: 16-19
22-25: Correct editor state access pattern.The editor instance and UI state (ruler, pixelgrid) are properly accessed through the centralized hooks, with transform state appropriately sourced from the specific state hook.
44-44: Comprehensive zoom control integration.All zoom operations (
editor.scale(),editor.zoomIn(),editor.zoomOut(),editor.fit()) are correctly implemented using the centralized editor methods, maintaining the expected zoom functionality.Also applies to: 51-51, 59-59, 67-67, 75-75, 83-83, 90-90, 98-98
107-107: UI toggle methods properly integrated.The pixel grid and ruler toggle functionality correctly uses
editor.togglePixelGrid()andeditor.toggleRuler()methods, maintaining the UI control functionality through the centralized editor interface.Also applies to: 117-117
editor/grida-canvas-react/viewport/hooks/use-edge-scrolling.ts (4)
5-8: State hook imports correctly updated.The imports properly migrate to the new state hooks (
usePointerState,useToolState,useTransformState) and centralized editor access (useCurrentEditor,useEditorState), following the established migration pattern.Also applies to: 11-11
27-31: Proper editor state access for edge scrolling.The editor instance and state access are correctly implemented. The
draggingstate is properly accessed throughuseEditorState, and other state hooks follow the new pattern while maintaining the edge scrolling functionality.
62-62: Transform operation correctly centralized.The transform update properly uses
instance.transform(next)instead of a direct setter, centralizing the transform operation through the editor instance while maintaining the edge scrolling behavior.
68-68: Complete useEffect dependency array.The dependency array correctly includes all state variables (
dragging,pointer.client,transform,viewport,enabled,tool.type) that are used within the effect, ensuring proper reactivity and preventing stale closures.editor/grida-canvas-react-starter-kit/starterkit-toolbar/brush-toolbar.tsx (4)
5-5: LGTM! Clean migration to unified editor API.The refactoring correctly replaces the previous
useBrushanduseDocumenthooks with the unifieduseCurrentEditorapproach, maintaining the same functionality while improving consistency across the codebase.Also applies to: 49-50
77-77: Correctly migrated brush control methods.The brush size and opacity changes properly use the editor instance methods while maintaining the same
NumberChangeAPI pattern.Also applies to: 107-107
154-154: Brush selection properly migrated.The brush change handler correctly uses the editor instance method.
296-296: EyedropButton correctly uses editor instance.The clipboard color setter is properly migrated to use
editor.setClipboardColor.Also applies to: 304-304
editor/scaffolds/sidecontrol/sidecontrol-document-properties.tsx (3)
25-25: Import statements correctly updated.The imports properly migrate from individual hooks to the unified editor approach with
useCurrentEditor,useDocumentState, anduseCurrentSceneState.Also applies to: 34-34
37-45: Scene background control properly migrated.The component correctly uses
useCurrentSceneStatefor state access and routes the background color change througheditor.changeSceneBackgroundwith the proper scene_id parameter.
53-60: Document properties management correctly migrated.All schema operations (
schemaDefineProperty,schemaRenameProperty,schemaUpdateProperty,schemaDeleteProperty) are properly routed through the editor instance while maintaining their original signatures.Also applies to: 96-103
editor/grida-canvas-react/viewport/hotkeys.tsx (4)
2-2: Hook setup correctly migrated.The imports and initialization properly use
useCurrentEditorwhile maintaining specific hooks likeuseCurrentSelection,useToolState, anduseA11yActionsfor their specialized interfaces.Also applies to: 7-7, 320-324
328-340: Configuration methods properly migrated in blur handler.All surface configuration methods are correctly routed through the editor instance, maintaining the same reset behavior on window blur.
349-362: Modifier key handlers correctly migrated.The Meta/Control, Alt, and Shift key handlers properly use editor configuration methods for both keydown and keyup events, maintaining the same modifier behavior.
Also applies to: 378-389
411-915: All hotkey handlers consistently migrated.The extensive hotkey handler migrations are all correctly implemented, including:
- Tool switching (
setTool)- Selection operations (
select,deleteNode,duplicate)- History operations (
undo,redo)- Clipboard operations (
cut,copy)- Transform operations (
nudgeResize,order,align,distributeEvenly)- View operations (
scale,fit,zoomIn,zoomOut)- Content operations (
toggleBold,setOpacity,autoLayout,contain)All maintain their original behavior while using the unified editor API.
editor/grida-canvas-react-starter-kit/starterkit-hierarchy/index.tsx (3)
10-10: Import statements properly updated.The imports correctly migrate to use
useCurrentEditor,useEditorState, anduseCurrentSceneStatefor the unified editor approach.Also applies to: 38-38
51-51: Scene management operations correctly migrated.All scene operations are properly routed through the editor instance:
duplicateScene,deleteScenein context menuloadScenefor selectionrenameScenefor name changes- State access via
useEditorStatewith proper selectorsAlso applies to: 70-70, 79-79, 91-93, 111-111, 175-175
197-197: Node hierarchy operations properly migrated.All node operations correctly use editor methods:
- Context menu actions (copy, order, toggleNodeActive, fit, toggleNodeLocked, deleteNode)
- Selection and hover handling
- Move operations
- Name changes
Also applies to: 205-205, 224-224, 231-231, 240-240, 248-248, 256-256, 265-265, 295-295, 312-312, 362-362, 365-365, 386-386, 400-400, 412-412
editor/scaffolds/playground-canvas/toolbar.tsx (4)
68-69: LGTM! Clean migration to the unified editor API.The refactoring properly uses the centralized
useCurrentEditorhook and delegates text changes through the editor instance.Also applies to: 134-134, 142-142
149-150: Consistent tool management through the editor instance.The toolbar correctly uses the centralized editor for all tool state changes.
Also applies to: 164-168, 181-181, 200-200, 212-212
243-244: Proper mode management through the editor API.The auxiliary toolbar correctly delegates tool and mode changes to the editor instance.
Also applies to: 256-256, 273-273
309-310: Excellent use of the state selector pattern.The clipboard color management properly uses
useEditorStatewith a selector function and updates through the editor instance.Also applies to: 343-343
editor/grida-canvas/reducers/node.reducer.ts (4)
1-467: Excellent refactoring that improves maintainability and extensibility.The consolidation of node property updates into a generic handler with centralized property definitions is a significant improvement. This approach:
- Reduces code duplication
- Makes it easier to add new properties
- Ensures consistent validation and update logic
- Aligns well with the broader editor API refactoring
The architecture is well-thought-out and will make the codebase more maintainable.
🧰 Tools
🪛 Biome (1.9.4)
[error] 367-367: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
8-19: Well-designed property handler pattern.The
defineNodePropertyhelper provides a clean abstraction for defining property-specific assertions and application logic. The type safety with generics ensures compile-time correctness.
384-393: Excellent refactor to generic property handling.The new
"node/change/*"case elegantly handles all property updates through a single mechanism. The destructuring pattern cleanly separates metadata from actual property updates.
24-360:Details
❓ Verification inconclusive
Verify complete property coverage.
The
safe_propertiesmap is comprehensive, but please verify that all properties previously handled by individual cases are now covered.Run this script to check for any properties that might be missing handlers:
🏁 Script executed:
#!/bin/bash # Description: Check for node properties that might not have handlers defined # Search for all property keys used in the UnknownNode type ast-grep --pattern 'type UnknwonNode = $_' | head -100 # Search for any node property assignments in the codebase that might not be in safe_properties rg -A 2 'node\.' --type ts | grep -E '^\s*(node|draft)\.[a-zA-Z]+\s*=' | grep -v -E '(type|id|children|_type|_id)' | sort | uniqLength of output: 1630
Verify all node schema properties have handlers
Our initial search didn’t reveal any unhandled
draft.<prop>assignments, but to ensure full coverage please cross-reference:
- editor/grida-canvas/reducers/node.reducer.ts – the keys in the
safe_propertiesmap- packages/grida-canvas-schema/grida.ts – all fields listed under
export type UnknwonNode = …In particular, confirm you have handlers for:
- VideoNode props (autoplay, loop, muted, playbackRate)
- HTMLIFrameNode attributes (allowFullScreen, sandbox, etc.)
- Effect settings (blur, innerShadow, layerBlur, clipContent)
- TextNode formatting (fontFamily, textDecoration, rich-text span attributes)
- Any new Component/Instance/TemplateInstance-specific properties
If any schema fields are missing, add corresponding
defineNodePropertyentries.editor/scaffolds/sidecontrol/sidecontrol-node-selection.tsx (2)
48-73: Well-executed migration to the new editor hooks architecture.The migration from general hooks like
useDocumentanduseSelectionto more specific hooks likeuseCurrentEditor,useEditorState, anduseNodeStateis a good architectural improvement that:
- Provides more granular state access
- Likely improves performance by reducing unnecessary re-renders
- Aligns with the unified editor API pattern
Also applies to: 93-93
1195-1284: Excellent modularization of position and dimension controls.The extraction of
SectionPositionandSectionDimensioncomponents is a great refactoring that:
- Improves code organization and readability
- Makes the components more reusable
- Follows the single responsibility principle
- Uses the new hooks pattern consistently
editor/grida-canvas/action.ts (2)
594-606: Clean and flexible action type design.The refactored
NodeChangeActiontype is well-designed:
- The generic
"node/change/*"variant provides flexibility for simple property updates- Specific action types are retained for complex operations that require special handling
- Type safety is maintained through proper use of
PartialandOmitThis aligns perfectly with the reducer implementation.
504-512: Good type refinement for brush actions.The change from
TChange<number>toeditor.api.NumberChangefor brush size and opacity actions improves type safety and maintains consistency with other numeric property changes throughout the codebase.editor/grida-canvas/index.ts (7)
1722-1736: Good addition of verbose error class.The
EditorConsumerVerboseErrorclass provides excellent debugging capabilities by attaching context data to errors. The implementation follows best practices with proper error name setting and stack trace capture.
1738-1751: Well-designed change type system.The
TChange<T>pattern elegantly handles both absolute ("set") and relative ("delta") value changes. TheNonNullable<T>constraint on delta values is a smart type safety measure.
1895-1914: Clean interface design for editor features.The new interfaces (
IBrushToolActions,IPixelGridActions,IRulerActions,IGuide2DActions) follow SOLID principles well with focused responsibilities and clear method signatures.
2023-2024: Excellent improvement to insertNode method.Returning the
NodeIDfrominsertNodeis a significant improvement that enables better programmatic control over newly created nodes.
1738-1751: Well-designed change types!The
TChange<T>generic type andNumberChangespecialization provide a clean abstraction for representing both absolute and relative changes. The use ofNonNullable<T>for delta values ensures type safety.
1895-1943: Excellent modular interface design!The new action interfaces (
IBrushToolActions,IPixelGridActions,IRulerActions,IGuide2DActions,ICameraActions) are well-structured with clear separation of concerns. The documentation on camera methods is particularly helpful.
2023-2023: Good API improvement:insertNodenow returns the created node ID.The change to have
insertNodereturn aNodeIDis a significant improvement that enables better programmatic control and chaining of operations.editor/grida-canvas/editor.ts (2)
15-49: Well-implemented utility functions.Both
throttleandresolveNumberChangeValueare properly implemented with good error handling. The number change resolver correctly validates node properties before applying delta changes.
899-918: Verify undefined handling for optional numeric properties.The
changeTextNodeLineHeightmethod correctly handles the case, but ensure this pattern is consistent across all methods dealing with optional numeric properties.editor/grida-canvas-react/viewport/surface.tsx (5)
5-19: LGTM! Clean refactor to centralized state hooks.The migration from individual hooks to unified state hooks through
useCurrentEditor()improves consistency and reduces the number of separate contexts. This aligns well with the Editor API V2 migration objective.
173-173: LGTM! Consistent with the editor instance pattern.The pointer move handler correctly delegates to the editor instance.
194-231: LGTM! Event handlers properly migrated to editor methods.All mouse and pointer event handlers consistently use the centralized editor instance methods, maintaining the same behavior while improving the architecture.
394-408: Well-implemented marquee overlay component.The
MarqueeOverlaycomponent correctly:
- Uses the editor state to access marquee data
- Transforms coordinates from canvas to surface space
- Returns null when no marquee is active
497-515: LGTM! Node interaction handlers properly refactored.The
NodeTitleBarcomponent correctly uses editor methods for all interactions including hover, rename, and selection operations.editor/grida-canvas/reducers/document.reducer.ts (2)
794-814: Good consolidation of node change actions.The refactor to handle all node changes through a single
"node/change/*"case improves maintainability by centralizing the logic. The fontFamily case is correctly kept separate due to its side effect of updating googlefonts.
947-950: Improved hierarchy graph update approach.Creating a new
editor.dq.Contextfrom the draft document and assigning its snapshot is a more reliable approach than manually mutating the children ID mapping. This ensures the document context accurately reflects the reordered state.
| return React.useMemo( | ||
| () => ({ | ||
| status: state.status, | ||
| nframes: state.nframes, | ||
| start: () => recorder.start(), | ||
| stop: () => recorder.stop(), | ||
| clear: () => recorder.clear(), | ||
| replay: () => recorder.play(), | ||
| exit: () => recorder.exit(), | ||
| dumps: () => recorder.dumps(), | ||
| }), | ||
| [state, recorder] | ||
| ); |
There was a problem hiding this comment.
Fix method naming inconsistency.
The memoized object structure is good, but there's a naming inconsistency between the exposed method name and the actual call.
Apply this diff to fix the method naming:
- replay: () => recorder.play(),
+ replay: () => recorder.replay(),Or if play() is the correct method to call, rename the exposed method:
- replay: () => recorder.play(),
+ play: () => recorder.play(),Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In editor/grida-canvas-react/plugins/use-recorder.ts around lines 18 to 30, the
exposed method name 'replay' does not match the actual method called 'play' on
the recorder object. To fix this, either rename the exposed method from 'replay'
to 'play' to match the recorder method, or if you want to keep the exposed
method as 'replay', change the call to recorder.replay() if that method exists.
Ensure the method names are consistent between the exposed API and the
underlying calls.
| fill: defineNodeProperty<"fill">({ | ||
| assert: (node) => | ||
| node.type === "vector" || | ||
| node.type === "path" || | ||
| node.type === "image" || | ||
| node.type === "rectangle" || | ||
| node.type === "ellipse" || | ||
| node.type === "text" || | ||
| node.type === "richtext" || | ||
| node.type === "container" || | ||
| node.type === "component", | ||
| apply: (draft, value, prev) => { | ||
| if (value === null) { | ||
| draft.fill = undefined; | ||
| return; | ||
| } | ||
|
|
||
| switch (value.type) { | ||
| case "linear_gradient": | ||
| case "radial_gradient": | ||
| draft.fill = { | ||
| ...(value as cg.LinearGradientPaint | cg.RadialGradientPaint), | ||
| id: `gradient-${v4()}`, | ||
| }; | ||
| break; | ||
| case "solid": | ||
| draft.fill = value as | ||
| | grida.program.nodes.i.props.SolidPaintToken | ||
| | cg.SolidPaint; | ||
| break; | ||
| } | ||
| }, | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent null handling across paint and text properties.
The codebase handles null values inconsistently:
fillandstrokeproperties convertnulltoundefined(lines 122, 192)textproperty preservesnullusingvalue ?? null(line 346)
This inconsistency could lead to unexpected behavior. Consider standardizing the approach.
Standardize null handling across all properties. Either consistently convert null to undefined or preserve null values:
text: defineNodeProperty<"text">({
assert: (node) => node.type === "text",
apply: (draft, value, prev) => {
- draft.text = value ?? null;
+ draft.text = value === null ? undefined : value;
},
}),Also applies to: 184-211, 343-348
🤖 Prompt for AI Agents
In editor/grida-canvas/reducers/node.reducer.ts between lines 109-141, 184-211,
and 343-348, the handling of null values for fill, stroke, and text properties
is inconsistent; fill and stroke convert null to undefined, while text preserves
null. To fix this, choose a consistent approach for null handling across all
these properties—either convert all null values to undefined or preserve null
values as is—and update the code accordingly to apply this standard uniformly.
| switch (value.type) { | ||
| case "linear_gradient": | ||
| case "radial_gradient": | ||
| draft.fill = { | ||
| ...(value as cg.LinearGradientPaint | cg.RadialGradientPaint), | ||
| id: `gradient-${v4()}`, | ||
| }; | ||
| break; | ||
| case "solid": | ||
| draft.fill = value as | ||
| | grida.program.nodes.i.props.SolidPaintToken | ||
| | cg.SolidPaint; | ||
| break; | ||
| } | ||
| }, |
There was a problem hiding this comment.
Potential issue with gradient ID generation.
The gradient handlers generate new IDs using v4() every time a gradient is applied. This could lead to:
- Unnecessary ID churn when the same gradient is reapplied
- Potential performance impact from frequent UUID generation
- Loss of gradient identity tracking
Consider reusing existing gradient IDs when appropriate or generating IDs at a higher level.
case "linear_gradient":
case "radial_gradient":
draft.fill = {
...(value as cg.LinearGradientPaint | cg.RadialGradientPaint),
- id: `gradient-${v4()}`,
+ id: value.id || `gradient-${v4()}`,
};
break;Also applies to: 196-210
🤖 Prompt for AI Agents
In editor/grida-canvas/reducers/node.reducer.ts around lines 126 to 140, the
current code generates a new UUID for gradient IDs every time a gradient is
applied, causing unnecessary ID changes and potential performance issues. To fix
this, modify the logic to check if the gradient already has an ID and reuse it
instead of generating a new one with v4(). Only generate a new ID if none
exists. Apply the same fix to lines 196 to 210 where similar gradient ID
generation occurs.
| case "node/change/*": { | ||
| const { type: _, node_id: __, ...values } = action; | ||
| for (const [key, value] of Object.entries(values)) { | ||
| applyNodeProperty( | ||
| draft as grida.program.nodes.UnknownNodeProperties, | ||
| key as keyof grida.program.nodes.UnknwonNode, | ||
| value | ||
| ); | ||
| } | ||
| break; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety in the generic node change handler.
The generic handler looks good, but the type casting on line 389 could be safer. Consider validating that the key is actually a valid node property before casting.
case "node/change/*": {
const { type: _, node_id: __, ...values } = action;
for (const [key, value] of Object.entries(values)) {
+ // Validate key is a known property
+ if (!(key in safe_properties)) {
+ console.warn(`Skipping unknown property: "${key}"`);
+ continue;
+ }
applyNodeProperty(
draft as grida.program.nodes.UnknownNodeProperties,
key as keyof grida.program.nodes.UnknwonNode,
value
);
}
break;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In editor/grida-canvas/reducers/node.reducer.ts around lines 384 to 394, the
code casts keys from the action values to node property keys without validation,
which risks unsafe type assertions. To fix this, add a runtime check to confirm
each key is a valid property of the node before casting and applying it. This
can be done by verifying the key against a known set of valid node property keys
or using a type guard function to ensure type safety before calling
applyNodeProperty.
| function defineNodeProperty< | ||
| K extends keyof grida.program.nodes.UnknwonNode, | ||
| >(handlers: { | ||
| assert?: (node: grida.program.nodes.UnknwonNode) => boolean; | ||
| apply: ( | ||
| draft: grida.program.nodes.UnknownNodeProperties, | ||
| value: NonNullable<grida.program.nodes.UnknwonNode[K]>, | ||
| prev?: grida.program.nodes.UnknwonNode[K] | ||
| ) => void; | ||
| }) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Address the type name inconsistency between UnknwonNode and UnknownNodeProperties.
While UnknwonNode appears to be an existing typo in the codebase, the inconsistency between line 9 using UnknwonNode and line 13 using UnknownNodeProperties makes the code harder to maintain. Consider using consistent naming throughout.
🏁 Script executed:
#!/bin/bash
# Description: Check the usage of UnknwonNode vs UnknownNode in the codebase
# Search for UnknwonNode usage
echo "=== UnknwonNode usage ==="
rg "UnknwonNode" -A 2 -B 2
echo -e "\n=== UnknownNode usage ==="
# Search for UnknownNode usage (without the typo)
rg "UnknownNode(?!Properties)" -A 2 -B 2Length of output: 11634
Rename UnknwonNode to UnknownNode for consistent naming
The core node type is misspelled as UnknwonNode in the schema but its properties helper uses the correctly spelled UnknownNodeProperties. This one-letter typo propagates confusion and makes maintenance harder. Please:
- In
packages/grida-canvas-schema/grida.ts, rename
export type UnknwonNode = …
to
export type UnknownNode = … - Update all references—including in
editor/grida-canvas/reducers/node.reducer.tseditor/grida-canvas-react/provider.tsxeditor/grida-canvas/action.tseditor/grida-canvas/editor.tseditor/grida-canvas/viewport/surface-hooks.ts- and any other files importing
grida.program.nodes.UnknwonNode—to useUnknownNodeinstead.
- Ensure the generic helper signatures align:
defineNodeProperty<K extends keyof grida.program.nodes.UnknownNode>(…)
withUnknownNodePropertiesas before.
This consistent renaming will eliminate the typo and align your type definitions.
🤖 Prompt for AI Agents
In editor/grida-canvas/reducers/node.reducer.ts around lines 8 to 17, the type
name UnknwonNode is misspelled and inconsistent with UnknownNodeProperties.
Rename all occurrences of UnknwonNode to UnknownNode in this file and across the
codebase, including the type definition in packages/grida-canvas-schema/grida.ts
and all imports in related files. Ensure the generic type parameter and function
signatures use UnknownNode consistently to fix the typo and maintain uniform
naming.
| function applyNodeProperty<K extends keyof grida.program.nodes.UnknwonNode>( | ||
| draft: grida.program.nodes.UnknownNodeProperties, | ||
| key: K, | ||
| value: any | ||
| ) { | ||
| "use strict"; | ||
| if (!(key in safe_properties)) { | ||
| throw new Error(`property handler not found: "${key}"`); | ||
| } | ||
| const property = safe_properties[key as keyof typeof safe_properties]; | ||
| if (property) { | ||
| const prev = (draft as any)[key]; | ||
| // TODO: assert - decide pre or after | ||
| property.apply(draft, value, prev); | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove redundant "use strict" directive and improve type safety.
The function has several issues:
- The "use strict" directive on line 367 is redundant in ES modules
- Using
anytype for the value parameter loses type safety - TODO comment about assertion timing should be addressed
function applyNodeProperty<K extends keyof grida.program.nodes.UnknwonNode>(
draft: grida.program.nodes.UnknownNodeProperties,
key: K,
- value: any
+ value: grida.program.nodes.UnknwonNode[K]
) {
- "use strict";
if (!(key in safe_properties)) {
throw new Error(`property handler not found: "${key}"`);
}
const property = safe_properties[key as keyof typeof safe_properties];
if (property) {
const prev = (draft as any)[key];
- // TODO: assert - decide pre or after
+ // Assert before applying to fail fast
+ if (property.assert && !property.assert(draft as grida.program.nodes.UnknwonNode)) {
+ throw new Error(`Invalid node type for property "${key}"`);
+ }
property.apply(draft, value, prev);
}
}📝 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.
| function applyNodeProperty<K extends keyof grida.program.nodes.UnknwonNode>( | |
| draft: grida.program.nodes.UnknownNodeProperties, | |
| key: K, | |
| value: any | |
| ) { | |
| "use strict"; | |
| if (!(key in safe_properties)) { | |
| throw new Error(`property handler not found: "${key}"`); | |
| } | |
| const property = safe_properties[key as keyof typeof safe_properties]; | |
| if (property) { | |
| const prev = (draft as any)[key]; | |
| // TODO: assert - decide pre or after | |
| property.apply(draft, value, prev); | |
| } | |
| } | |
| function applyNodeProperty<K extends keyof grida.program.nodes.UnknwonNode>( | |
| draft: grida.program.nodes.UnknownNodeProperties, | |
| key: K, | |
| value: grida.program.nodes.UnknwonNode[K] | |
| ) { | |
| if (!(key in safe_properties)) { | |
| throw new Error(`property handler not found: "${key}"`); | |
| } | |
| const property = safe_properties[key as keyof typeof safe_properties]; | |
| if (property) { | |
| const prev = (draft as any)[key]; | |
| // Assert before applying to fail fast | |
| if (property.assert && !property.assert(draft as grida.program.nodes.UnknwonNode)) { | |
| throw new Error(`Invalid node type for property "${key}"`); | |
| } | |
| property.apply(draft, value, prev); | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 367-367: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🤖 Prompt for AI Agents
In editor/grida-canvas/reducers/node.reducer.ts lines 362 to 377, remove the
redundant "use strict" directive as ES modules are strict by default. Replace
the `any` type of the `value` parameter with a more specific type that
corresponds to the property type for key K to improve type safety. Address the
TODO comment by deciding whether to assert before or after applying the property
and implement the chosen assertion accordingly.
Remove redundant 'use strict' directive.
The implementation is solid, but the 'use strict' directive is redundant in module scope.
Apply this diff to fix the issue:
) {
- "use strict";
if (!(key in safe_properties)) {📝 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.
| function applyNodeProperty<K extends keyof grida.program.nodes.UnknwonNode>( | |
| draft: grida.program.nodes.UnknownNodeProperties, | |
| key: K, | |
| value: any | |
| ) { | |
| "use strict"; | |
| if (!(key in safe_properties)) { | |
| throw new Error(`property handler not found: "${key}"`); | |
| } | |
| const property = safe_properties[key as keyof typeof safe_properties]; | |
| if (property) { | |
| const prev = (draft as any)[key]; | |
| // TODO: assert - decide pre or after | |
| property.apply(draft, value, prev); | |
| } | |
| } | |
| function applyNodeProperty<K extends keyof grida.program.nodes.UnknwonNode>( | |
| draft: grida.program.nodes.UnknownNodeProperties, | |
| key: K, | |
| value: any | |
| ) { | |
| if (!(key in safe_properties)) { | |
| throw new Error(`property handler not found: "${key}"`); | |
| } | |
| const property = safe_properties[key as keyof typeof safe_properties]; | |
| if (property) { | |
| const prev = (draft as any)[key]; | |
| // TODO: assert - decide pre or after | |
| property.apply(draft, value, prev); | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 367-367: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🤖 Prompt for AI Agents
In editor/grida-canvas/reducers/node.reducer.ts around lines 362 to 377, remove
the redundant 'use strict' directive inside the applyNodeProperty function since
modules are strict by default. Simply delete the line containing "use strict";
without altering any other code.
| export class NodeProxy<T extends grida.program.nodes.Node> { | ||
| constructor( | ||
| private editor: Editor, | ||
| private node_id: string | ||
| ) { | ||
| // @ts-expect-error - this is a workaround to allow the proxy to be used as a node | ||
| return new Proxy(this, { | ||
| get: (target, prop: string) => { | ||
| return (target.editor.getNodeSnapshotById(target.node_id) as T)[ | ||
| prop as keyof T | ||
| ]; | ||
| }, | ||
| set: (target, prop: string, value) => { | ||
| try { | ||
| target.editor.dispatch({ | ||
| type: "node/change/*", | ||
| node_id: target.node_id, | ||
| [prop]: value, | ||
| }); | ||
| return true; | ||
| } catch (e) { | ||
| return false; // unknown prop | ||
| } | ||
| }, | ||
| }) as T; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor NodeProxy to avoid returning from constructor.
While the Proxy pattern works, returning from a constructor is non-standard and flagged by static analysis. Consider using a factory method instead for cleaner code.
Apply this refactor for a cleaner implementation:
-export class NodeProxy<T extends grida.program.nodes.Node> {
- constructor(
- private editor: Editor,
- private node_id: string
- ) {
- // @ts-expect-error - this is a workaround to allow the proxy to be used as a node
- return new Proxy(this, {
+export class NodeProxy<T extends grida.program.nodes.Node> {
+ private constructor(
+ private editor: Editor,
+ private node_id: string
+ ) {}
+
+ static create<T extends grida.program.nodes.Node>(
+ editor: Editor,
+ node_id: string
+ ): T {
+ const instance = new NodeProxy<T>(editor, node_id);
+ return new Proxy(instance, {
get: (target, prop: string) => {
return (target.editor.getNodeSnapshotById(target.node_id) as T)[
prop as keyof T
];
},
set: (target, prop: string, value) => {
try {
target.editor.dispatch({
type: "node/change/*",
node_id: target.node_id,
[prop]: value,
});
return true;
} catch (e) {
return false; // unknown prop
}
},
}) as T;
}
}Then update the getNodeById method:
- public getNodeById(
- node_id: editor.NodeID
- ): NodeProxy<grida.program.nodes.Node> {
- return new NodeProxy(this, node_id);
- }
+ public getNodeById(
+ node_id: editor.NodeID
+ ): grida.program.nodes.Node {
+ return NodeProxy.create(this, node_id);
+ }Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1551-1569: The constructor should not return a value.
The constructor is here:
Returning a value from a constructor is ignored.
(lint/correctness/noConstructorReturn)
🤖 Prompt for AI Agents
In editor/grida-canvas/editor.ts around lines 1545 to 1571, the NodeProxy class
currently returns a Proxy instance directly from its constructor, which is
non-standard and flagged by static analysis. Refactor by removing the Proxy
return from the constructor and instead create a static factory method (e.g.,
NodeProxy.create) that returns the Proxy instance. Then update the getNodeById
method to use this factory method to obtain the proxied node. This approach
maintains the Proxy functionality while adhering to standard class construction
patterns.
1 participant
Migrating from react -> Pure js Editor class.
day-238-grida-canvas-scripting-interface-v2.mp4
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores