Skip to content

fix: prevent notification close buttons from submitting forms - #408

Merged
yoyo837 merged 1 commit into
react-component:masterfrom
nrps9909:codex/close-button-type
Aug 27, 2026
Merged

fix: prevent notification close buttons from submitting forms#408
yoyo837 merged 1 commit into
react-component:masterfrom
nrps9909:codex/close-button-type

Conversation

@nrps9909

@nrps9909 nrps9909 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • set type="button" on the built-in notification close control
  • add a regression assertion to the existing close-button test

Native buttons default to type="submit". Notifications can be mounted into a custom container inside a form, so clicking the built-in close control could otherwise submit an unrelated form. The action buttons in this repository already use explicit button types; this aligns the close control with that contract.

Verification

  • Exact-base regression: 45 passed, 1 failed because the close button had no type
  • Fixed focused file: 46/46 passed
  • Fixed full suite: 3 files, 52/52 passed
  • npm run tsc
  • npm run lint — 0 errors, 10 pre-existing warnings
  • npm run compile — ESM, CJS, declarations, and Less compilation passed
  • focused Prettier check and git diff --check

I found no open issue or PR addressing notification close-button submission, and audited all current open PR changed files before submission.

AI assistance disclosure: Codex was used to audit native-button defaults, check open work for overlap, add the regression, and run verification. The exact-base failure and final one-attribute runtime change were reviewed locally.

Summary by CodeRabbit

  • 错误修复
    • 修复通知关闭按钮在表单中可能意外触发表单提交的问题。
    • 关闭按钮现明确标记为普通按钮类型,提升交互稳定性。

@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

@nrps9909 is attempting to deploy a commit to the afc163's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: df09b4d5-d0b6-4fa5-8e2d-471b3ec88194

📥 Commits

Reviewing files that changed from the base of the PR and between 4f26598 and ecfa169.

📒 Files selected for processing (2)
  • src/Notification.tsx
  • tests/index.test.tsx

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

通知组件的关闭按钮新增 type="button" 属性。测试新增断言,验证关闭按钮不会按默认提交按钮处理。

Changes

通知关闭按钮行为

Layer / File(s) Summary
关闭按钮类型与测试
src/Notification.tsx, tests/index.test.tsx
关闭按钮显式设置 type="button"notification close node 测试验证该属性值为 button

Estimated code review effort: 1(简单)| ~5 分钟

Merge Risk: ⚪ Minimal · up to ecfa1

The close control now avoids unintentionally submitting forms when notifications are mounted inside them, with regression coverage and validation completed; no actionable merge-blocking risk remains.

Poem

小兔看见按钮换了类型
表单不再误触提交铃
关闭动作轻轻落地
测试守住属性边界
胡萝卜也为此欢庆

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了主要变更:为通知关闭按钮添加 type="button",防止其在表单中触发表单提交。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 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

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

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.20%. Comparing base (4f26598) to head (ecfa169).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #408   +/-   ##
=======================================
  Coverage   99.20%   99.20%           
=======================================
  Files          12       12           
  Lines         376      376           
  Branches      102      102           
=======================================
  Hits          373      373           
  Misses          3        3           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yoyo837
yoyo837 merged commit 12df0e6 into react-component:master Aug 27, 2026
11 of 12 checks passed
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.

2 participants