fix: フォーム系のフォーカスリングをエラー時に negative 色へ統一#280
Conversation
input / textarea / select / checkbox / radio で、従来エラー(isInvalid)時も 青(--color-ring-normal)固定だったフォーカスリングを、エラー時は枠線と同じ negative-500 に変更。リング色は isInvalid バリアントで排他指定（base から 除外）し、同色クラスの競合を避ける。switch はエラー状態が無いため対象外。 input にエラー時/通常時のリング色テストを追加。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
フォーム系コンポーネント生成時に、エラー時のリング色を negative に 切り替える規約（base に固定せず isInvalid バリアントで排他指定）を Focus Management 節へ追加。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthroughフォーム系コンポーネント（Input、Checkbox、Radio、Select、Textarea）のフォーカスリング色をエラー状態に応じて動的に切り替える仕様に統一。ベースクラスから色指定を削除し、 Changesフォーカスリング色のエラー状態連動化
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
フォーム系コンポーネント（Input/Textarea/Select/Checkbox/Radio）のエラー状態（isInvalid）において、フォーカスリング色を通常時の青から negative 系に統一し、枠線色との不整合を解消するPRです。CVA の base クラスからリング色指定を外し、isInvalid バリアント側で排他的に指定することで、クラス重複による順序依存も避けています。
Changes:
- Textarea / Select / Checkbox / Radio の focus ring 色を
isInvalidバリアントで切り替え（通常=ring-[var(--color-ring-normal)]、エラー=ring-negative-500） - Input は
isFocused（内部状態）に対するリング色を compound variants でisInvalid連動に変更 - 生成スキルの参照ドキュメントに「フォーム系のフォーカスリング色」規約を追記
- Input にリング色のテストを追加（ただし現状テスト修正が必要）
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/components/ui/input/index.tsx |
フォーカス状態時の ring 色を isInvalid 連動にするため、ring 色を compoundVariants に分離 |
src/components/ui/input/index.test.tsx |
フォーカスリング色のテストを追加（現在の実装に合わせた修正が必要） |
src/components/ui/textarea/index.tsx |
isInvalid バリアントで focus ring 色を通常/エラーで切替 |
src/components/ui/select/index.tsx |
isInvalid バリアントで focus ring 色を通常/エラーで切替 |
src/components/ui/checkbox/index.tsx |
isInvalid バリアントで focus ring 色を通常/エラーで切替（group focus-visible 連動） |
src/components/ui/radio/index.tsx |
isInvalid バリアントで focus ring 色を通常/エラーで切替（group focus-visible 連動） |
.claude/skills/add-sparkle-component/references/sparkle-design-features.md |
フォーム系 focus ring 色の規約を追記し、今後の生成に反映 |
| it("uses the negative focus ring on error (isInvalid + isFocused)", () => { | ||
| // Given: エラー状態でフォーカスされた Input | ||
| testContainer.render(<Input isInvalid isFocused />); | ||
| const container = testContainer.getContainer().firstElementChild; | ||
|
|
||
| // Then: フォーカスリングは negative 色になり、通常の青リングは付かない | ||
| expect(container?.className).toContain("ring-negative-500"); | ||
| expect(container?.className).not.toContain( | ||
| "ring-[var(--color-ring-normal)]" | ||
| ); | ||
| }); | ||
|
|
||
| it("uses the normal focus ring when valid (isFocused only)", () => { | ||
| // Given: 通常状態でフォーカスされた Input | ||
| testContainer.render(<Input isFocused />); | ||
| const container = testContainer.getContainer().firstElementChild; | ||
|
|
||
| // Then: フォーカスリングは通常の青色になり、negative リングは付かない | ||
| expect(container?.className).toContain("ring-[var(--color-ring-normal)]"); | ||
| expect(container?.className).not.toContain("ring-negative-500"); | ||
| }); |
| }, | ||
| // エラー状態のバリアント | ||
| isInvalid: { | ||
| true: "border-negative-500 hover:border-negative-600 focus-visible:border-negative-600", | ||
| true: "border-negative-500 hover:border-negative-600 focus-visible:border-negative-600 focus-visible:ring-negative-500", | ||
| false: | ||
| "border-neutral-500 hover:border-neutral-600 focus-visible:border-neutral-600", | ||
| "border-neutral-500 hover:border-neutral-600 focus-visible:border-neutral-600 focus-visible:ring-[var(--color-ring-normal)]", |
| lg: "h-12 py-1 pl-4 pr-2 gap-2 character-4-regular-pro", | ||
| }, | ||
| isInvalid: { | ||
| true: "bg-white border-negative-500 hover:border-negative-600 data-[state=open]:border-negative-600", | ||
| true: "bg-white border-negative-500 hover:border-negative-600 data-[state=open]:border-negative-600 focus-visible:ring-negative-500", | ||
| false: | ||
| "border-neutral-500 hover:border-neutral-600 data-[state=open]:border-neutral-600", | ||
| "border-neutral-500 hover:border-neutral-600 data-[state=open]:border-neutral-600 focus-visible:ring-[var(--color-ring-normal)]", | ||
| }, |
| lg: "h-6 w-6", | ||
| }, | ||
| isInvalid: { | ||
| true: [ | ||
| "border-negative-500", | ||
| "[.group:focus-visible_&]:ring-negative-500", | ||
| "[.group[data-state=checked]_&]:bg-negative-500 [.group[data-state=checked]_&]:border-none", | ||
| "[.group[data-state=indeterminate]_&]:bg-negative-500 [.group[data-state=indeterminate]_&]:border-none", | ||
| ].join(" "), | ||
| false: [ | ||
| "border-neutral-500", | ||
| "[.group:focus-visible_&]:ring-[var(--color-ring-normal)]", | ||
| "[.group[data-state=checked]_&]:bg-primary-500 [.group[data-state=checked]_&]:border-none", | ||
| "[.group[data-state=indeterminate]_&]:bg-primary-500 [.group[data-state=indeterminate]_&]:border-none", |
| lg: "h-6 w-6", | ||
| }, | ||
| isInvalid: { | ||
| true: "border-negative-500 [.group[data-state=checked]_&]:border-negative-500", | ||
| false: "border-neutral-500", | ||
| true: "border-negative-500 [.group:focus-visible_&]:ring-negative-500 [.group[data-state=checked]_&]:border-negative-500", | ||
| false: "border-neutral-500 [.group:focus-visible_&]:ring-[var(--color-ring-normal)]", | ||
| }, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/ui/input/index.test.tsx`:
- Around line 270-290: The tests incorrectly pass the isFocused prop to Input
(which isn’t used to control focus); update both cases in the Input tests so
they simulate real focus events instead: render <Input isInvalid /> or <Input />
as appropriate, locate the actual input element (from the rendered tree), call
EventHelpers.focus(input) to trigger the component’s onFocus/onBlur-driven
internal state (isInputFocused / isIconButtonFocused), then assert the
container.className contains "ring-negative-500" when isInvalid + focused and
contains "ring-[var(--color-ring-normal)]" when only focused; remove direct uses
of the isFocused prop in the assertions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d28b4a21-d364-4e39-b576-b697b154656a
📒 Files selected for processing (7)
.claude/skills/add-sparkle-component/references/sparkle-design-features.mdsrc/components/ui/checkbox/index.tsxsrc/components/ui/input/index.test.tsxsrc/components/ui/input/index.tsxsrc/components/ui/radio/index.tsxsrc/components/ui/select/index.tsxsrc/components/ui/textarea/index.tsx
| it("uses the negative focus ring on error (isInvalid + isFocused)", () => { | ||
| // Given: エラー状態でフォーカスされた Input | ||
| testContainer.render(<Input isInvalid isFocused />); | ||
| const container = testContainer.getContainer().firstElementChild; | ||
|
|
||
| // Then: フォーカスリングは negative 色になり、通常の青リングは付かない | ||
| expect(container?.className).toContain("ring-negative-500"); | ||
| expect(container?.className).not.toContain( | ||
| "ring-[var(--color-ring-normal)]" | ||
| ); | ||
| }); | ||
|
|
||
| it("uses the normal focus ring when valid (isFocused only)", () => { | ||
| // Given: 通常状態でフォーカスされた Input | ||
| testContainer.render(<Input isFocused />); | ||
| const container = testContainer.getContainer().firstElementChild; | ||
|
|
||
| // Then: フォーカスリングは通常の青色になり、negative リングは付かない | ||
| expect(container?.className).toContain("ring-[var(--color-ring-normal)]"); | ||
| expect(container?.className).not.toContain("ring-negative-500"); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 失敗原因の確認: isFocusedの利用箇所と入力コンポーネントの受け取り状況を確認
rg -n "isFocused" src/components/ui/input/index.test.tsx src/components/ui/input/index.tsx
sed -n '220,260p' src/components/ui/input/index.tsxRepository: goodpatch/sparkle-design
Length of output: 2031
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect InputProps and how `isFocused` affects rendering/classes
sed -n '1,140p' src/components/ui/input/index.tsx
# Inspect focus/trigger-related class logic around where `isFocused` is referenced
sed -n '300,420p' src/components/ui/input/index.tsx
# Inspect the test file around the two tests and available helpers/imports
sed -n '1,120p' src/components/ui/input/index.test.tsx
sed -n '240,320p' src/components/ui/input/index.test.tsxRepository: goodpatch/sparkle-design
Length of output: 13512
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all occurrences of the `isFocused` prop usage (not just variant keys)
rg -n "isFocused" src/components/ui/input/index.tsx
# Inspect focus/blur handlers and any effect that syncs prop->state
rg -n "handleInputFocus|handleInputBlur|setIsInputFocused|setIsIconButtonFocused|useEffect" src/components/ui/input/index.tsxRepository: goodpatch/sparkle-design
Length of output: 925
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how focus handlers update internal state
sed -n '230,340p' src/components/ui/input/index.tsx
# Confirm whether `isFocused` prop is destructured or otherwise used
sed -n '1,130p' src/components/ui/input/index.tsx
# (Optional) Inspect test helpers API for EventHelpers.focus signature
ls -R src/components/ui/test 2>/dev/null || true
rg -n "class EventHelpers|focus\\(" -S src/test src/components -g"*.ts" -g"*.tsx" || trueRepository: goodpatch/sparkle-design
Length of output: 8070
isFocused prop を直接渡すテストは focus イベントで駆動し直してください（src/components/ui/input/index.test.tsx:270-290）
Input はリング色を内部 state（isInputFocused / isIconButtonFocused、onFocus/onBlur）で制御しており、isFocused prop は参照されないため、テストが実挙動とズレます（isFocused は未使用のまま ...props で <input> に流れるだけです）。EventHelpers.focus() で input をフォーカスしてからクラスを検証してください。
修正案
- it("uses the negative focus ring on error (isInvalid + isFocused)", () => {
+ it("uses the negative focus ring on error (isInvalid + focus)", () => {
// Given: エラー状態でフォーカスされた Input
- testContainer.render(<Input isInvalid isFocused />);
+ testContainer.render(<Input isInvalid />);
+ const input = testContainer.queryInput();
+ EventHelpers.focus(input);
const container = testContainer.getContainer().firstElementChild;
// Then: フォーカスリングは negative 色になり、通常の青リングは付かない
expect(container?.className).toContain("ring-negative-500");
expect(container?.className).not.toContain(
"ring-[var(--color-ring-normal)]"
);
});
- it("uses the normal focus ring when valid (isFocused only)", () => {
+ it("uses the normal focus ring when valid (focus only)", () => {
// Given: 通常状態でフォーカスされた Input
- testContainer.render(<Input isFocused />);
+ testContainer.render(<Input />);
+ const input = testContainer.queryInput();
+ EventHelpers.focus(input);
const container = testContainer.getContainer().firstElementChild;
// Then: フォーカスリングは通常の青色になり、negative リングは付かない
expect(container?.className).toContain("ring-[var(--color-ring-normal)]");
expect(container?.className).not.toContain("ring-negative-500");
});🧰 Tools
🪛 GitHub Actions: CI / 0_lint-and-test.txt
[error] 276-276: Test failed: expected container.className to contain 'ring-negative-500' but received 'flex gap-0 items-center w-full rounded-action border transition-colors p-1 h-10 ... border-negative-500 ...' (Input > Invalid State > uses the negative focus ring on error (isInvalid + isFocused)).
[error] 288-288: Test failed: expected container.className to contain 'ring-[var(--color-ring-normal)]' but it did not (received 'flex gap-0 items-center w-full rounded-action border ... border-neutral-500 ... cursor-text') (Input > Invalid State > uses the normal focus ring when valid (isFocused only)).
🪛 GitHub Actions: CI / lint-and-test
[error] 276-276: AssertionError: expected container.className to contain 'ring-negative-500' but received '...border-negative-500...' (missing expected focus ring class).
[error] 288-288: AssertionError: expected container.className to contain 'ring-[var(--color-ring-normal)]' but received '...border-neutral-500...' (missing expected normal focus ring class).
🪛 GitHub Check: lint-and-test
[failure] 288-288: src/components/ui/input/index.test.tsx > Input > Invalid State > uses the normal focus ring when valid (isFocused only)
AssertionError: expected 'flex gap-0 items-center w-full rounde…' to contain 'ring-[var(--color-ring-normal)]'
Expected: "ring-[var(--color-ring-normal)]"
Received: "flex gap-0 items-center w-full rounded-action border bg-white transition-colors p-1 h-10 character-3-regular-pro border-neutral-500 hover:border-neutral-600 cursor-text"
❯ src/components/ui/input/index.test.tsx:288:36
[failure] 276-276: src/components/ui/input/index.test.tsx > Input > Invalid State > uses the negative focus ring on error (isInvalid + isFocused)
AssertionError: expected 'flex gap-0 items-center w-full rounde…' to contain 'ring-negative-500'
Expected: "ring-negative-500"
Received: "flex gap-0 items-center w-full rounded-action border transition-colors p-1 h-10 character-3-regular-pro border-negative-500 hover:border-negative-600 bg-white cursor-text"
❯ src/components/ui/input/index.test.tsx:276:36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/input/index.test.tsx` around lines 270 - 290, The tests
incorrectly pass the isFocused prop to Input (which isn’t used to control
focus); update both cases in the Input tests so they simulate real focus events
instead: render <Input isInvalid /> or <Input /> as appropriate, locate the
actual input element (from the rendered tree), call EventHelpers.focus(input) to
trigger the component’s onFocus/onBlur-driven internal state (isInputFocused /
isIconButtonFocused), then assert the container.className contains
"ring-negative-500" when isInvalid + focused and contains
"ring-[var(--color-ring-normal)]" when only focused; remove direct uses of the
isFocused prop in the assertions.
|
取り下げます。ガイドライン（sparkle-design-docs の input/textarea/select/checkbox/radio の states）で「Invalid Focus 状態のフォーカス枠は青」が意図的に規定されており（例: input.json「Invalid Focus状態のInput。青い枠線で…」）、本変更（エラー時リングを negative=赤系にする）は設計仕様と矛盾するため。元の実装（フォーカスリングは常に青）が仕様準拠でした。 もしエラー時のフォーカス色を変える設計自体を見直す場合は、Figma / ガイドライン側の更新を起点に進めます。 |
背景
エラー（
isInvalid）状態でフォーカスしても、フォーカスリングが青（--color-ring-normal）のままで、赤い枠線とちぐはぐだった。「エラー時はフォーカスも negative 色に」をフォーム系コンポーネント全般で統一する。変更
input/textarea/select/checkbox/radioのフォーカスリング色を状態連動に変更：ring-[var(--color-ring-normal)]（青）isInvalid):ring-negative-500（枠線と同色）リング色は base クラスから外し
isInvalidバリアントで排他指定（false→青 / true→negative）。同一プロパティの色クラスが二重に当たって Tailwind の生成順に依存する不具合を避けるため。専用トークンは新設せず、エラー枠線と同じnegative-500をコンポーネント側で指定。switchはエラー状態が無いため対象外。internalの各 Input（input-date/number/file 等）はisInvalidを本 Input に委譲済みのため、自動で波及する。スキル
add-sparkle-componentスキルの Focus Management 節に focus ring 色の規約（normal=ring-normal / invalid=negative、base 固定禁止）を追記。今後のフォーム系生成で踏襲される。検証
type-check✅ /build（tsc -p tsconfig.build.json）✅inputにエラー時/通常時のリング色テストを追加sparkle-design-cli check（変更5コンポーネント）: 新規 findings 0（既存 11 は main と同一）関連・フォローアップ（別対応）
🤖 Generated with Claude Code