Skip to content

fix: normalize combobox & select to 32px field height + keyboard focus fixes#141

Open
boramyi-ts wants to merge 4 commits into
mainfrom
fix/combobox-select-align-focus
Open

fix: normalize combobox & select to 32px field height + keyboard focus fixes#141
boramyi-ts wants to merge 4 commits into
mainfrom
fix/combobox-select-align-focus

Conversation

@boramyi-ts

@boramyi-ts boramyi-ts commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Normalizes Combobox and Select to the standard 32px field height, and corrects several keyboard-focus indicators. (The input-group focus inner-shadow bug is split into its own PR — SW-2043 / #142.)

Field height → 32px (consistency)

  • Select: default h-10 (40px) → h-8 (32px); smh-7 (28px).
  • Combobox: 40px → 32px — now inherits the InputGroup base height instead of overriding it.
  • Brings both in line with Input, Button, Toggle (already 32px), so all field controls share one height.

Combobox ↔ Select consistency

  • Combobox single field: bg-card background + item gap-1.5 (match Select).
  • Combobox chips: bg-card + keyboard-only focus (has-[[data-slot=combobox-chip-input]:focus-visible] instead of focus-within, which also fired on mouse click).

Focus-shadow fixes (keyboard :focus-visible correctness)

  • Slider thumb: focus halo scoped to :focus-visible only (was also on hover + active drag).
  • Accordion trigger: removed the orphan focus-visible:after:border-ring (no ::after geometry — a dead no-op).

Files: combobox.tsx, select.tsx, slider.tsx, accordion.tsx. yarn lint + yarn typecheck pass.

Prototypes

Field height — 40px vs 32px (the same form rendered at both heights, all controls):

field height 40 vs 32

Focus before/after (slider, accordion, combobox chips, + Input/Textarea/Select reference rows):

🤖 Generated with Claude Code

- Combobox single field now matches the Select trigger: h-10 (40px) +
  bg-card, and items use gap-1.5 — the two controls were visually
  inconsistent in height and background.
- Combobox chips: bg-card background + keyboard-only focus indicator
  (has-[chip-input:focus-visible] instead of focus-within, which fired on
  mouse click too).
- Slider thumb: focus shadow scoped to :focus-visible only (was also
  showing on hover and active drag).
- Accordion trigger: removed the orphan focus-visible:after:border-ring
  (no ::after geometry — a dead no-op).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@boramyi-ts boramyi-ts requested review from a team as code owners June 18, 2026 17:11
@vercel

vercel Bot commented Jun 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ts-lib-ui-kit-storybook Ready Ready Preview, Comment Jun 23, 2026 7:41pm

Request Review

@boramyi-ts boramyi-ts temporarily deployed to artifactory-prod June 18, 2026 17:11 — with GitHub Actions Inactive
@boramyi-ts boramyi-ts changed the title fix: align combobox with select and correct keyboard focus indicators fix: SW-2075 - align combobox with select and correct keyboard focus indicators Jun 18, 2026
Brings Select (default 40px -> 32px; sm -> 28px) and Combobox (was 40px)
down to the standard 32px field height shared by Input, Input Group,
Button, and Toggle. Combobox now inherits the InputGroup base height
rather than overriding it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@boramyi-ts boramyi-ts temporarily deployed to artifactory-prod June 18, 2026 17:46 — with GitHub Actions Inactive
@boramyi-ts boramyi-ts changed the title fix: SW-2075 - align combobox with select and correct keyboard focus indicators fix: normalize combobox & select to 32px field height + keyboard focus fixes Jun 18, 2026
@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🟢 Lines 94.74% (🎯 83%)
🟰 ±0%
20627 / 21770
🟢 Statements 94.74% (🎯 83%)
🟰 ±0%
20627 / 21770
🟢 Functions 93.58% (🎯 74%)
🟰 ±0%
919 / 982
🟢 Branches 88.74% (🎯 81%)
🟰 ±0%
3768 / 4246
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/components/ui/accordion.tsx 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
src/components/ui/combobox.tsx 100%
🟰 ±0%
86.95%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
src/components/ui/select.tsx 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
src/components/ui/slider.tsx 96%
🟰 ±0%
44.44%
🟰 ±0%
100%
🟰 ±0%
96%
🟰 ±0%
17, 20
Generated in workflow #809 for commit 958f949 by the Vitest Coverage Report Action

Copilot AI left a comment

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.

Pull request overview

This PR aligns Combobox and Select with the library’s standard 32px field height and tightens several focus-indicator behaviors to be keyboard-accurate (:focus-visible) across UI primitives.

Changes:

  • Normalize SelectTrigger heights to 32px (default) and 28px (sm).
  • Adjust Combobox styling to better match Select (field background, item spacing, and chips focus-ring behavior scoped to keyboard focus).
  • Fix focus indicator behavior by scoping slider thumb halo to :focus-visible and removing a no-op accordion trigger class; relax PR title subject requirements to allow optional Jira prefix.

Reviewed changes

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

Show a summary per file
File Description
src/components/ui/combobox.tsx Aligns combobox field/chips background and focus behavior, and tweaks item spacing for consistency with Select.
src/components/ui/select.tsx Updates trigger height tokens to match the 32px/28px field-height standard.
src/components/ui/slider.tsx Ensures thumb focus halo appears only for keyboard focus (:focus-visible).
src/components/ui/accordion.tsx Removes an orphaned focus-visible:after:* utility that had no effect.
.github/workflows/pr-title-checker.yml Allows optional Jira prefix while still requiring a non-empty semantic subject.

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

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.

4 participants