fix(LearningCard): suppress <img> when cardImage is empty/missing#1588
Conversation
Empty banner values caused <img src=""> to render, which the browser
interprets as the current page URL, fails to decode as an image, and
shows a broken-image icon. This affected academy content cards in
Layer5 Cloud whenever a learning path lacked a banner asset.
- Gate <CardImage> on truthy cardImage in both branches
- Type cardImage as string | undefined (it was already optional in
practice; callers pass metadata?.banner which may be undefined)
- Add alt={title} to the active branch for consistency with the
disabled branch and basic a11y
- Add regression tests covering present/empty/undefined banner and
the disabled branch
Signed-off-by: miacycle <184569369+miacycle@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request makes the cardImage property optional on LearningCard and conditionally renders the <img> tag only when cardImage is present. This prevents broken-image icons from rendering when the image source is empty or undefined. Additionally, a new test suite has been added to verify this behavior. The review feedback suggests improving accessibility (a11y) by falling back to courseTitle for the image's alt attribute if title is empty or undefined.
There was a problem hiding this comment.
Pull request overview
This PR updates LearningCard to avoid rendering a broken decorative banner image when cardImage is missing or an empty string, aligning component behavior with real-world consumer data (e.g., academy cards without banners).
Changes:
- Make
cardImageoptional and conditionally render the<CardImage>/<img>only when a truthy banner URL is present. - Add/adjust
althandling on the banner image and keep behavior consistent across enabled/disabled (“Coming Soon”) branches. - Add Jest regression tests covering banner present vs empty/undefined banner, including the disabled branch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/custom/LearningCard/LearningCard.tsx | Makes cardImage optional and gates rendering of the <img> to prevent src="" broken-image behavior. |
| src/testing/LearningCard.test.tsx | Adds regression tests verifying <img> is rendered only when cardImage is provided and omitted for empty/undefined. |
| </CardImage> | ||
| {tutorial.frontmatter.cardImage ? ( | ||
| <CardImage> | ||
| <img src={tutorial.frontmatter.cardImage} alt={tutorial.frontmatter.title} /> |
| </CardImage> | ||
| {tutorial.frontmatter.cardImage ? ( | ||
| <CardImage> | ||
| <img src={tutorial.frontmatter.cardImage} alt={tutorial.frontmatter.title} /> |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lee Calcote <leecalcote@gmail.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Lee Calcote <leecalcote@gmail.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Lee Calcote <leecalcote@gmail.com>
Summary
LearningCardrendered<img src={cardImage} />unconditionally. WhencardImageis the empty string, the browser interpretssrc=""as the current page URL, fails to decode it as an image, and shows a broken-image icon in the decorative bottom-right area of every card without a banner.This surfaced in Layer5 Cloud's academy view: 12 of 19 published learning paths (
intro-*,workshop-*) ship withmetadata.banner = "", so every card without an explicit banner asset showed a broken icon.Fix
<CardImage>on truthycardImagein both the active and disabled (Coming Soon) branches - no<img>rendered when there is no banner.cardImageasstring | undefined. In practice consumers already passmetadata?.banner, which can beundefined; the type now matches usage.alt={title}to the active branch (was missing). Brings it in line with the disabled branch and gives screen readers a label.src/__testing__/LearningCard.test.tsxcovering:<img>rendered withsrcandaltcardImage: ""→ no<img>(this is the bug being fixed)cardImage: undefined→ no<img><img>Before/after
Before, with
cardImage: "":After: the
CardImagewrapper and its<img>are simply omitted.Compatibility
cardImagerender identically.stringtostring | undefinedis a relaxation; consumers that were already passingstringcontinue to type-check.Test plan
npx jest src/__testing__/LearningCard.test.tsx- 4/4 passnpx jest- 277/277 pass, 10 suitesnpx eslint src/custom/LearningCard/ src/__testing__/LearningCard.test.tsx- cleannpx prettier --check ...- cleanlayer5io/meshery-cloudafter this lands in a sistent release and the dep is bumped (broken icons in academy cards should disappear)