Skip to content

feat: add RadioCard slot-based styling and recipe story #1726

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.

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

nhironaka
Copy link
Contributor

@nhironaka nhironaka commented Jul 29, 2025

Summary

This PR implements a CSS-only RadioCard variant for RadioGroup components using react-aria-components, replacing the previous JavaScript context-based approach with pure CSS descendant selectors.

Key Changes:

  • Added variant="card" prop to RadioGroup that applies card styling to child Radio components
  • Implemented CSS-only parent-to-child styling using data-variant attributes and descendant selectors
  • Added support for slotted content: heading (icon + label), subtitle, and description
  • Added conditional border radius (bottom radius = 0 when no description slot present)
  • Added subtitle slot that matches gonfalon's shortDescription styling (secondary text color, body-2-regular font, primary color when selected)

Technical Approach:

  • RadioGroup sets data-variant="card" attribute on container
  • CSS targets child Radio components with .group[data-variant='card'] label[data-rac] selector
  • Uses CSS Grid for complex layouts with conditional template areas based on slot presence
  • Maintains all existing Radio component functionality without modifications

Screenshots

The RadioCardGroup story demonstrates the card styling with all slot types:

Jul-30-2025 16-14-15

Testing approaches

  • Storybook integration: Added RadioCardGroup story in composition.stories.tsx demonstrating all slot combinations
  • Visual testing: Verified styling matches gonfalon's RadioCard component design
  • Linting: Passes biome checks (minor CSS specificity warning noted but non-blocking)
  • Slot-based layout: Tested heading, subtitle, description slots in various combinations

Human Review Checklist

⚠️ Critical items to verify:

  • Visual consistency with gonfalon's RadioCard component (especially subtitle styling)
  • CSS grid layout works correctly with all slot combinations (heading±subtitle±description)
  • CSS specificity warning doesn't cause styling conflicts in production
  • Accessibility maintained (focus states, ARIA attributes, keyboard navigation)
  • Disabled and selected states render correctly
  • CSS-only approach is robust and won't break if Radio component internals change

Additional Context

  • Session: https://app.devin.ai/sessions/ff993275c60449cba22b3558e0ea87d7
  • Requested by: @nhironaka
  • Approach Evolution: Started with slot-based styling, moved to JavaScript context, finally settled on CSS-only descendant selectors for better separation of concerns
  • CSS Warning: Minor descending specificity warning in RadioGroup.module.css line 80 - should be monitored for potential conflicts

- Add slot='card' styling to Radio component for enhanced card layout
- Implement slot-based styling for icon, label, and description content
- Add RadioCardGroup recipe story for experiment type selection patterns
- Follow existing component patterns from Menu component slot styling
- Support all radio states (selected, disabled, focus, hover) in card mode

Co-Authored-By: Naomi Hironaka <[email protected]>
@nhironaka nhironaka requested a review from a team as a code owner July 29, 2025 15:42
Copy link

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link

changeset-bot bot commented Jul 29, 2025

🦋 Changeset detected

Latest commit: 374e709

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@launchpad-ui/components Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

pkg-pr-new bot commented Jul 29, 2025

yarn add https://pkg.pr.new/@launchpad-ui/[email protected]
yarn add https://pkg.pr.new/@launchpad-ui/[email protected]
yarn add https://pkg.pr.new/@launchpad-ui/[email protected]

commit: 374e709

Copy link
Contributor

github-actions bot commented Jul 29, 2025

Size Change: +584 B (+0.11%)

Total Size: 527 kB

Filename Size Change
packages/components/dist/index.es.js 17.6 kB +118 B (+0.67%)
packages/components/dist/index.js 18.5 kB +106 B (+0.58%)
packages/components/dist/style.css 8.28 kB +360 B (+4.54%) 🔍
ℹ️ View Unchanged
Filename Size
apps/vscode/dist/client.js 111 kB
apps/vscode/dist/server.js 261 kB
packages/box/dist/index.es.js 7.26 kB
packages/box/dist/index.js 7.82 kB
packages/box/dist/style.css 2.67 kB
packages/button/dist/index.es.js 1.89 kB
packages/button/dist/index.js 2.32 kB
packages/button/dist/style.css 3 kB
packages/core/dist/index.es.js 512 B
packages/core/dist/index.js 1.27 kB
packages/drawer/dist/index.es.js 1.76 kB
packages/drawer/dist/index.js 2.22 kB
packages/drawer/dist/style.css 497 B
packages/dropdown/dist/index.es.js 1.15 kB
packages/dropdown/dist/index.js 1.59 kB
packages/filter/dist/index.es.js 2.23 kB
packages/filter/dist/index.js 2.68 kB
packages/filter/dist/style.css 881 B
packages/focus-trap/dist/index.es.js 418 B
packages/focus-trap/dist/index.js 852 B
packages/form/dist/index.es.js 4.25 kB
packages/form/dist/index.js 4.73 kB
packages/form/dist/style.css 2.21 kB
packages/icons/dist/index.es.js 1.3 kB
packages/icons/dist/index.js 1.73 kB
packages/icons/dist/style.css 532 B
packages/menu/dist/index.es.js 3.69 kB
packages/menu/dist/index.js 4.16 kB
packages/menu/dist/style.css 872 B
packages/modal/dist/index.es.js 3.08 kB
packages/modal/dist/index.js 3.55 kB
packages/modal/dist/style.css 898 B
packages/navigation/dist/index.es.js 2.75 kB
packages/navigation/dist/index.js 3.21 kB
packages/navigation/dist/style.css 874 B
packages/overlay/dist/index.es.js 1.02 kB
packages/overlay/dist/index.js 1.42 kB
packages/popover/dist/index.es.js 3.01 kB
packages/popover/dist/index.js 3.43 kB
packages/popover/dist/style.css 529 B
packages/portal/dist/index.es.js 420 B
packages/portal/dist/index.js 835 B
packages/table/dist/index.es.js 1.01 kB
packages/table/dist/index.js 1.44 kB
packages/table/dist/style.css 700 B
packages/tokens/dist/fonts.css 183 B
packages/tokens/dist/index.css 1.47 kB
packages/tokens/dist/index.es.js 3.07 kB
packages/tokens/dist/index.js 3.11 kB
packages/tokens/dist/media-queries.css 113 B
packages/tokens/dist/themes.css 2.27 kB
packages/tooltip/dist/index.es.js 598 B
packages/tooltip/dist/index.js 1.02 kB
packages/tooltip/dist/style.css 337 B
packages/vars/dist/index.es.js 2.66 kB
packages/vars/dist/index.js 2.66 kB

