Skip to content

fix: Figma comment and deep link for instance-internal nodes#230

Merged
let-sunny merged 2 commits intomainfrom
fix/figma-comment-deeplink
Mar 31, 2026
Merged

fix: Figma comment and deep link for instance-internal nodes#230
let-sunny merged 2 commits intomainfrom
fix/figma-comment-deeplink

Conversation

@let-sunny
Copy link
Copy Markdown
Owner

@let-sunny let-sunny commented Mar 31, 2026

Summary

  • Comment API 400 fix: Instance-internal node IDs like I3010:7457;1442:7704 are stripped to 3010:7457 before posting — Figma Comments API only accepts simple node IDs
  • Deep link fix: Strip instance path to top-level node and convert colons to hyphens (I175:7425;1442:7704node-id=175-7425)
  • UI cleanup: Remove lock icon from authorize modal, remove checkmark from Sent button

Test plan

  • pnpm test:run — 656 tests pass
  • Deep link navigates to correct node in Figma
  • Comment posts successfully on instance-internal nodes

🤖 Generated with Claude Code

- Comment API: strip instance path (semicolons) and "I" prefix from
  node IDs before posting — API only accepts simple "3010:7457" format
- Deep link: convert colons to hyphens in node IDs for valid Figma URLs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 218ff0e3-0020-44e1-b7d0-ff036874e568

📥 Commits

Reviewing files that changed from the base of the PR and between 992f149 and 61c6e59.

📒 Files selected for processing (3)
  • app/web/src/index.html
  • src/browser.ts
  • src/core/report-html/index.ts

📝 Walkthrough

Walkthrough

A new utility normalizes Figma node IDs and is applied to comment payloads and deep-link construction. Node IDs are truncated at the first semicolon, a leading "I" is removed, and colon/dash format conversions are applied where appropriate.

Changes

Cohort / File(s) Summary
Figma Node ID Utility
src/core/adapters/figma-url-parser.ts
Added exported toCommentableNodeId(nodeId: string) which takes the first semicolon-delimited segment and strips a leading I. buildFigmaDeepLink() now replaces : with - before encoding.
Exports
src/browser.ts
Re-exported toCommentableNodeId alongside existing Figma URL helpers.
Comment Posting Scripts
app/web/src/index.html, src/core/report-html/index.ts
Updated embedded postComment logic to convert dashes→colons, then pass the first semicolon segment (with leading I removed) through toCommentableNodeId and use that value for client_meta.node_id in the Figma comments POST payload.

Sequence Diagram(s)

sequenceDiagram
    participant UI as Client UI (button)
    participant Script as Embedded postComment script
    participant Util as toCommentableNodeId
    participant API as Figma Comments API
    UI->>Script: click (dataset.nodeId with dashes)
    Script->>Script: replace '-' -> ':' (intermediate nodeId)
    Script->>Util: toCommentableNodeId(intermediate nodeId)
    Util-->>Script: commentNodeId (first segment, no leading "I")
    Script->>API: POST /v1/files/{fileKey}/comments (client_meta.node_id = commentNodeId)
    API-->>Script: 200/response
    Script-->>UI: update UI (success/failure)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 I nibble dashes, chew a colon bite,
Semicolons split by morning light.
I pluck the leading "I" away,
And hop a deep-link down the way.
Comments land neat — a rabbit's delight! ✨

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: fixing both Figma comment posting and deep linking for instance-internal nodes, which are the primary focuses of this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/figma-comment-deeplink

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

@let-sunny let-sunny marked this pull request as ready for review March 31, 2026 13:07
Copy link
Copy Markdown
Contributor

@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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/core/adapters/figma-url-parser.ts`:
- Around line 48-57: The helper toCommentableNodeId is implemented but not
exposed to consumers — either export it through the public browser API or remove
it to avoid duplicate logic in HTML scripts; specifically, add an export
forwarding to src/browser.ts (e.g., expose toCommentableNodeId on the CanICode
public object so callers like the inline scripts in
src/core/report-html/index.ts and app/web/src/index.html can call
CanICode.toCommentableNodeId()), or if you want it private, delete the export
from src/core/adapters/figma-url-parser.ts and update the inline scripts to keep
their local logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 944bcac6-929c-4151-90da-7a7ea78fd0b2

📥 Commits

Reviewing files that changed from the base of the PR and between 7ffa433 and 992f149.

📒 Files selected for processing (3)
  • app/web/src/index.html
  • src/core/adapters/figma-url-parser.ts
  • src/core/report-html/index.ts

…uplication

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@let-sunny let-sunny merged commit d82bebf into main Mar 31, 2026
3 checks passed
@let-sunny let-sunny deleted the fix/figma-comment-deeplink branch March 31, 2026 13:16
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.

1 participant