Skip to content

fix(LearningCard): prevent academy card overlap on wrapped titles#1609

Merged
leecalcote merged 2 commits into
masterfrom
claude/sleepy-tesla-F12mv
Jun 3, 2026
Merged

fix(LearningCard): prevent academy card overlap on wrapped titles#1609
leecalcote merged 2 commits into
masterfrom
claude/sleepy-tesla-F12mv

Conversation

@leecalcote
Copy link
Copy Markdown
Member

Description

Academy cards on /academy overlapped one another whenever a card title wrapped to two lines (e.g. DigitalOcean Certified AI Engineer (DO-CAIE) Exam).

Root cause

LearningCard's outer CardWrapper used a fixed height: 16rem, while its inner CardParent declares its own minHeight: 16rem and grows with content. When a title wrapped to a second line, the card body grew past 16rem and overflowed the fixed-height wrapper. In the consuming flex-wrap grid (LearningPathWrapper in meshery-cloud) the row height is driven by the wrapper, so the overflowing body bled into the card below — the overlap seen in the screenshot.

Fix

Switch CardWrapper from a fixed height to minHeight: 16rem so the wrapper expands to contain a multi-line title. Short cards keep the same baseline height; tall cards grow and the grid lays them out without overlap.

Changes

  • src/custom/LearningCard/style.tsxCardWrapper now uses minHeight instead of a fixed height.
  • src/__testing__/LearningCard.test.tsx — regression test asserting the wrapper grows for a long, wrapping title.

Testing

  • jest src/__testing__/LearningCard.test.tsx — 5/5 pass.
  • eslint clean on both changed files.

Downstream

The card is owned here in Sistent; meshery-cloud (ui/components/academy/contentList.tsx) consumes it via @sistent/sistent and will pick up the fix on the next package bump. No meshery-cloud source change is required.

The card wrapper used a fixed height of 16rem while the inner card body
has its own 16rem minHeight and grows with content. When a challenge or
learning-path title wrapped to two lines, the body overflowed the
fixed-height wrapper and overlapped the next card in the flex-wrap grid.

Switch the wrapper to minHeight so it expands to contain a multi-line
title. Add a regression test asserting the wrapper grows rather than
clipping to a fixed height.

Signed-off-by: Lee Calcote <leecalcote@gmail.com>
Copilot AI review requested due to automatic review settings June 3, 2026 19:20
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request changes the CardWrapper height from a fixed 16rem to a minHeight of 16rem to prevent wrapped titles from overflowing, and adds a regression test to verify this behavior. The reviewer noted that this change might cause layout issues where adjacent cards in a row have unequal visual heights, and suggested configuring CardWrapper as a flex container to ensure consistent stretching.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +8 to +12
// Use minHeight rather than a fixed height so the card grows to contain a
// title that wraps to multiple lines. A fixed height let the inner
// CardParent (which has its own minHeight) overflow the wrapper, causing
// the overflowing content to overlap the next card in the flex-wrap grid.
minHeight: '16rem',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While switching CardWrapper to minHeight allows the card to grow, it can introduce a layout issue in flexbox/grid containers (like the one in meshery-cloud). When one card in a row grows due to a wrapped title, the container stretches all other CardWrapper elements in that row to match the tallest card's height.

Since the intermediate children (CardActive, Card2, CardLink) and the inner CardParent do not have height-stretching properties, they will remain at their minimum height (16rem), resulting in unequal visual heights and empty space at the bottom of shorter cards.

To ensure all cards in a row stretch to equal heights, we should make CardWrapper a flex container and configure the inner elements to fill the height (e.g., using height: '100%' or flex: 1).

Suggested change
// Use minHeight rather than a fixed height so the card grows to contain a
// title that wraps to multiple lines. A fixed height let the inner
// CardParent (which has its own minHeight) overflow the wrapper, causing
// the overflowing content to overlap the next card in the flex-wrap grid.
minHeight: '16rem',
// Use minHeight rather than a fixed height so the card grows to contain a
// title that wraps to multiple lines. A fixed height let the inner
// CardParent (which has its own minHeight) overflow the wrapper, causing
// the overflowing content to overlap the next card in the flex-wrap grid.
minHeight: '16rem',
display: 'flex',
flexDirection: 'column',

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented — thanks, this was the right call. I checked the consumer: meshery-cloud's LearningPathWrapper is display: flex; flex-wrap: wrap with no align-items set, so it defaults to stretch — confirming the empty-space-below-shorter-cards issue you described.

Fix: CardWrapper is now a flex column, and flex: 1 + column layout is carried down the full chain (CardLink / CardActive / Card2CardParent) so the bordered card body fills the stretched wrapper and every card in a row renders at equal height. CardParent's minHeight: 16rem still holds the baseline when a row isn't stretched. Pushed in 5d52d9c.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes layout overlap for LearningCard instances in a wrapping flex grid by ensuring the outer wrapper can expand when titles wrap to multiple lines, preventing inner content from overflowing its container.

Changes:

  • Updated CardWrapper styling to use minHeight: 16rem instead of a fixed height: 16rem.
  • Added a regression test to ensure the wrapper is not constrained by a fixed height when rendering a long title.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/custom/LearningCard/style.tsx Replaces fixed wrapper height with minHeight so cards can grow with wrapped titles and avoid overlapping in flex-wrap layouts.
src/testing/LearningCard.test.tsx Adds a regression test intended to ensure the wrapper isn’t fixed-height for long/wrapping titles.

Comment thread src/__testing__/LearningCard.test.tsx Outdated
Comment on lines +86 to +90
const wrapper = container.firstChild as HTMLElement;
const computed = window.getComputedStyle(wrapper);
expect(computed.minHeight).toBe('16rem');
// No fixed height: a fixed height would re-clip a wrapped title.
expect(computed.height).toBe('');
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The assertion now accepts both 16rem and the normalized 256px for minHeight, and rather than checking height === '' it asserts the wrapper is not pinned to a fixed height (not '16rem' / not '256px') — robust to JSDOM reporting an unset height as '' or 'auto'. I also added coverage for the new flex-column stretch (display: flex, flexDirection: column). Pushed in 5d52d9c.

The minHeight fix stopped wrapped titles from overflowing, but the academy
grid lays cards out with the default `align-items: stretch`, so a row with a
taller (wrapped-title) card stretched every sibling CardWrapper while the
inner card body stayed at its content height — leaving empty space below the
shorter cards.

Make CardWrapper a flex column and carry `flex: 1` + column layout down the
chain (CardLink / CardActive / Card2 -> CardParent) so the bordered card body
fills the wrapper and all cards in a row render at equal height. CardParent's
minHeight keeps the baseline when a row isn't stretched.

Also harden the regression test: accept rem or normalized px for minHeight,
assert the wrapper isn't pinned to a fixed height (JSDOM reports unset height
as '' or 'auto'), and cover the new flex-column stretch.

Signed-off-by: Lee Calcote <leecalcote@gmail.com>
@leecalcote leecalcote merged commit 7533a7a into master Jun 3, 2026
5 checks passed
@leecalcote leecalcote deleted the claude/sleepy-tesla-F12mv branch June 3, 2026 23:18
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