-
-
Notifications
You must be signed in to change notification settings - Fork 696
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
changed columns from 3 to 2,added vertical spacing in mobile view #3511
changed columns from 3 to 2,added vertical spacing in mobile view #3511
Conversation
WalkthroughThe pull request modifies the Changes
Suggested labels
Suggested reviewers
Possibly related PRs
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-3511--asyncapi-website.netlify.app/ |
We require all PRs to follow Conventional Commits specification.
|
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: 2
🧹 Nitpick comments (1)
components/CaseStudyCard.tsx (1)
27-27
: Consider using Tailwind class for min-heightInstead of using inline style, consider using Tailwind's min-height utilities for better consistency and maintainability.
- style={{ minHeight: '300px' }} + className="min-h-[300px] flex flex-col items-center h-full overflow-hidden rounded-md border border-gray-200 bg-white p-4 transform transition-transform duration-300 hover:scale-105 hover:shadow-lg"🧰 Tools
🪛 eslint
[error] 27-27: Insert
····
(prettier/prettier)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/CaseStudyCard.tsx
(1 hunks)
🧰 Additional context used
🪛 eslint
components/CaseStudyCard.tsx
[error] 22-22: Replace "grid·gap-y-4·pt-10·sm:gap-y-6·lg:grid-cols-2·lg:gap-y-8·lg:justify-items-center·lg:gap-x-0"
with 'grid·gap-y-4·pt-10·sm:gap-y-6·lg:grid-cols-2·lg:gap-y-8·lg:justify-items-center·lg:gap-x-0'
(prettier/prettier)
[error] 22-22: Unexpected usage of doublequote.
(jsx-quotes)
[error] 23-23: Insert ····
(prettier/prettier)
[error] 24-24: Replace ····<a·key={index}·href={
casestudies/${study.id}}·className="w-full·max-w-sm"
with ········<a·key={index}·href={
casestudies/${study.id}}·className='w-full·max-w-sm'
(prettier/prettier)
[error] 24-24: Unexpected usage of doublequote.
(jsx-quotes)
[error] 25-25: Replace ······
with ··········
(prettier/prettier)
[error] 26-26: Replace ········className="flex·flex-col·items-center·h-full·overflow-hidden·rounded-md·border·border-gray-200·bg-white·p-4·transform·transition-transform·duration-300·hover:scale-105·hover:shadow-lg"
with ············className='flex·flex-col·items-center·h-full·overflow-hidden·rounded-md·border·border-gray-200·bg-white·p-4·transform·transition-transform·duration-300·hover:scale-105·hover:shadow-lg'
(prettier/prettier)
[error] 26-26: Unexpected usage of doublequote.
(jsx-quotes)
[error] 27-27: Insert ····
(prettier/prettier)
[error] 28-28: Replace data-testid="CaseStudyCard-main"
with ····data-testid='CaseStudyCard-main'
(prettier/prettier)
[error] 28-28: Unexpected usage of doublequote.
(jsx-quotes)
[error] 29-29: Replace ······
with ··········
(prettier/prettier)
[error] 30-30: Replace ········<span·className="mr-2">·
with ············<span·className='mr-2'>
(prettier/prettier)
[error] 30-30: Unexpected usage of doublequote.
(jsx-quotes)
[error] 30-30: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 31-31: Replace ··········<img·className="m-auto·h-16"
with ··············<img·className='m-auto·h-16'
(prettier/prettier)
[error] 31-31: Unexpected usage of doublequote.
(jsx-quotes)
[error] 32-32: Replace ········
with ············
(prettier/prettier)
[error] 33-33: Replace ········<Paragraph·typeStyle={ParagraphTypeStyle.md}·className="my-4·text-center"
with ············<Paragraph·typeStyle={ParagraphTypeStyle.md}·className='my-4·text-center'
(prettier/prettier)
[error] 33-33: Unexpected usage of doublequote.
(jsx-quotes)
[error] 34-34: Replace ··········
with ··············
(prettier/prettier)
[error] 35-35: Replace ········
with ············
(prettier/prettier)
[error] 36-36: Insert ····
(prettier/prettier)
[error] 37-37: Replace ····
with ········
(prettier/prettier)
[error] 38-38: Insert ····
(prettier/prettier)
[error] 39-40: Replace </div>⏎
with ····</div>
(prettier/prettier)
🔇 Additional comments (1)
components/CaseStudyCard.tsx (1)
22-22
: Grid layout implementation aligns with requirements
The transition to a 2-column grid layout with responsive spacing matches the PR objectives. The progressive gap scaling (gap-y-4 → gap-y-6 → gap-y-8) ensures proper vertical spacing across viewports.
Let's verify the mobile view spacing:
✅ Verification successful
Grid layout implementation is correct and consistent
The grid layout implementation in CaseStudyCard.tsx is properly implemented and aligns with both the requirements and the broader codebase patterns. The search results confirm:
- The component correctly implements a 2-column layout for larger screens (
lg:grid-cols-2
) - The progressive spacing (
gap-y-4 → gap-y-6 → gap-y-8
) follows similar responsive patterns seen elsewhere in the codebase - No conflicting grid styles are present in this component
- The implementation is consistent with other grid layouts in the codebase that transition from single to multi-column layouts at different breakpoints
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any other grid-related styles that might conflict
rg "grid-cols-[123]" --type tsx
Length of output: 61
Script:
#!/bin/bash
# Check for any other grid-related styles that might conflict, using correct file extension
rg "grid-cols-[123]" --type ts --type tsx --type jsx --type js
Length of output: 92
Script:
#!/bin/bash
# Search for grid-related styles without file type restrictions
rg "grid-cols-[123]"
# Also check for the specific component file
cat components/CaseStudyCard.tsx || true
Length of output: 5974
🧰 Tools
🪛 eslint
[error] 22-22: Replace "grid·gap-y-4·pt-10·sm:gap-y-6·lg:grid-cols-2·lg:gap-y-8·lg:justify-items-center·lg:gap-x-0"
with 'grid·gap-y-4·pt-10·sm:gap-y-6·lg:grid-cols-2·lg:gap-y-8·lg:justify-items-center·lg:gap-x-0'
(prettier/prettier)
[error] 22-22: Unexpected usage of doublequote.
(jsx-quotes)
<span className="mr-2"> | ||
<img className="m-auto h-16" src={study.company.logo} alt={study.company.name} /> | ||
</span> |
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.
🛠️ Refactor suggestion
Remove unnecessary span wrapper
The span with mr-2
class appears to serve no purpose as it's the only child in a centered flex container.
- <span className="mr-2">
- <img className="m-auto h-16" src={study.company.logo} alt={study.company.name} />
- </span>
+ <img className="m-auto h-16" src={study.company.logo} alt={study.company.name} />
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<span className="mr-2"> | |
<img className="m-auto h-16" src={study.company.logo} alt={study.company.name} /> | |
</span> | |
<img className="m-auto h-16" src={study.company.logo} alt={study.company.name} /> |
🧰 Tools
🪛 eslint
[error] 30-30: Replace ········<span·className="mr-2">·
with ············<span·className='mr-2'>
(prettier/prettier)
[error] 30-30: Unexpected usage of doublequote.
(jsx-quotes)
[error] 30-30: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 31-31: Replace ··········<img·className="m-auto·h-16"
with ··············<img·className='m-auto·h-16'
(prettier/prettier)
[error] 31-31: Unexpected usage of doublequote.
(jsx-quotes)
[error] 32-32: Replace ········
with ············
(prettier/prettier)
</div> | ||
<div className="grid gap-y-4 pt-10 sm:gap-y-6 lg:grid-cols-2 lg:gap-y-8 lg:justify-items-center lg:gap-x-0"> | ||
{studies.map((study, index) => ( | ||
<a key={index} href={`casestudies/${study.id}`} className="w-full max-w-sm"> |
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.
Avoid using array index as key prop
Using array indices as keys can lead to issues with component state and performance when the list items are reordered, removed, or inserted.
Consider using a unique identifier from the study object:
- <a key={index} href={`casestudies/${study.id}`} className="w-full max-w-sm">
+ <a key={study.id} href={`casestudies/${study.id}`} className="w-full max-w-sm">
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<a key={index} href={`casestudies/${study.id}`} className="w-full max-w-sm"> | |
<a key={study.id} href={`casestudies/${study.id}`} className="w-full max-w-sm"> |
🧰 Tools
🪛 eslint
[error] 24-24: Replace ····<a·key={index}·href={
casestudies/${study.id}}·className="w-full·max-w-sm"
with ········<a·key={index}·href={
casestudies/${study.id}}·className='w-full·max-w-sm'
(prettier/prettier)
[error] 24-24: Unexpected usage of doublequote.
(jsx-quotes)
@Sonalilipsa17375 Kindly file an issue and wait for a triage. More information about it is available in the Contributors Guide. |
Title:- Fixed card layout issues
Description
Before changes desktop view
Before changes mobile view
after changes in desktop view
after changes in mobile view
Testing Results:
Before:
Misaligned grid with 3 columns.
Inconsistent card heights.
No spacing between cards in mobile view.
After:
2-column grid layout.
Uniform card heights.
Adequate spacing in mobile view.
Enhanced hover effect for better interactivity
Summary by CodeRabbit
New Features
Bug Fixes
Style