-
-
Notifications
You must be signed in to change notification settings - Fork 697
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: bug#3504 improved the alignment and the gap #3506
Conversation
WalkthroughThe pull request introduces minor layout modifications to the Changes
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
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3506 +/- ##
=======================================
Coverage 86.59% 86.59%
=======================================
Files 21 21
Lines 664 664
=======================================
Hits 575 575
Misses 89 89 ☔ View full report in Codecov by Sentry. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3506--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: 1
🧹 Nitpick comments (1)
pages/tools/generator.tsx (1)
139-141
: Image layout improved, but alt text needed for accessibilityThe layout changes effectively address the alignment issues:
- Consistent width (80%) for both images
- Proper horizontal centering
- Appropriate vertical spacing
However, both images are missing meaningful alt text descriptions.
- <img className='relative mx-auto w-4/5 rounded shadow-lg' src='/img/tools/generator-1.png' alt='' /> - <img className='relative mx-auto w-4/5 mt-8 rounded shadow-lg' src='/img/tools/generator-2.png' alt='' /> + <img className='relative mx-auto w-4/5 rounded shadow-lg' src='/img/tools/generator-1.png' alt='AsyncAPI Generator interface screenshot' /> + <img className='relative mx-auto w-4/5 mt-8 rounded shadow-lg' src='/img/tools/generator-2.png' alt='AsyncAPI Generator output example' />
@@ -54,7 +54,7 @@ export default function GeneratorPage() { | |||
<Paragraph className='mt-4'>{description}</Paragraph> | |||
</div> | |||
|
|||
<div className='relative mt-12 lg:mt-20 lg:grid lg:grid-cols-2 lg:items-center lg:gap-8'> | |||
<div className='relative mt-12 lg:mt-20 lg:grid lg:grid-cols-2 lg:items-center lg:gap-8 lg:gap-x-24'> |
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.
💡 Codebase verification
Consider applying consistent gap spacing in modelina.tsx
The verification revealed that pages/tools/modelina.tsx
uses a similar two-column grid layout but with different horizontal spacing (lg:gap-8
without lg:gap-x-24
). For consistent UI appearance across tools pages, consider applying the same gap spacing to modelina.tsx.
pages/tools/modelina.tsx
: Updatelg:gap-8
to includelg:gap-x-24
🔗 Analysis chain
LGTM: Horizontal gap improvement implemented correctly
The addition of lg:gap-x-24
provides appropriate horizontal spacing between text and images on larger screens while maintaining responsive behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify no other instances of this layout pattern need similar gap adjustments
rg -l 'lg:grid-cols-2.*items-center' | xargs rg 'gap-[x]?-'
Length of output: 301
Script:
#!/bin/bash
# Let's check the full context of both files to compare their layouts
echo "=== generator.tsx ==="
rg -A 5 -B 5 'lg:grid-cols-2.*items-center' pages/tools/generator.tsx
echo -e "\n=== modelina.tsx ==="
rg -A 5 -B 5 'lg:grid-cols-2.*items-center' pages/tools/modelina.tsx
Length of output: 1236
@Mayaleeeee @akshatnema @derberg @magicmatatjahu kindly please review the Pull request |
Description
Screenshots
-Before
-After
Summary by CodeRabbit
New Features
Bug Fixes