-
-
Notifications
You must be signed in to change notification settings - Fork 751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: normalize the logos of sponsors #3534
base: master
Are you sure you want to change the base?
Conversation
Warning Rate limit exceeded@29deepanshutyagi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThe pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant Parent as Sponsors Component
participant SponsorImage as SponsorImage Component
Parent->>SponsorImage: Provide src, alt, className
SponsorImage-->>Parent: Render centered logo
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3534--asyncapi-website.netlify.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
components/sponsors/GoldSponsors.tsx (1)
3-3
: Consider sorting imports as recommended by lint rules.The linter warns about sorting imports. To adhere to project conventions and maintain a cleaner code structure, please consider applying the recommended sorting order.
🧰 Tools
🪛 eslint
[error] 1-3: Run autofix to sort these imports!
(simple-import-sort/imports)
components/sponsors/SponsorImage.tsx (1)
13-26
: Address ESLint/prettier formatting issues and trailing space warnings.
Some ESLint rules flag double quotes usage in JSX, trailing spaces, and missing a newline at the end of the file. You can fix them automatically using your project's lint or format command to keep code style consistent across the codebase.Below is a sample diff to address potential format warnings (assuming single quotes are preferred in JSX and a newline is required at end of file):
<div className="flex items-center justify-center w-full h-full"> - <img - src={src} - alt={alt} - className={twMerge( - 'max-h-9 sm:max-h-12 w-auto object-contain', - className - )} - /> + <img + src={src} + alt={alt} + className={twMerge('max-h-9 sm:max-h-12 w-auto object-contain', className)} + /> </div> -} \ No newline at end of file +} +🧰 Tools
🪛 eslint
[error] 15-15: Replace
"flex·items-center·justify-center·w-full·h-full"
with'flex·items-center·justify-center·w-full·h-full'
(prettier/prettier)
[error] 15-15: Unexpected usage of doublequote.
(jsx-quotes)
[error] 16-23: Replace
⏎········src={src}⏎········alt={alt}⏎········className={twMerge(⏎··········'max-h-9·sm:max-h-12·w-auto·object-contain',⏎··········className⏎········)}⏎·····
with·src={src}·alt={alt}·className={twMerge('max-h-9·sm:max-h-12·w-auto·object-contain',·className)}
(prettier/prettier)
[error] 26-26: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 26-26: Replace
·
with⏎
(prettier/prettier)
[error] 26-26: Newline required at end of file but not found.
(eol-last)
components/sponsors/SilverSponsors.tsx (1)
3-3
: Import statement is valid. Consider reordering imports to appease ESLint sorting rules.
If your project requires a particular import order, run your formatter or linter's autofix functionality to maintain consistency.🧰 Tools
🪛 eslint
[error] 1-3: Run autofix to sort these imports!
(simple-import-sort/imports)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/sponsors/GoldSponsors.tsx
(2 hunks)components/sponsors/SilverSponsors.tsx
(2 hunks)components/sponsors/SponsorImage.tsx
(1 hunks)
🧰 Additional context used
🪛 eslint
components/sponsors/GoldSponsors.tsx
[error] 1-3: Run autofix to sort these imports!
(simple-import-sort/imports)
components/sponsors/SilverSponsors.tsx
[error] 1-3: Run autofix to sort these imports!
(simple-import-sort/imports)
components/sponsors/SponsorImage.tsx
[error] 15-15: Replace "flex·items-center·justify-center·w-full·h-full"
with 'flex·items-center·justify-center·w-full·h-full'
(prettier/prettier)
[error] 15-15: Unexpected usage of doublequote.
(jsx-quotes)
[error] 16-23: Replace ⏎········src={src}⏎········alt={alt}⏎········className={twMerge(⏎··········'max-h-9·sm:max-h-12·w-auto·object-contain',⏎··········className⏎········)}⏎·····
with ·src={src}·alt={alt}·className={twMerge('max-h-9·sm:max-h-12·w-auto·object-contain',·className)}
(prettier/prettier)
[error] 26-26: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 26-26: Replace ·
with ⏎
(prettier/prettier)
[error] 26-26: Newline required at end of file but not found.
(eol-last)
🔇 Additional comments (4)
components/sponsors/GoldSponsors.tsx (1)
29-35
: New <SponsorImage>
usage looks solid.
Wrapping images in a reusable component effectively standardizes styling and ensures consistent behavior across the application. No issues found.
components/sponsors/SponsorImage.tsx (2)
1-2
: Imports look good, but consider following a consistent order.
The usage of React and twMerge
is correct. In some projects, there's a preference to sort imports alphabetically or by import type. Adhering to a consistent sorting convention can improve readability.
4-8
: Interface definitions appear clear and concise.
Declaring src
as required and alt
as optional is logical for sponsor images. The className
prop is also neatly integrated for flexibility.
components/sponsors/SilverSponsors.tsx (1)
29-32
: Great use of the new SponsorImage component!
The code neatly replaces the <img>
tag, improving consistency for sponsor logos. Make sure to confirm that the alt
and className
values align with your team’s standards for accessibility and styling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
components/sponsors/SilverSponsors.tsx (1)
Line range hint
6-9
: Fix props interface inconsistencyThe interface marks props as required but the implementation provides default values. Update the interface to match the implementation:
interface SilverSponsorsProps { - className: string; - showSupportBanner: boolean; + className?: string; + showSupportBanner?: boolean; }
🧹 Nitpick comments (1)
components/sponsors/SponsorImage.tsx (1)
19-28
: Consider performance optimizations for image loadingWhile the implementation correctly handles responsive design and aspect ratio, consider these performance improvements:
- Add
loading="lazy"
for better performance- Consider using Next.js
Image
component for automatic optimization<div className="flex size-full items-center justify-center"> <img src={src} alt={alt} + loading="lazy" className={twMerge( 'max-h-9 sm:max-h-12 w-auto object-contain', className, )} /> </div>
🧰 Tools
🪛 eslint
[error] 19-19: Replace
"flex·size-full·items-center·justify-center"
with'flex·size-full·items-center·justify-center'
(prettier/prettier)
[error] 19-19: Unexpected usage of doublequote.
(jsx-quotes)
[error] 20-27: Replace
⏎········src={src}⏎········alt={alt}⏎········className={twMerge(⏎··········'max-h-9·sm:max-h-12·w-auto·object-contain',⏎··········className,⏎········)}⏎·····
with·src={src}·alt={alt}·className={twMerge('max-h-9·sm:max-h-12·w-auto·object-contain',·className)}
(prettier/prettier)
[error] 25-25: Unexpected trailing comma.
(comma-dangle)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/sponsors/GoldSponsors.tsx
(2 hunks)components/sponsors/SilverSponsors.tsx
(2 hunks)components/sponsors/SponsorImage.tsx
(1 hunks)
🧰 Additional context used
🪛 eslint
components/sponsors/GoldSponsors.tsx
[error] 17-19: Replace ⏎··className·=·'',⏎
with ·className·=·''·
(prettier/prettier)
[error] 18-18: Unexpected trailing comma.
(comma-dangle)
[error] 22-22: Replace "mb-8·flex·flex-wrap·items-center·justify-center·md:px-4"
with 'mb-8·flex·flex-wrap·items-center·justify-center·md:px-4'
(prettier/prettier)
[error] 22-22: Unexpected usage of doublequote.
(jsx-quotes)
[error] 27-27: Replace "_blank"
with '_blank'
(prettier/prettier)
[error] 27-27: Unexpected usage of doublequote.
(jsx-quotes)
[error] 28-28: Replace "relative·block·w-2/3·p-4·text-center·sm:w-1/2·sm:p-0·md:w-1/3·lg:w-1/5"
with 'relative·block·w-2/3·p-4·text-center·sm:w-1/2·sm:p-0·md:w-1/3·lg:w-1/5'
(prettier/prettier)
[error] 28-28: Unexpected usage of doublequote.
(jsx-quotes)
[error] 29-29: Replace "noopener·noreferrer"
with 'noopener·noreferrer'
(prettier/prettier)
[error] 29-29: Unexpected usage of doublequote.
(jsx-quotes)
[error] 30-30: Replace "GoldSponsors-link"
with 'GoldSponsors-link'
(prettier/prettier)
[error] 30-30: Unexpected usage of doublequote.
(jsx-quotes)
[error] 36-36: Replace "GoldSponsors-img"
with 'GoldSponsors-img'
(prettier/prettier)
[error] 36-36: Unexpected usage of doublequote.
(jsx-quotes)
components/sponsors/SilverSponsors.tsx
[error] 17-19: Replace ⏎··className·=·'',⏎
with ·className·=·''·
(prettier/prettier)
[error] 18-18: Unexpected trailing comma.
(comma-dangle)
[error] 22-22: Replace "mb-8·flex·flex-wrap·items-center·justify-center·md:px-4"
with 'mb-8·flex·flex-wrap·items-center·justify-center·md:px-4'
(prettier/prettier)
[error] 22-22: Unexpected usage of doublequote.
(jsx-quotes)
[error] 27-27: Replace "_blank"
with '_blank'
(prettier/prettier)
[error] 27-27: Unexpected usage of doublequote.
(jsx-quotes)
[error] 28-28: Replace "relative·block·w-2/3·p-4·text-center·sm:w-1/2·md:w-1/3·lg:w-1/4"
with 'relative·block·w-2/3·p-4·text-center·sm:w-1/2·md:w-1/3·lg:w-1/4'
(prettier/prettier)
[error] 28-28: Unexpected usage of doublequote.
(jsx-quotes)
[error] 29-29: Replace "noopener·noreferrer"
with 'noopener·noreferrer'
(prettier/prettier)
[error] 29-29: Unexpected usage of doublequote.
(jsx-quotes)
[error] 30-30: Replace "SilverSponsors-link"
with 'SilverSponsors-link'
(prettier/prettier)
[error] 30-30: Unexpected usage of doublequote.
(jsx-quotes)
[error] 36-36: Replace "SilverSponsors-img"
with 'SilverSponsors-img'
(prettier/prettier)
[error] 36-36: Unexpected usage of doublequote.
(jsx-quotes)
components/sponsors/SponsorImage.tsx
[error] 13-17: Replace ⏎··src,⏎··alt·=·'Sponsor·logo',⏎··className,⏎
with ·src,·alt·=·'Sponsor·logo',·className·
(prettier/prettier)
[error] 16-16: Unexpected trailing comma.
(comma-dangle)
[error] 19-19: Replace "flex·size-full·items-center·justify-center"
with 'flex·size-full·items-center·justify-center'
(prettier/prettier)
[error] 19-19: Unexpected usage of doublequote.
(jsx-quotes)
[error] 20-27: Replace ⏎········src={src}⏎········alt={alt}⏎········className={twMerge(⏎··········'max-h-9·sm:max-h-12·w-auto·object-contain',⏎··········className,⏎········)}⏎·····
with ·src={src}·alt={alt}·className={twMerge('max-h-9·sm:max-h-12·w-auto·object-contain',·className)}
(prettier/prettier)
[error] 25-25: Unexpected trailing comma.
(comma-dangle)
🔇 Additional comments (3)
components/sponsors/SponsorImage.tsx (1)
4-8
: LGTM! Well-structured props interface.
The props interface is clear and properly typed with appropriate optional properties.
components/sponsors/GoldSponsors.tsx (1)
32-37
: LGTM! Proper implementation of SponsorImage component
The integration maintains all necessary attributes including testing hooks while standardizing the image display.
🧰 Tools
🪛 eslint
[error] 36-36: Replace "GoldSponsors-img"
with 'GoldSponsors-img'
(prettier/prettier)
[error] 36-36: Unexpected usage of doublequote.
(jsx-quotes)
components/sponsors/SilverSponsors.tsx (1)
32-37
: LGTM! Consistent implementation with GoldSponsors
The SponsorImage integration matches the pattern used in GoldSponsors, maintaining consistency across sponsor tiers.
🧰 Tools
🪛 eslint
[error] 36-36: Replace "SilverSponsors-img"
with 'SilverSponsors-img'
(prettier/prettier)
[error] 36-36: Unexpected usage of doublequote.
(jsx-quotes)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3534 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 19 19
Lines 668 668
=========================================
Hits 668 668 ☔ View full report in Codecov by Sentry. |
kindly review this pr , as no one had been go through it , since two weeks , @akshatnema and @derberg please review it, |
@devilkiller-ag Please review the PR once. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes looks GTM. @29deepanshutyagi Can you use the same SponsorImage
class for Platinum Sponsors also for consistency?
@akshatnema, Currently the |
@devilkiller-ag Yepp that makes sense. |
@29deepanshutyagi Add |
yeah sure, will soon commit the change |
kindly check it @devilkiller-ag and @akshatnema , |
Fixes issue #3158
I've made the following changes to normalize the sponsor logos:
Created a new SponsorImage component that:
Wraps the image in a flex container for consistent alignment
Uses max-h-9 (36px) for mobile and max-h-12 (48px) for larger screens
Maintains aspect ratio with w-auto and object-contain
Use the new SponsorImage component
Maintain existing layout and responsive behavior
These changes will ensure that:
All sponsor logos have consistent maximum heights
Logos maintain their aspect ratios
The layout remains responsive
The HDI logo will be properly constrained to match other sponsor logos
Summary by CodeRabbit
New Features
SponsorImage
component for consistent and centralized sponsor logo rendering.Refactor
<img>
tags with a reusableSponsorImage
component in sponsor sections.