[LG-5747] feat: Card - update styles, deprecate clickability #3318
+71
−32
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 438ddea The changes in this PR will be included in the next version bump. This PR includes changesets to release 87 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 |
Contributor
|
Size Change: +1.61 kB (+0.09%) Total Size: 1.79 MB
ℹ️ View Unchanged
|
adamrasheed
requested review from
Copilot,
stephl3 and
tsck
and removed request for
a team
Copilot finished reviewing on behalf of
adamrasheed
November 18, 2025 22:54
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the Card component by simplifying its visual appearance and deprecating root-level clickability. The changes align with design system evolution toward cleaner, more focused component responsibilities.
Key changes:
- Updated border color from
gray.light2togray.light1for a subtler appearance - Removed base box shadows from both light and dark themes
- Deprecated
ContentStyleenum,contentStyleprop,onClick, andhrefwith JSDoc annotations
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/card/src/Card/types.ts | Added deprecation JSDoc comments to ContentStyle enum, contentStyle prop, and newly documented onClick/href props |
| packages/card/src/Card/styles.ts | Updated light theme border color, removed base box shadows, refactored styling logic into getCardStyles helper, and changed focus ring styles |
| packages/card/src/Card/Card.tsx | Simplified className generation by using the new getCardStyles helper function |
| packages/card/src/Card.stories.tsx | Removed deprecated ContentStyle import and related Storybook configuration |
| .changeset/vast-facts-press.md | Added changeset documenting the deprecation and style updates |
Comments suppressed due to low confidence (1)
packages/card/src/Card/styles.ts:71
- The focus box shadow should include the focus ring. Currently this only shows
${darkBaseBoxShadow}, but it should bebox-shadow: ${darkFocusBoxShadow}, ${darkBaseBoxShadow};to display the focus ring when the card is focused during hover/active states in dark theme.
box-shadow: ${darkBaseBoxShadow}, ${darkFocusBoxShadow};
.changeset/vast-facts-press.md
Outdated
| '@leafygreen-ui/card': minor | ||
| --- | ||
|
|
||
| deprecated clickable stlying and functionality, updated styles (removed shadows and added border) |
There was a problem hiding this comment.
Typo in the changeset description: "stlying" should be "styling".
Suggested change
| deprecated clickable stlying and functionality, updated styles (removed shadows and added border) | |
| deprecated clickable styling and functionality, updated styles (removed shadows and added border) |
.changeset/vast-facts-press.md
Outdated
| '@leafygreen-ui/card': minor | ||
| --- | ||
|
|
||
| deprecated clickable stlying and functionality, updated styles (removed shadows and added border) |
There was a problem hiding this comment.
The changeset description mentions "added border" but the changes actually update an existing border color from gray.light2 to gray.light1, not adding a new border. Consider revising to: "deprecated clickable styling and functionality, updated styles (removed shadows and updated border color)".
Suggested change
| deprecated clickable stlying and functionality, updated styles (removed shadows and added border) | |
| deprecated clickable styling and functionality, updated styles (removed shadows and updated border color) |
packages/card/src/Card/Card.tsx
Outdated
| }, | ||
| className, | ||
| )} | ||
| className={cx(getCardStyles({ theme, contentStyle, className }))} |
There was a problem hiding this comment.
The cx() call is redundant here since getCardStyles() already returns a result from cx(). You can simplify this to just className={getCardStyles({ theme, contentStyle, className })} without the outer cx() wrapper.
Suggested change
| className={cx(getCardStyles({ theme, contentStyle, className }))} | |
| className={getCardStyles({ theme, contentStyle, className })} |
| } from '@leafygreen-ui/polymorphic'; | ||
|
|
||
| /** | ||
| * @deprecated No longer supported. We don't want card to be clickable from a root level. |
There was a problem hiding this comment.
The deprecation message should specify what developers should use instead. Consider adding guidance like: "@deprecated No longer supported. We don't want card to be clickable from a root level. If you need clickable behavior, wrap interactive elements inside the Card instead."
Comment on lines
43
to
47
| * Click handler for the Card component. | ||
| * | ||
| * @deprecated No longer supported. We don't want card to be clickable from a root level. | ||
| */ | ||
| onClick?: React.MouseEventHandler<any>; |
There was a problem hiding this comment.
The JSDoc comment for the onClick handler is missing a description of what the parameter type is. Consider updating to: "Click handler for the Card component. Receives a MouseEvent." Also, using React.MouseEventHandler<any> with any is not type-safe - consider specifying the actual element type (e.g., HTMLDivElement) instead of any.
Suggested change
| * Click handler for the Card component. | |
| * | |
| * @deprecated No longer supported. We don't want card to be clickable from a root level. | |
| */ | |
| onClick?: React.MouseEventHandler<any>; | |
| * Click handler for the Card component. Receives a MouseEvent. | |
| * | |
| * @param event - The mouse event triggered by clicking the Card. | |
| * @deprecated No longer supported. We don't want card to be clickable from a root level. | |
| */ | |
| onClick?: React.MouseEventHandler<HTMLDivElement>; |
Contributor
|
Coverage after merging ar/LG-5747-card-border into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✍️ Proposed changes
This PR updates the Card component's visual styling by simplifying the border appearance and deprecates the
contentStyleprop. The changes remove box shadows from the light theme and update the border color to usepalette.gray.light1for a cleaner, more subtle look. ThecontentStyleprop is now deprecated as we no longer want the Card component to be clickable at the root level.Key changes:
gray.light2togray.light1ContentStyleenum andcontentStyleprop with appropriate JSDoc commentsgetCardStyleshelper function🎟️ Jira ticket: LG-5747
✅ Checklist
pnpm changesetand documented my changes🧪 How to test changes
gray.light1instead ofgray.light2)contentStyleprop shows deprecation warnings in TypeScript/IDE