Conversation
|
azdanov
force-pushed
the
mobile-sponsors
branch
from
aa57122 to
46f5159
Compare
|
I'm updating the logos also as part of this PR to use svg for crisp images |
There was a problem hiding this comment.
Pull request overview
This pull request refactors the Sponsors component to fix logo overflow issues on mobile devices. The changes replace PNG sponsor logos with SVG versions and modernize the layout implementation using CSS Grid for better responsive behavior.
Changes:
- Replaced PNG logo files with SVG versions for CDN77 and Cloudflare (Framework already had SVG)
- Refactored component from repetitive HTML blocks to a data-driven array-based approach
- Changed layout from flexbox to CSS Grid with responsive columns (1 col mobile, 2 col tablet, 3 col desktop)
Reviewed changes
Copilot reviewed 1 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/components/widgets/Sponsors.astro | Refactored to use data array and CSS Grid layout; replaced PNG imports with SVG; simplified image properties |
| src/assets/images/cf_logo.svg | Added Cloudflare logo as SVG |
| src/assets/images/cdn77_logo.svg | Added CDN77 logo as SVG |
| src/assets/images/CF_logo_stacked_whitetype.png | Removed PNG version of Cloudflare logo |
| src/assets/images/CDN77_Horizontal_logo-negative.png | Removed PNG version of CDN77 logo |
Comments suppressed due to low confidence (1)
src/components/widgets/Sponsors.astro:35
- Consider adding an explicit
heightattribute alongsidewidthto prevent Cumulative Layout Shift (CLS) issues. While SVG images with viewBox attributes maintain aspect ratio when only width is specified, providing both dimensions helps the browser reserve the correct space before the image loads. Based on the viewBox values in the SVG files, appropriate heights would be: framework logo (~40px for 230px width), cdn77 logo (~67px), and Cloudflare logo (~76px).
width={230}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| loading="lazy" | ||
| decoding="async" | ||
| width={230} | ||
| layout="constrained" |
There was a problem hiding this comment.
The layout prop is not a valid property for the Image component from 'astro:assets' in Astro 5.x. This prop is only supported by the Picture component. Remove the layout="constrained" line. If you need to control the image sizing behavior, consider using the inferSize prop (which automatically infers dimensions from the source image) or explicitly provide both width and height attributes.
| layout="constrained" | |
| inferSize |
There was a problem hiding this comment.
1 participant
Fixes issue with logo overflowing the bounding box.