Skip to content

feat(Switch, Checkbox, Radio): add padded variant + new Radio component#751

Open
sadiqxansari wants to merge 9 commits into
frappe:mainfrom
sadiqxansari:feat/switch-padded-variant
Open

feat(Switch, Checkbox, Radio): add padded variant + new Radio component#751
sadiqxansari wants to merge 9 commits into
frappe:mainfrom
sadiqxansari:feat/switch-padded-variant

Conversation

@sadiqxansari
Copy link
Copy Markdown
Collaborator

@sadiqxansari sadiqxansari commented Jun 2, 2026 •
edited by github-actions Bot
Loading

Adds a padded variant to Switch, Checkbox, and Radio — a clickable row surface with hover, active, and keyboard-only focus states. All three share the same tokens, 28/32px row heights, and data-slot click guards.

  • Switch: variant + RTL-safe switchPosition (start/end). Error text now aligns with label (not the switch) on switch-at-start rows.
  • Checkbox: variant, padding deprecated. px-2 → px-1.5 on sm padded rows.
  • Radio: new component. value + name group selection; thick-ring/white-centre selected state (4px sm / 4.5px md); required omitted by design (asterisk belongs on the group heading). Stories: Variants, Sizes, Labeling, Required, States, Settings list.
  • All three padded containers get transition-colors for smooth hover.

Default variants are unchanged; existing callers are unaffected.

Coverage: 56.14% (+0.11% vs main)

@sadiqxansari sadiqxansari force-pushed the feat/switch-padded-variant branch 2 times, most recently from 4a71f65 to 26dddf7 Compare June 2, 2026 14:35
@netchampfaris
Copy link
Copy Markdown
Contributor

netchampfaris commented Jun 2, 2026

/barista review

@barista-for-frappe
Copy link
Copy Markdown

barista-for-frappe Bot commented Jun 2, 2026 •
edited
Loading

Re-reviewing per @netchampfaris.

Concerns — new public surface drifts from the canonical variant vocabulary, and the change silently flips the existing default layout.

  • src/components/Switch/types.ts:12 — variant: 'default' | 'padded' repurposes the library's most canonical prop name. variant is fixed to the visual-style axis (solid | outline | subtle | ghost) — CONTEXT.md:75, P4; the shared InputVariant in src/composables/inputTypes.ts:8 is subtle | outline | ghost. Using variant for a layout/affordance axis collides with that meaning and locks the real variant out of Switch later. Name the axis instead (P3). Since it's exactly two states, a boolean reads best — precedent is Dialog's bare (CONTEXT.md:36), a boolean that toggles chrome/layout, not a variant enum. Suggest padded?: boolean.

  • src/components/Switch/Switch.vue (switchGroupClasses, inline/no-description branch) — the default variant's layout changed, not just the new one. Was flex justify-between + gap-x-3 (label left, switch right). Now flex flex-row-reverse justify-end ... gap-x-2.5 (switch left, label right, tighter gap) for all default-variant rows. Every existing <Switch label="…" /> with no description silently flips on a minor release. The new look should be gated behind the new prop; leave default rendering as it did. (P13 spirit — don't break existing callers without opt-in.)

Non-blocking, verified fine:

  • Double-toggle guard in onContainerClick is correct — data-slot="label" (InputLabel.vue:6) and data-slot="control" (Switch.vue:39) both exist, so label-for and control self-toggle are both excluded.
  • Padded focus handling is a11y-sound: control ring suppressed, row shows :has(:focus-visible) ring on keyboard focus only (P12).

Suggest: rename variant → padded boolean, and scope the layout flip to the padded case so the default stays put.

barista · claude-opus-4-8 · 66 in / 7.7k out · 1347k cached · 120s · $0.435

@sadiqxansari sadiqxansari changed the title feat(Switch): add padded variant with settings-row examples feat(Switch, Checkbox): add padded variant Jun 2, 2026
@netchampfaris
Copy link
Copy Markdown
Contributor

netchampfaris commented Jun 3, 2026

@sadiqxansari Add UI tests

sadiqxansari and others added 7 commits June 4, 2026 00:45
@sadiqxansari @claude
feat(Switch): add padded variant with settings-row examples
3512488 
- Add `variant` prop (`default` | `padded`) — padded wraps the row in a
  clickable surface with hover, active and focus-visible ring states
- Left-align the switch before the label for inline (default) rows;
  right-align when a description is present
- Make the entire padded row clickable via a container click handler;
  guard against double-toggle from the label `for` attribute and the
  control itself using `data-slot` selectors
- Use `has(:focus-visible)` instead of `focus-within` so the focus ring
  only appears on keyboard navigation, not mouse clicks
