Skip to content

fix: Checkbox が className を受け取れるようにする#17

Open
ryoshin0830 wants to merge 2 commits into
pepabo:mainfrom
ryoshin0830:fix/checkbox-classname-merge
Open

fix: Checkbox が className を受け取れるようにする#17
ryoshin0830 wants to merge 2 commits into
pepabo:mainfrom
ryoshin0830:fix/checkbox-classname-merge

Conversation

@ryoshin0830

Copy link
Copy Markdown

概要

Checkbox に className を渡してもルートの label に反映されず消えてしまう問題を修正します。

Closes #16

変更内容

  • {...props} の後に固定で当てていた className="aria-checkbox" を、Radio(src/Radio/Radio.tsx)と同じ clsx マージに変更
  • 挙動を固定するテストを追加(react-dom/server の SSR 出力で class 属性を検証。依存追加なし・node 環境で動作)

変更前後

// before: 渡した className が上書きされて消える
<AriaCheckbox {...props} ref={ariaRef} className="aria-checkbox">

// after: マージされて残る
<AriaCheckbox {...props} ref={ariaRef} className={clsx('aria-checkbox', className)}>

確認

  • npx vitest run … 2 passed(修正前は 1 failed で、class="aria-checkbox" のみになり渡した class が消えることを確認済み)
  • npm run lint / npm run build … PASS

スコープ外

{...props} の後に固定の className を当てていたため、利用側から渡した
className が上書きされて消えていた。Radio と同じ clsx マージに揃える。

Closes pepabo#16

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 3, 2026 09:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Checkbox コンポーネントに渡した className が固定 className によって上書きされて消えてしまう問題(Radio と挙動不一致)を解消し、今後の回帰を防ぐためのテストを追加する PR です。

Changes:

  • Checkbox のルート要素(AriaCheckbox)に指定する classNameclsx でマージするよう修正
  • react-dom/server の SSR 出力を使って className マージ挙動を検証するテストを追加

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/Checkbox/index.tsx 固定クラスと利用側 classNameclsx でマージして上書き消失を防止
src/Checkbox/Checkbox.test.tsx SSR マークアップで className マージ挙動の回帰テストを追加

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Checkbox/Checkbox.test.tsx Outdated
Comment on lines +8 to +11
const html = renderToStaticMarkup(<Checkbox className="my-card">ラベル</Checkbox>)

expect(html).toContain('aria-checkbox')
expect(html).toContain('my-card')
Comment thread src/Checkbox/Checkbox.test.tsx Outdated
Comment on lines +15 to +18
const html = renderToStaticMarkup(<Checkbox>ラベル</Checkbox>)

expect(html).toContain('aria-checkbox')
expect(html).not.toContain('undefined')
HTML 全文への部分一致だと、クラスが別の要素に付いた場合や無関係な
文字列にも反応し得るため、ルートの label の class 属性を取り出して
検証する。レビュー指摘への対応。

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ほぼclsxのテストなので若干過剰か?とは思ったりした

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

確かに過剰です!修正します

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.

Checkbox が className を受け取れない(固定 className に上書きされる)

3 participants