Skip to content

fix: simplify Figma comment to enriched message only#99

Merged
let-sunny merged 5 commits intomainfrom
fix/figma-comment-improve
Mar 26, 2026
Merged

fix: simplify Figma comment to enriched message only#99
let-sunny merged 5 commits intomainfrom
fix/figma-comment-improve

Conversation

@let-sunny
Copy link
Copy Markdown
Owner

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

Summary

  • Figma 코멘트 본문을 고도화된 violation.message만 보내도록 간소화
  • 중복된 why/impact/fix 정적 텍스트 제거 (PR #96에서 message에 이미 수정 방향 포함)
  • 에러 핸들링 개선: 403/404/429 구분 + hover시 상세 에러 표시
  • 웹앱 + CLI HTML 리포트 양쪽 동일하게 수정

Before:

[CanICode] No Auto Layout

Fix: Apply Auto Layout to create clear...
Why: Without Auto Layout, AI must guess...
Impact: Generated code uses hardcoded...

Frame "Card" has no auto-layout...
Node: Page > Card

After:

[CanICode] No Auto Layout
Frame "Card" has no auto-layout (3 children arranged vertically) — apply VERTICAL auto-layout
Node: Page > Card

Closes #98

Test plan

  • pnpm lint pass
  • pnpm test:run — 592 tests pass
  • 웹앱에서 코멘트 버튼 동작 확인 (Figma 토큰 필요)

https://claude.ai/code/session_017n9jQMQWFoEE3Mje7MfgXs

Summary by CodeRabbit

  • Bug Fixes

    • Improved Figma comment submission error handling with HTTP status–specific messages (400/403/404/429 and default). Failed submissions now show "Failed" (no ✗) and display the error text as a tooltip; server responses are appended (truncated) when available.
  • Changes

    • Comments sent to Figma now contain only the user-facing message (prefixed with a short tag) and exclude diagnostic metadata.
    • Analytics for successful comments no longer include the rule property.

Comment body now sends only the enriched violation message instead of
redundant why/impact/fix fields. Also improves error handling with
specific messages for 403/404/429 and shows error detail on hover.

Before:
  [CanICode] No Auto Layout
  Fix: Apply Auto Layout to create clear...
  Why: Without Auto Layout, AI must guess...
  Impact: Generated code uses hardcoded...
  Frame "Card" has no auto-layout...
  Node: Page > Card

After:
  [CanICode] No Auto Layout
  Frame "Card" has no auto-layout (3 children arranged vertically) — apply VERTICAL auto-layout
  Node: Page > Card

Closes #98

https://claude.ai/code/session_017n9jQMQWFoEE3Mje7MfgXs
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 26, 2026

📝 Walkthrough
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: simplifying the Figma comment to send only the enriched message instead of duplicated static text.
Linked Issues check ✅ Passed The PR successfully addresses all three objectives from issue #98: fixing error handling for the Figma comment API, removing duplication of static Fix/Why/Impact text, and ensuring the enriched violation.message is used in Figma comments.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the stated objectives: simplifying comment payload, improving error handling, and applying the changes consistently across both the web app and CLI HTML report.

✏️ 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-improve

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 26, 2026 05:03
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: 3

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

Inline comments:
In `@app/web/src/index.html`:
- Line 408: Remove the dead data-path attribute from the generated comment
button: in the code that builds commentBtn (the var commentBtn string) delete
the data-path="..." segment and the esc(issue.violation.nodePath) expression so
the button no longer outputs data-path; this is safe because postComment() reads
the node via client_meta.node_id and no longer uses data-path.

In `@src/core/report-html/index.ts`:
- Line 348: The button markup contains unused attributes data-rule and
data-path; remove these two attributes from the template string where the button
is created so the element only includes the attributes consumed by postComment
(e.g., data-file-key, data-node-id, data-message) and ensure the postComment
function is not expecting rule or path parameters; update any related template
references to use only those existing attributes (look for the button
construction and the postComment function name to locate the relevant code).
- Around line 168-170: Remove the dead extra dataset reads: the variables rule
and path (from the lines declaring const rule = btn.dataset.rule; and const path
= btn.dataset.path;) are never used, so delete those declarations and keep only
the used dataset extraction (e.g., const message = btn.dataset.message;). This
removes unused variables and satisfies linters without changing behavior in the
surrounding click/handler code that references btn.dataset or message.
🪄 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: a2ea2264-3f6f-4a8e-a740-cad6fd4678ae

📥 Commits

Reviewing files that changed from the base of the PR and between 78d6e5e and cb0a761.

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

Remove dead data-path, data-rule attributes and unused rule/path variables.
Add 400 error handling for bad request responses.

https://claude.ai/code/session_017n9jQMQWFoEE3Mje7MfgXs
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: 2

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

Inline comments:
In `@app/web/src/index.html`:
- Line 408: The button markup removed the data-rule attribute causing
btn.dataset.rule to be undefined when the analytics tracking (the click handler
that reads btn.dataset.rule) runs; fix by either re-adding the attribute to the
created button string (e.g., include data-rule="' + esc(issue.violation.rule) +
'" in the var commentBtn HTML) or remove/guard the tracking usage that reads
btn.dataset.rule (update the analytics call to skip or fallback if
btn.dataset.rule is undefined) — locate the button creation (var commentBtn ...
postComment(this)) and the click/tracking code that reads btn.dataset.rule and
make the corresponding change.

In `@src/core/report-html/index.ts`:
- Around line 181-185: The error handling for the fetch response in
src/core/report-html/index.ts only maps 403/404/429 to friendly messages; update
the status-to-message logic (the block that sets errMsg where res.status is
checked) to also handle status 400 by returning "Bad request — check node ID
format" so the thrown Error includes this same message (and existing errBody
slice) for consistency with the web app; keep the existing 403/404/429 branches
and concatenation with errBody unchanged.
🪄 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: c3eef431-2268-4e85-9a0c-3d321a5f9adf

📥 Commits

Reviewing files that changed from the base of the PR and between cb0a761 and 5f0e817.

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

- Remove btn.dataset.rule from analytics (attribute was removed)
- Add 400 status handling to report-html for consistency with web app

https://claude.ai/code/session_017n9jQMQWFoEE3Mje7MfgXs
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: 2

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

Inline comments:
In `@app/web/src/index.html`:
- Around line 436-437: The button's tooltip (btn.title) is only set on error and
persists after a later success; clear it either before re-sending or in the
success path by removing/resetting the title attribute on the same DOM element
referenced as btn. Locate the error handler that sets btn.textContent = 'Failed'
/ btn.title = e.message || String(e) and add a line to reset (e.g., btn.title =
'' or btn.removeAttribute('title')) in the success/completion path and also
clear it at the start of the retry/send function so old error text cannot
reappear on hover.

In `@src/core/report-html/index.ts`:
- Around line 190-191: The button's tooltip (btn.title) is left set on failure
so a subsequent successful retry still shows the stale error; before initiating
the retry/submit flow clear the previous error state by removing or resetting
the tooltip (e.g., set btn.title = '' or remove the title attribute) and also
reset any visible state like btn.textContent as needed; locate the retry/submit
handler that manipulates btn (references to btn.textContent and btn.title) and
insert the clearing step immediately before the send/retry logic so stale error
text cannot persist on hover.
🪄 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: 8fb96698-3275-484a-9044-f2bad86cdef1

📥 Commits

Reviewing files that changed from the base of the PR and between 5f0e817 and 4e9deb1.

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

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

♻️ Duplicate comments (1)
src/core/report-html/index.ts (1)

170-174: ⚠️ Potential issue | 🟡 Minor

Error styling classes not cleared on retry.

When a user retries after a failure, btn.title is cleared (line 174), but the error styling classes (text-red-600, border-red-500/30) added on failure (line 194) are not removed. This means:

  1. During retry, the button shows "Sending..." with red error styling
  2. On success after a prior failure, both red and green classes are applied
🐛 Clear error classes before retry
       btn.disabled = true;
       btn.textContent = 'Sending...';
       btn.title = '';
+      btn.classList.remove('text-red-600', 'border-red-500/30');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/report-html/index.ts` around lines 170 - 174, Before switching the
button into the retry/sending state, remove the error styling classes so the
"Sending..." state doesn't keep red styles and later success doesn't stack
classes; update the retry path where you set btn.disabled, btn.textContent and
btn.title to also call btn.classList.remove('text-red-600', 'border-red-500/30')
(and any other failure classes added in the failure handler) so the button is
reset before applying the sending or success styles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/web/src/index.html`:
- Around line 417-420: Before starting a retry/send attempt, clear the error
styling classes that were added on previous failure: remove 'text-red-600' and
'border-red-500/30' (and any related error classes) from the button/input
element(s) referenced by btn (the same element using btn.dataset.message,
btn.disabled, btn.textContent, btn.title) so the UI resets when you set
btn.disabled = true and btn.textContent = 'Sending...'; implement this by
calling classList.remove(...) on btn (and the specific input container if
applicable) immediately before disabling and updating the button text.

---

Duplicate comments:
In `@src/core/report-html/index.ts`:
- Around line 170-174: Before switching the button into the retry/sending state,
remove the error styling classes so the "Sending..." state doesn't keep red
styles and later success doesn't stack classes; update the retry path where you
set btn.disabled, btn.textContent and btn.title to also call
btn.classList.remove('text-red-600', 'border-red-500/30') (and any other failure
classes added in the failure handler) so the button is reset before applying the
sending or success styles.
🪄 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: 26644670-2faf-42f8-85e8-5408fb1ae0b1

📥 Commits

Reviewing files that changed from the base of the PR and between 4e9deb1 and fac7404.

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

Comment on lines +417 to +420
var commentBody = '[CanICode] ' + btn.dataset.message;
btn.disabled = true;
btn.textContent = 'Sending...';
btn.title = '';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Error styling classes not cleared on retry.

Same issue as in src/core/report-html/index.ts: the tooltip is cleared (line 420), but the error styling classes (text-red-600, border-red-500/30) added on failure (line 440) are not removed before a retry attempt.

🐛 Clear error classes before retry
       btn.disabled = true;
       btn.textContent = 'Sending...';
       btn.title = '';
+      btn.classList.remove('text-red-600', 'border-red-500/30');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/web/src/index.html` around lines 417 - 420, Before starting a retry/send
attempt, clear the error styling classes that were added on previous failure:
remove 'text-red-600' and 'border-red-500/30' (and any related error classes)
from the button/input element(s) referenced by btn (the same element using
btn.dataset.message, btn.disabled, btn.textContent, btn.title) so the UI resets
when you set btn.disabled = true and btn.textContent = 'Sending...'; implement
this by calling classList.remove(...) on btn (and the specific input container
if applicable) immediately before disabling and updating the button text.

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.

fix: Figma 코멘트 남기기 오류 수정 및 메시지 개선

2 participants