fix(LearningCard): prevent academy card overlap on wrapped titles#1609
Conversation
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>
There was a problem hiding this comment.
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.
| // 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', |
There was a problem hiding this comment.
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).
| // 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', |
There was a problem hiding this comment.
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 / Card2 → CardParent) 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.
There was a problem hiding this comment.
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
CardWrapperstyling to useminHeight: 16reminstead of a fixedheight: 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. |
| 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(''); |
There was a problem hiding this comment.
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>
Description
Academy cards on
/academyoverlapped one another whenever a card title wrapped to two lines (e.g. DigitalOcean Certified AI Engineer (DO-CAIE) Exam).Root cause
LearningCard's outerCardWrapperused a fixedheight: 16rem, while its innerCardParentdeclares its ownminHeight: 16remand grows with content. When a title wrapped to a second line, the card body grew past16remand overflowed the fixed-height wrapper. In the consuming flex-wrap grid (LearningPathWrapperin 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
CardWrapperfrom a fixedheighttominHeight: 16remso 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.tsx—CardWrappernow usesminHeightinstead of a fixedheight.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.eslintclean on both changed files.Downstream
The card is owned here in Sistent; meshery-cloud (
ui/components/academy/contentList.tsx) consumes it via@sistent/sistentand will pick up the fix on the next package bump. No meshery-cloud source change is required.