fix: add missing .logoImage CSS class in CNCFInfo component#330
fix: add missing .logoImage CSS class in CNCFInfo component#330shubhtrek wants to merge 1 commit into
Conversation
✅ Deploy Preview for kmesh-net ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request introduces a new .logoImage CSS class to the CNCFInfo styles module to manage image sizing and centering. The review feedback suggests a cleaner approach by letting the parent container handle the maximum width and centering, which simplifies the child image's styles and improves maintainability.
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.
| .logoImage { | ||
| width: 100%; | ||
| max-width: 400px; | ||
| height: auto; | ||
| display: block; | ||
| margin: 0 auto; | ||
| } No newline at end of file |
There was a problem hiding this comment.
There is a mismatch between the max-width of the parent container .cncfLogo (500px) and the child image .logoImage (400px). Because of this, margin: 0 auto is required on .logoImage to keep it centered within the parent when the viewport is between 400px and 500px.
A cleaner and more maintainable approach is to let the parent container .cncfLogo handle the sizing and centering, and keep the image styles simple. You can update .cncfLogo (on line 16) to max-width: 400px (or keep it at 500px if that was the intended size) and simplify .logoImage as suggested below. This also ensures there is a newline at the end of the file.
.logoImage {
width: 100%;
height: auto;
display: block;
}There was a problem hiding this comment.
Pull request overview
This PR fixes a missing CSS Module class referenced by the CNCFInfo component so the CNCF logo image is styled consistently and responsively on the homepage.
Changes:
- Adds the missing
.logoImageclass tosrc/components/CNCFInfo/styles.module.css. - Applies responsive sizing and centering rules to prevent the logo from rendering at default SVG dimensions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
711d2f5 to
9d08bed
Compare
9d08bed to
79c054d
Compare
The CNCFInfo component applies styles.logoImage to the CNCF logo img element, but the corresponding .logoImage class was not defined in styles.module.css. This caused the image to fall back to its default SVG dimensions instead of rendering with consistent sizing. Add the missing .logoImage class with responsive styling: - width: 100% for full container width - max-width: 400px to cap the logo size - height: auto to preserve aspect ratio - display: block + margin: 0 auto for centering Signed-off-by: Shubh Pingale <135618936+shubhtrek@users.noreply.github.com>
79c054d to
b42d718
Compare
| .logoImage { | ||
| width: 100%; | ||
| height: auto; | ||
| display: block; | ||
| } No newline at end of file |
Hey,
While going through the codebase I noticed that in the
CNCFInfocomponent,the
<img>tag for the CNCF logo hasclassName={styles.logoImage}appliedto it, but if you check
styles.module.css— the.logoImageclass issimply not there.
So basically the styling was being referenced but never defined, meaning the
logo just renders at whatever default size the SVG falls back to. Not great.
Added the class with some straightforward responsive styles:
width: 100%so it fills its containermax-width: 400pxto keep it from going too wideheight: autoto maintain aspect ratiodisplay: block+margin: 0 autoto center it properlyTested locally, logo looks fine on the homepage in both light and dark mode.
Closes #326