Skip to content

test: cover viewport-aware paste positioning#699

Open
k08200 wants to merge 1 commit intogridaco:mainfrom
k08200:test/368-paste-positioning-regression
Open

test: cover viewport-aware paste positioning#699
k08200 wants to merge 1 commit intogridaco:mainfrom
k08200:test/368-paste-positioning-regression

Conversation

@k08200
Copy link
Copy Markdown
Contributor

@k08200 k08200 commented Apr 30, 2026

Summary

  • add regression coverage for viewport-aware paste placement at scene level
  • add coverage for container-target paste preserving viewport-aware absolute placement after parent adjustment

Verification

  • pnpm --dir editor exec vitest run grida-canvas/reducers/tests/paste-position.test.ts
  • pnpm exec oxfmt --check editor/grida-canvas/reducers/tests/paste-position.test.ts
  • cargo fmt --all --check

Closes #368

Summary by CodeRabbit

  • Tests
    • Enhanced test coverage for paste positioning functionality, including verification of node placement and layout positioning when pasting into different document targets.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments
Project Deployment Actions Updated (UTC)
backgrounds Skipped Skipped Apr 30, 2026 8:18am
blog Skipped Skipped Apr 30, 2026 8:18am
viewer Skipped Skipped Apr 30, 2026 8:18am

Request Review

@vercel vercel Bot temporarily deployed to Preview – blog April 30, 2026 08:17 Inactive
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 30, 2026

@k08200 is attempting to deploy a commit to the Grida Team on Vercel.

A member of the Team first needs to authorize it.

@vercel vercel Bot temporarily deployed to Preview – viewer April 30, 2026 08:17 Inactive
@vercel vercel Bot temporarily deployed to Preview – backgrounds April 30, 2026 08:17 Inactive
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

Walkthrough

This PR introduces a dedicated test suite for the document reducer's paste positioning functionality. The tests verify that pasted items are correctly positioned at viewport-centered coordinates and linked appropriately when pasting to both scene and container targets.

Changes

Cohort / File(s) Summary
Paste Position Tests
editor/grida-canvas/reducers/__tests__/paste-position.test.ts
New test suite with two test cases verifying paste positioning logic: one for pasting onto scene target with viewport-centered coordinates, and another for pasting onto container target with coordinates adjusted by the container's layout inset.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding test coverage for viewport-aware paste positioning, which directly corresponds to the PR's primary objective of regression testing.
Linked Issues check ✅ Passed The PR adds test coverage for viewport-aware paste positioning as specified in issue #368, verifying both scene-level and container-target paste behavior with viewport centering logic.
Out of Scope Changes check ✅ Passed The PR contains only a new test file with no modifications to production code, and all tests directly validate the viewport-aware paste positioning requirements from issue #368.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
editor/grida-canvas/reducers/__tests__/paste-position.test.ts (1)

74-82: Type the paste helper against the paste action contract (avoid as DocumentAction).

The target parameter is typed as string, but EditorPasteAction requires target: NodeID | NodeID[]. The as DocumentAction cast bypasses this validation and hides potential action-shape regressions.

Proposed change
+type PasteAction = Extract<DocumentAction, { type: "paste" }>;
+
 function paste(
   state: editor.state.IEditorState,
-  target: string
+  target: PasteAction["target"]
 ): editor.state.IEditorState {
   let id = 0;
+  const action: PasteAction = { type: "paste", target };
   return documentReducer(
     state,
-    { type: "paste", target } as DocumentAction,
+    action,
     createReducerContext({
       viewport: VIEWPORT,
       idgen: {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@editor/grida-canvas/reducers/__tests__/paste-position.test.ts` around lines
74 - 82, The paste helper is currently typed with target: string and casts the
action to DocumentAction, bypassing the EditorPasteAction shape; update the
paste function signature to accept target: NodeID | NodeID[] (the same type
required by EditorPasteAction) and construct the action using that precise type
(e.g., create an object matching EditorPasteAction or cast to EditorPasteAction
instead of DocumentAction) before passing it to documentReducer; ensure
imports/reference to EditorPasteAction and NodeID are used so the compiler
enforces the correct action shape and remove the unsafe `as DocumentAction`
cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@editor/grida-canvas/reducers/__tests__/paste-position.test.ts`:
- Around line 74-82: The paste helper is currently typed with target: string and
casts the action to DocumentAction, bypassing the EditorPasteAction shape;
update the paste function signature to accept target: NodeID | NodeID[] (the
same type required by EditorPasteAction) and construct the action using that
precise type (e.g., create an object matching EditorPasteAction or cast to
EditorPasteAction instead of DocumentAction) before passing it to
documentReducer; ensure imports/reference to EditorPasteAction and NodeID are
used so the compiler enforces the correct action shape and remove the unsafe `as
DocumentAction` cast.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f2c3dee2-5923-4a19-98c8-7a0771319b82

📥 Commits

Reviewing files that changed from the base of the PR and between 5e63162 and ec09798.

📒 Files selected for processing (1)
  • editor/grida-canvas/reducers/__tests__/paste-position.test.ts

Copy link
Copy Markdown
Contributor Author

k08200 commented Apr 30, 2026

Behavior verification update: I also exercised the actual editor API flow (doc.copydoc.blursurface.a11yPaste()) with no active selection. The pasted offscreen rectangle was inserted into the scene, moved to viewport-centered coordinates (450, 450) in a 1000x1000 viewport, and selected after paste.

@k08200
Copy link
Copy Markdown
Contributor Author

k08200 commented Apr 30, 2026

default.mov

is it right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Grida Canvas] Better Copy & Paste positioning

1 participant