Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
ActionCard’s new fitHeight behavior lacks public-component tests, and both fit-height examples use fixed pixel gaps instead of shared spacing tokens.
2 open issues
What changed in this PR
Adds an optional fitHeight property to ActionCard and ItemCard so consumers can align card heights within a row, while preserving the existing Cards component behavior.
Changes:
- Exposes
fitHeight, defaulting tofalse, through both public components and their styles. - Replaces ItemCard’s internal
fullHeightproperty and updates its caller and tests. - Adds layout demos and updates generated API documentation snapshots.
| File | Description |
|---|---|
src/item-card/styles.scss |
Renames the height class to fit-height. |
src/item-card/internal.tsx |
Applies the class through fitHeight. |
src/item-card/internal-interfaces.ts |
Removes the internal-only fullHeight property. |
src/item-card/interfaces.ts |
Declares and documents public fitHeight. |
src/item-card/index.tsx |
Defaults, records, and forwards fitHeight. |
src/item-card/__tests__/card.test.tsx |
Tests public fitHeight class application. |
src/cards/index.tsx |
Migrates the internal caller to fitHeight. |
src/action-card/styles.scss |
Adds full-height styling. |
src/action-card/internal.tsx |
Conditionally applies the height class. |
src/action-card/interfaces.ts |
Declares and documents public fitHeight. |
src/action-card/index.tsx |
Defaults, records, and forwards fitHeight. |
src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap |
Records both new public API properties. |
pages/item-card/fit-height.scss |
Styles the ItemCard layout demo. |
pages/item-card/fit-height.page.tsx |
Demonstrates layouts with optional footers. |
pages/action-card/fit-height.scss |
Styles the ActionCard layout demo. |
pages/action-card/fit-height.page.tsx |
Demonstrates height alignment across layouts. |
Review effort: Balanced
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5233 +/- ##
==========================================
+ Coverage 97.76% 97.86% +0.10%
==========================================
Files 1003 1016 +13
Lines 32572 32850 +278
Branches 12110 12179 +69
==========================================
+ Hits 31843 32149 +306
- Misses 684 695 +11
+ Partials 45 6 -39 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jperals
changed the title
fitHeight property to action card and item cardfitHeight prop to action card and item card
| */ | ||
| disableContentPaddings?: boolean; | ||
|
|
||
| /** |
There was a problem hiding this comment.
We should add a note about scroll behavior, just to differentiate it from container's fitHeight
| */ | ||
| disableFooterPaddings?: boolean; | ||
|
|
||
| /** |
There was a problem hiding this comment.
We should add a note about scroll behavior, just to differentiate it from container's fitHeight
| it.each([true])('applies full-height class when fullHeight=%s', fullHeight => { | ||
| const { itemCard } = renderInternalItemCard(<InternalItemCard fullHeight={fullHeight}>content</InternalItemCard>); | ||
| expect(itemCard.getElement()).toHaveClass(styles['full-height']); | ||
| describe('fitHeight', () => { |
There was a problem hiding this comment.
nit : The fitHeight block sits inside describe('internal props') and renders via renderItemCard. It's a public prop now, so it should move out of that block.
There was a problem hiding this comment.
Thanks for catching this. I removed the test, because I don't think it's really useful. Asserting the presence of a classname is testing internal implementation. This is better covered with visual regression tests. This PR adds the dev page and a later PR will provide the test definition (if both are added together the comparison with main fails).
GitHub Actions
Failure
Description
Doc:
xYEyp4yQGcBlHow has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.