Skip to content

fix: フォーム系のフォーカスリングをエラー時に negative 色へ統一 #280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
touyou wants to merge 2 commits into from
Closed

fix: フォーム系のフォーカスリングをエラー時に negative 色へ統一 #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
21345d2
fix: フォーム系のフォーカスリングをエラー時に negative 色へ ♿
touyou Jun 3, 2026
d71eca7
docs(skill): focus ring 色の規約を add-sparkle-component に追記 📝
touyou Jun 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .claude/skills/add-sparkle-component/references/sparkle-design-features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,19 @@ Components include appropriate ARIA attributes:
- Logical tab order
- Skip links for navigation

#### Focus ring color (form components)

Form components (`input`, `textarea`, `select`, `checkbox`, `radio`, …) drive the
focus ring color from their error state — do **not** hardcode the blue ring on the
error path:

- Normal state → `ring-[var(--color-ring-normal)]` (blue)
- Error state (`isInvalid`) → `ring-negative-500`, matching the negative border

Define the ring color in the `isInvalid` variant (`false` → ring-normal, `true` →
ring-negative-500) rather than in the base class, so only one ring color ever
applies. `switch` has no error state, so it keeps the normal ring only.

### Color Contrast

All color combinations meet WCAG 2.1 Level AA standards:
Expand Down
6 changes: 5 additions & 1 deletion src/components/ui/checkbox/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ const checkboxItemVariants = cva(
const checkboxRootVariants = cva(
[
"rounded-xs border-2 transition-colors",
"[.group:focus_&]:outline-hidden [.group:focus-visible_&]:ring-2 [.group:focus-visible_&]:ring-[var(--color-ring-normal)] [.group:focus-visible_&]:ring-offset-2",
// フォーカスリング色は isInvalid バリアントで指定（通常=青／エラー=negative）。
// en: Focus ring color is set in the isInvalid variant (blue normally / negative on error).
"[.group:focus_&]:outline-hidden [.group:focus-visible_&]:ring-2 [.group:focus-visible_&]:ring-offset-2",
].join(" "),
{
variants: {
Expand All @@ -49,11 +51,13 @@ const checkboxRootVariants = cva(
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",
].join(" "),
Expand Down
22 changes: 22 additions & 0 deletions src/components/ui/input/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,28 @@
// Then: invalid状態のクラスが保持される（実際のCVAクラス名）
expect(container?.className).toContain("border-negative-500");
});

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");

Check failure on line 276 in src/components/ui/input/index.test.tsx

View workflow job for this annotation

GitHub Actions / lint-and-test

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
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)]");

Check failure on line 288 in src/components/ui/input/index.test.tsx

View workflow job for this annotation

GitHub Actions / lint-and-test

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
expect(container?.className).not.toContain("ring-negative-500");
});
Comment on lines +270 to +290
Comment on lines +270 to +290

@coderabbitai coderabbitai Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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.tsx

Repository: 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.tsx

Repository: 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.tsx

Repository: 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" || true

Repository: 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.

});

describe("Accessibility", () => {
Expand Down
14 changes: 13 additions & 1 deletion src/components/ui/input/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,23 @@ const inputVariants = cva(
false: "",
},
isFocused: {
true: "ring-2 ring-[var(--color-ring-normal)] ring-offset-2 outline-hidden",
true: "ring-2 ring-offset-2 outline-hidden",
false: "",
},
},
compoundVariants: [
// フォーカスリング色。通常は青、エラー時は枠線と同じ negative に揃える。
// en: Focus ring color. Blue normally; on error it matches the negative border.
{
isFocused: true,
isInvalid: false,
className: "ring-[var(--color-ring-normal)]",
},
{
isFocused: true,
isInvalid: true,
className: "ring-negative-500",
},
// 通常状態
{
isInvalid: false,
Expand Down
8 changes: 5 additions & 3 deletions src/components/ui/radio/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ const radioItemVariants = cva(
const radioIndicatorVariants = cva(
[
"flex items-center justify-center rounded-full border border-2 transition-colors",
"[.group:focus_&]:outline-hidden [.group:focus-visible_&]:ring-2 [.group:focus-visible_&]:ring-[var(--color-ring-normal)] [.group:focus-visible_&]:ring-offset-2",
// フォーカスリング色は isInvalid バリアントで指定（通常=青／エラー=negative）。
// en: Focus ring color is set in the isInvalid variant (blue normally / negative on error).
"[.group:focus_&]:outline-hidden [.group:focus-visible_&]:ring-2 [.group:focus-visible_&]:ring-offset-2",
].join(" "),
{
variants: {
Expand All @@ -64,8 +66,8 @@ const radioIndicatorVariants = cva(
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)]",
},
Comment on lines 66 to 71
isDisabled: {
true: "",
Expand Down
8 changes: 5 additions & 3 deletions src/components/ui/select/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import { cn } from "@/lib/utils";
const selectTriggerVariants = cva(
[
"flex items-center justify-between w-full rounded-action border bg-white text-text-high transition-colors",
"focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[var(--color-ring-normal)] focus-visible:ring-offset-2",
// フォーカスリング色は isInvalid バリアントで指定（通常=青／エラー=negative）。
// en: Focus ring color is set in the isInvalid variant (blue normally / negative on error).
"focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2",
"overflow-hidden whitespace-nowrap",
].join(" "),
{
Expand All @@ -25,9 +27,9 @@ const selectTriggerVariants = cva(
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)]",
},
Comment on lines 27 to 33
isDisabled: {
true: "cursor-not-allowed bg-neutral-50 border-neutral-200 hover:border-neutral-200 text-text-disabled",
Expand Down
8 changes: 5 additions & 3 deletions src/components/ui/textarea/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import { cn } from "@/lib/utils";
*/
const textareaVariants = cva(
// ベーススタイル
"flex w-full rounded-action border bg-white px-3 py-1 ring-offset-background placeholder:text-base-400 focus-visible:outline-hidden focus-visible:ring-2 focus-visible:ring-[var(--color-ring-normal)] focus-visible:ring-offset-2 resize",
// フォーカスリング色は isInvalid バリアントで指定（通常=青／エラー=negative）。
// en: Focus ring color is set in the isInvalid variant (blue normally / negative on error).
"flex w-full rounded-action border bg-white px-3 py-1 ring-offset-background placeholder:text-base-400 focus-visible:outline-hidden focus-visible:ring-2 focus-visible:ring-offset-2 resize",
{
variants: {
// サイズバリアント（sm, md, lg）
Expand All @@ -26,9 +28,9 @@ const textareaVariants = cva(
},
// エラー状態のバリアント
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)]",
Comment on lines 28 to +33
},
// 無効状態のバリアント
isDisabled: {
Expand Down
Loading