- Add Variants, SettingsList and SettingsRows story examples
- Regenerate Switch.api.md to include the new `variant` prop

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sadiqxansari @claude
feat(Switch): switchPosition prop, toolbar example, and polish
552b54a 
- Add `switchPosition` prop with automatic defaults — `left` for
  label-only rows, `right` when a description is present; explicit
  value always wins
- Restore default variant layout to original (switch right, label left)
  so existing callers are unaffected; new prop required to opt in to
  left placement
- Fix description rows: `items-start` → `items-center` so switch aligns
  with the label vertically
- Padded variant: fixed height (`h-7` sm / `h-8` md) with `justify-center`
  instead of vertical padding, and `group` moved to outer container so
  hover applies across the entire row including corners
- Focus ring switched from `focus-within` to `has(:focus-visible)` so
  it only appears on keyboard navigation
- Replace grouped-toggles card example with a toolbar story showing the
  padded switch alongside Button components
- Icons story: Notifications row now `justify-between` with switch right
- Variants story: default row padded to align switch with padded row
- Remove deprecated `change` emit section and LegacyChange story
- Regenerate Switch.api.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sadiqxansari @claude
refactor(Switch): use RTL-safe start/end for switchPosition
75358d6 
Rename the `switchPosition` values from physical `left`/`right` to logical
`start`/`end` so the prop reads correctly under RTL (matching the codebase's
`ps-`/`me-` logical-property convention). The underlying layout already used
direction-aware `flex-row-reverse` / `justify-*`, so only the public values
change.

- `start` = leading side, `end` = trailing side
- Auto default unchanged in behaviour: `start` for label-only rows, `end`
  when a description is present
- Update the Icons story and regenerate Switch.api.md

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sadiqxansari @claude
feat(Checkbox): add padded variant with selection and settings examples
e0bba3d 
Mirror the Switch padded variant so the two controls stay consistent.

- Add a `variant` prop (`default` | `padded`); `padded` wraps the control and
  label in a clickable surface with hover, active and keyboard-only focus ring
  states, at a fixed 28px (sm) / 32px (md) row height. The checkbox always
  stays on the leading side
- Make the whole padded row clickable via a container click handler, guarding
  against double-toggle from the input and the label `for` using `data-slot`
  selectors
- Point the deprecated `padding` prop at `variant="padded"`
- Add Variants, MemberList (selection list) and SettingsList example stories
- Regenerate Checkbox.api.md

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sadiqxansari
feat(Radio): add Radio component with padded variant
4552b82 
- New Radio component mirroring Checkbox/Switch: size, variant, label,
  description, error, disabled, name, value props; v-model group selection
- `padded` variant: 28/32px clickable surface, same tokens as Checkbox/Switch
- Selected state: thick ring (4px sm / 4.5px md) + white hollow centre
- All interaction states: hover, active, keyboard-only focus ring; disabled
  states use muted #ededed ring and #c7c7c7 centre
- `required` intentionally omitted — asterisk belongs on the group heading;
  docs show the correct pattern in a new Required section
- Stories: Default, Variants, Sizes, Labeling, Required, States, Settings list
- Checkbox: px-2 → px-1.5 for sm padded; transition-colors on padded container
- Switch: error/description text aligned with label (not switch) for
  switch-at-start rows; transition-colors on padded container
- Radio padded container: transition-colors

Co-Authored-By: Sadiq Ansari <sadiqxansari@gmail.com>
@sadiqxansari
fix(Switch,Checkbox,Radio): cursor-pointer on labels and checkbox input
af25019 
Co-Authored-By: Sadiq Ansari <sadiqxansari@gmail.com>
@sadiqxansari
test(Switch,Checkbox,Radio): add UI tests for padded variant
90a5bbd 
Co-Authored-By: Sadiq Ansari <sadiqxansari@gmail.com>
@sadiqxansari sadiqxansari force-pushed the feat/switch-padded-variant branch 2 times, most recently from 9261af0 to 90a5bbd Compare June 3, 2026 19:15
@sadiqxansari @claude
fix(Checkbox, Radio): remove duplicate update:modelValue emit
85742c4 
defineModel's setter already emits update:modelValue internally;
the explicit emit() calls were firing it twice, causing the padded
variant "does not double-toggle/select" Cypress tests to fail.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sadiqxansari sadiqxansari changed the title feat(Switch, Checkbox): add padded variant feat(Switch, Checkbox, Radio): add padded variant + new Radio component Jun 3, 2026
@sadiqxansari @claude
ci: retrigger docs preview
41e93a8 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sadiqxansari @netchampfaris