[Brand Refactor] Card fixes - #1428
Conversation
🦋 Changeset detectedLatest commit: c1f60cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
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 |
🟢 No design token changes found |
🟢 No unit test coverage changes foundAll components and hooks with tests maintain the same coverage as the main branch. |
🟢 Bundle size report
|
🟢 No visual differences foundOur visual comparison tests did not find any differences in the UI. |
There was a problem hiding this comment.
Pull request overview
This PR updates the Card component’s responsive layout and interaction styling to address QA feedback: tighter spacing on narrow viewports, arrow CTA labels visible on non-hover devices, and removal of the hover scale effect while keeping arrow CTA motion.
Changes:
- Adjust
Cardpadding to be more compact on small screens, with desktop-specific inline padding. - Rework arrow CTA label/arrow-only styling to expose labels on non-hover devices while preserving hover/focus animations on hover-capable devices.
- Update Card Storybook examples to remove wrapper padding now that spacing is handled by the component, and add a changeset entry.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/Card/Cards.example.stories.tsx | Removes wrapper Box padding around Cards to avoid duplicate spacing in examples. |
| packages/react/src/Card/Card.module.css | Updates Card padding and arrow CTA hover/non-hover behavior; removes global hover scale effect. |
| packages/react/src/Card/Card.features.stories.tsx | Removes wrapper Box padding for stacked Cards in feature stories. |
| .changeset/tidy-cards-responsive.md | Documents the patch-level changes for release notes. |
Review details
- Files reviewed: 4/8 changed files
- Comments generated: 1
- Review effort level: Low
|
@danielguillan looks so much better. one thing to note: do we want to expose the links when hover interactions are not available? that's one of the recommendations i had, but how do you feel about that? |
|
@jesussandreas Yes, that's already implemented. Check this example on a phone or tablet and you should see the label by default. |
| justify-self: start; | ||
| } | ||
|
|
||
| @media (hover: hover) { |
There was a problem hiding this comment.
Really nice approach @danielguillan. Is there anything else worth/capable of hoisting into this progressive enhancement query? Maybe you've already done that. Just a nice opportunity to clean this up more.
There was a problem hiding this comment.
Good idea! There was more we could hoist. Updated.
rezrah
left a comment
There was a problem hiding this comment.
Works as expected. Love the opt-in for animation.
@danielguillan left some feedback around the interaction test.
Summary
Resolves https://github.com/orgs/github/projects/23995/views/5?pane=issue&itemId=209292904
Resolves https://github.com/orgs/github/projects/23995/views/5?pane=issue&itemId=196242231
Addresses 2 items of
CardQA feedback from the Brand Refactor board: responsive cards now match the intended compact layout and cards no longer scale on hover.List of notable changes:
What should reviewers focus on?
Steps to test:
Use the following Storybook links to verify the changes:
32pxblock /20pxinline padding,44px × 44pxicons,24pxicon spacing, and visible CTA labels.Supporting resources (related issues, external links, etc):
Contributor checklist:
update snapshotslabel to the PR)Reviewer checklist: