SC-51613 Remove card body min-height

[marciopmoreira6]

Summary

• Remove the size-driven min-height from Card.Body
• Keep Card.Body padding controlled by card size
• Add a regression test for Card.Body min-height

Test

• yarn jest src/components/card/tests/Card-test.tsx

---

Summary by Gitar

• Refactored component styles:
  • Removed min-height and max-height variables from CardHeaderStyled, RightContentStyled, and CardHeaderTitleButtonStyled
  • Updated CARD_HEADER_SIZE_STYLES to rely on padding-based sizing instead of explicit height variables
• Regression testing:
  • Added tests to verify min-height is unset for CardHeader, CardBody, and collapsible header buttons
  • Added unit tests to ensure CARD_HEADER_SIZE_STYLES and CARD_SIZE_STYLES only use padding configurations

_{This will update automatically on new commits.}

[netlify]

✅ Deploy Preview for echoes-react ready!

Name	Link
🔨 Latest commit	5e73bf4
🔍 Latest deploy log	https://app.netlify.com/projects/echoes-react/deploys/6a427727c44b4a000736086e
😎 Deploy Preview	https://deploy-preview-711--echoes-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[david-cho-lerat-sonarsource]

@marciopmoreira6 the review skill is saying this:

Must-fix: src/components/card/__tests__/Card-test.tsx:231

The consolidated does not apply a minimum height to the header or body test is still ineffective for the regression this PR is trying to guard against. Before this change, both removed implementations were min-height: var(--card-*-min-height), and in jsdom getComputedStyle(...).minHeight stays '' for unresolved var(...) values, so reintroducing the old rules would still keep this test green.

Suggested fix: assert on the style source of truth instead of computed style here, for example by verifying that CARD_HEADER_SIZE_STYLES / CARD_SIZE_STYLES no longer define the --card-*-min-height variables, or by asserting the emitted style rules on CardHeaderStyled / CardBodyStyled directly. Cost: small test-only follow-up, no runtime risk.

[marciopmoreira6]

Handled in 0bbac48. I removed the getComputedStyle(...).minHeight regression check and extended the source-of-truth style-map assertion to cover both CARD_HEADER_SIZE_STYLES and CARD_SIZE_STYLES. Reintroducing --card-header-min-height or --card-body-min-height in those sizing maps will now fail the test instead of relying on jsdom resolving CSS variables.

[marciopmoreira6]

Handled the two remaining Gitar suggestions in 5e73bf4: removed the unused size prop from CardBodyStyled/CardBody so it is not forwarded to the DOM, and renamed the internal noPadding flag to disablePadding to follow the boolean prop naming convention. I also added a Card body test assertion that the rendered body does not receive a size attribute.

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[david-cho-lerat-sonarsource]

if you confirm the 200 to 250 change was intentional then I have no further objections your honor

[gitar-bot]

Code Review ✅ Approved 6 resolved / 6 findings

Refactors Card sizing by removing min-height constraints in favor of padding-based layout, resolving all previous naming, testing, and CSS design token findings.

✅ 6 resolved

✅ Quality: New header heading-height values bypass cssVar design tokens

📄 src/components/card/CardStyles.tsx:110 📄 src/components/card/CardStyles.tsx:115 📄 src/components/card/CardStyles.tsx:120
The newly added --card-header-heading-height values are hardcoded raw rem literals (1.875rem, 1.5rem, 1.25rem) instead of being routed through the cssVar design-token helper, which the project conventions require for all CSS dimension values. Every other dimension in this file (paddings, gaps, radii) uses cssVar('dimension-...'). These raw values should be mapped to existing design tokens via cssVar(...). (The neighbouring --card-header-min-height values 58px/45px/36px are also raw literals, but they pre-date this change.)

✅ Quality: Header min-height regression test is ineffective in jsdom

📄 src/components/card/tests/Card-test.tsx:93-104
The new test 'does not apply a minimum height' asserts getComputedStyle(header).minHeight is ''. Before this change the header's min-height was set via min-height: var(--card-header-min-height). jsdom does not resolve CSS custom properties (var(...)), so getComputedStyle().minHeight returns '' whether or not that rule exists. The assertion therefore passes regardless of the regression it is meant to guard against — reintroducing min-height: var(--card-header-min-height) would NOT fail this test. (Contrast with the collapsible-button test at L283 which asserts the literal 'auto', a concrete value jsdom can return.)

Suggestion: assert against the actual styled-component output instead of computed style — e.g. use toHaveStyleRule('min-height', undefined) via jest-styled-components, or snapshot/inspect the emitted CSS for the header class. Alternatively remove the assertion's reliance on var() resolution so the test genuinely fails if a size-driven min-height returns.

✅ Quality: New tests call render() directly instead of a shared helper

📄 src/components/card/tests/Card-test.tsx:94-98 📄 src/components/card/tests/Card-test.tsx:275-281
Per the project test conventions, test files should define a local setup*/render* helper that wraps render() and returns { user, ...rest } so that userEvent.setup() wiring is consistent and tests are easier to refactor. The two newly added test cases call render() directly inside the it() blocks. While this matches the existing pattern in the file, the new additions continue to bypass the shared-helper convention. Consider introducing/using a renderCard() helper for the new cases (and ideally migrating the file).

✅ Bug: Test 'title is clicked' does not actually click the title

📄 src/components/card/tests/Card-test.tsx:309-323
The test toggles the body visibility when the title is clicked is described as clicking the title, but it actually clicks screen.getByRole('button', { name: 'Collapse' }) / 'Expand' — the same header toggle button exercised by the earlier toggles the body visibility when the header button is clicked test. As written it does not verify that clicking the title text triggers the toggle, so the intended behavior is untested and the test is effectively a duplicate. Target the title element explicitly (e.g. click the heading/title node) to cover the intended interaction.

✅ Quality: Unused size prop still declared/passed to CardBodyStyled

📄 src/components/card/CardStyles.tsx:49-57
After removing the size-driven min-height, CardBodyStyled no longer references props.size anywhere in its template, yet it still declares size: \${CardSize}`in its prop type (CardStyles.tsx:49) andCardBodystill passessize={size}(CardBody.tsx:41). The size is only consumed viaCARD_SIZE_STYLES[size]for--card-paddingin thecssprop, so the explicitsizeprop is now dead. Sincesizeis a valid DOM attribute, emotion will forward it to the underlying

rather than filtering it, leaving a straysizeattribute in the rendered HTML. Consider removingsizefrom theCardBodyStyledgeneric and thesize={size}` JSX to avoid dead code and an unintended DOM attribute.

...and 1 more resolved from earlier reviews

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}