compressed-size-action

- Wrap icon and label elements in heading slot container
- Add conditional border radius styling when no description present
- Update RadioCardGroup story to use new heading slot structure
- Add test case without description to verify border radius behavior
- Use CSS Grid layout with heading and radio areas for proper alignment

Co-Authored-By: Naomi Hironaka <[email protected]>
@apucacao
Copy link
Contributor

After reading your PR description, variants came to mind, not slots. Did you consider that too and decided slots were more appropriate?

Copy link

Great question! I did consider variants vs slots and chose slots for several key reasons:

Flexibility & Composition: Slots allow for much more flexible content composition. With slots, the icon, label, and description can contain any React content (custom icons, formatted text, etc.), whereas variants would typically be more restrictive with predefined styling.

Existing Patterns: The codebase already uses slot-based styling patterns extensively (like in the Menu component), so this approach maintains consistency with established patterns.

User Requirements: The original task specifically requested slot-based styling with slot="card" to dictate the RadioCard component styles.

Content Structure: The heading slot wrapper allows for semantic grouping of icon+label while keeping description separate, which provides better layout control and accessibility structure.

Extensibility: Slots make it easier to add new content areas in the future without breaking changes to the API.

Variants would have been simpler for basic styling differences, but wouldn't provide the same level of compositional flexibility that this use case requires. The slot approach follows React Aria Components' patterns and gives consumers more control over their content structure.

What are your thoughts on this approach?

@nhironaka
Copy link
Contributor Author

(aside)

After reading your PR description, variants came to mind, not slots. Did you consider that too and decided slots were more appropriate?

@apucacao I remember Robb saying that recipes use a composition of different components so thought that approach was more appropriate, but I guess since Radio will only be used under RadioGroup a variant would also make sense!

I think we still may want to use slots for styling the labels/description/icons

thoughts?

@apucacao
Copy link
Contributor

(aside)

Yeah, I think slots are still useful for passing context down to certain props and to style certain descendants of each <Radio>. I was mostly thinking that variant sounded like how we've achieved these stylistic differences.

Do we ever want to have radios that look like cards and others that do no, within the same radio group? If not, should we move the prop to the group itself?

@nhironaka
Copy link
Contributor Author

(aside)

(aside)

Yeah, I think slots are still useful for passing context down to certain props and to style certain descendants of each <Radio>. I was mostly thinking that variant sounded like how we've achieved these stylistic differences.

Do we ever want to have radios that look like cards and others that do no, within the same radio group? If not, should we move the prop to the group itself?

So true, let me put devin to work

devin-ai-integration bot and others added 5 commits July 29, 2025 17:00
- Add variant support to RadioGroup component with card variant
- Update Radio component to consume variant context from RadioGroup
- Convert slot-based CSS selectors to variant classes in Radio.module.css
- Update RadioCardGroup story to use variant='card' prop instead of slot='card'
- Maintain existing heading slot structure for content composition
- Preserve conditional border radius and grid layout features

Co-Authored-By: Naomi Hironaka <[email protected]>
- Remove JavaScript context passing from RadioGroup and Radio components
- Add data-variant attribute to RadioGroup for CSS targeting
- Move card styling from Radio.module.css to RadioGroup.module.css using descendant selectors
- Maintain existing slot-based content structure and functionality
- Use pure CSS parent-to-child styling instead of Provider/useContext pattern

Co-Authored-By: Naomi Hironaka <[email protected]>
- Add subtitle slot styling to RadioGroup.module.css with secondary text color
- Update selected state styling to use primary color and medium font weight
- Update RadioCardGroup story to demonstrate subtitle slot usage
- Maintain CSS-only approach using descendant selectors for styling

Co-Authored-By: Naomi Hironaka <[email protected]>
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.

2 participants