-
-
Notifications
You must be signed in to change notification settings - Fork 726
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: #3544 Misalignment of Height and Spacing in Case Study Block on Desktop and Mobile Views. #3561
Conversation
WalkthroughThe pull request modifies the Changes
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. |
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/CaseStudyCard.tsx (3)
22-22
: Reorder Tailwind classes for better maintainability.The class ordering should follow Tailwind's recommended pattern: layout → spacing → responsive variants.
- <div className='flex flex-wrap pt-10 lg:grid lg:grid-cols-3 lg:gap-8 lg:text-center gap-y-3'> + <div className='flex flex-wrap gap-y-3 pt-10 lg:grid lg:grid-cols-3 lg:gap-8 lg:text-center'>🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[warning] 22-22: Invalid Tailwind CSS classnames order
26-26
: Fix formatting issues and reorder Tailwind classes.The line contains trailing whitespace and needs class reordering.
- className='max-w-sm h-full overflow-hidden min-h-[300px] rounded-md border border-gray-200 bg-white p-4' + className='h-full min-h-[300px] max-w-sm overflow-hidden rounded-md border border-gray-200 bg-white p-4'🧰 Tools
🪛 eslint
[error] 26-26: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 26-26: Delete
·
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[warning] 26-26: Invalid Tailwind CSS classnames order
[error] 26-26: Trailing spaces not allowed
[error] 26-26: Delete trailing whitespace
Line range hint
23-36
: Consider accessibility and performance improvements.
- Using array index as key might cause issues with React's reconciliation if the studies array order changes.
- Add ARIA labels to improve accessibility for screen readers.
- Consider lazy loading images that might be below the fold.
- {studies.map((study, index) => ( + {studies.map((study) => ( - <a key={index} href={`casestudies/${study.id}`}> + <a + key={study.id} + href={`casestudies/${study.id}`} + aria-label={`Case study for ${study.company.name}`} + > <div className='h-full min-h-[300px] max-w-sm overflow-hidden rounded-md border border-gray-200 bg-white p-4' data-testid='CaseStudyCard-main' > <span className='mr-2'> - <img className='m-auto h-16' src={study.company.logo} alt={study.company.name} /> + <img + className='m-auto h-16' + src={study.company.logo} + alt={study.company.name} + loading="lazy" + />🧰 Tools
🪛 eslint
[error] 26-26: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 26-26: Delete
·
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[warning] 22-22: Invalid Tailwind CSS classnames order
[warning] 26-26: Invalid Tailwind CSS classnames order
[error] 26-26: Trailing spaces not allowed
[error] 26-26: Delete trailing whitespace
📜 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] 26-26: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 26-26: Delete ·
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
components/CaseStudyCard.tsx
[warning] 22-22: Invalid Tailwind CSS classnames order
[warning] 26-26: Invalid Tailwind CSS classnames order
[error] 26-26: Trailing spaces not allowed
[error] 26-26: Delete trailing whitespace
⏰ Context from checks skipped due to timeout of 180000ms (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
🔇 Additional comments (1)
components/CaseStudyCard.tsx (1)
Line range hint
22-36
: Implementation successfully addresses the height and spacing issues.The changes effectively solve the misalignment problems by:
- Adding vertical spacing between cards with
gap-y-3
- Ensuring consistent card heights with
h-full
andmin-h-[300px]
- Maintaining proper responsive behavior between mobile and desktop views
🧰 Tools
🪛 eslint
[error] 26-26: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 26-26: Delete
·
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[warning] 22-22: Invalid Tailwind CSS classnames order
[warning] 26-26: Invalid Tailwind CSS classnames order
[error] 26-26: Trailing spaces not allowed
[error] 26-26: Delete trailing whitespace
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3561--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)
components/CaseStudyCard.tsx (1)
22-22
: Optimize Tailwind class ordering for better maintainability.Consider grouping related Tailwind classes together following this pattern: layout → spacing → responsive modifiers.
- <div className='flex flex-wrap pt-10 lg:grid lg:grid-cols-3 lg:gap-8 lg:text-center gap-3'> + <div className='flex flex-wrap gap-3 pt-10 lg:grid lg:grid-cols-3 lg:gap-8 lg:text-center'>🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[warning] 22-22: Invalid Tailwind CSS classnames order
📜 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] 25-25: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 25-26: Replace ·⏎
with ⏎··
(prettier/prettier)
[error] 27-27: Replace data-testid='CaseStudyCard-main'
with ··data-testid='CaseStudyCard-main'⏎··········
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
components/CaseStudyCard.tsx
[warning] 22-22: Invalid Tailwind CSS classnames order
[error] 25-25: Trailing spaces not allowed
[error] 25-25: Replace spaces with proper indentation
[warning] 26-26: Invalid Tailwind CSS classnames order
[error] 27-27: Incorrect indentation for data-testid attribute
🔇 Additional comments (1)
components/CaseStudyCard.tsx (1)
25-27
: Height adjustments look good!The addition of
h-full
andmin-h-[300px]
classes effectively addresses the height misalignment issue mentioned in PR #3544.🧰 Tools
🪛 eslint
[error] 25-25: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 25-26: Replace
·⏎
with⏎··
(prettier/prettier)
[error] 27-27: Replace
data-testid='CaseStudyCard-main'
with··data-testid='CaseStudyCard-main'⏎··········
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[error] 25-25: Trailing spaces not allowed
[error] 25-25: Replace spaces with proper indentation
[warning] 26-26: Invalid Tailwind CSS classnames order
[error] 27-27: Incorrect indentation for data-testid attribute
components/CaseStudyCard.tsx
Outdated
<div | ||
className='max-w-sm h-full overflow-hidden min-h-[300px] rounded-md border border-gray-200 bg-white p-4' | ||
data-testid='CaseStudyCard-main'> |
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.
Fix formatting issues.
Address the following formatting issues:
- Remove trailing spaces
- Fix indentation
- <div
- className='max-w-sm h-full overflow-hidden min-h-[300px] rounded-md border border-gray-200 bg-white p-4'
- data-testid='CaseStudyCard-main'>
+ <div
+ className='max-w-sm h-full overflow-hidden min-h-[300px] rounded-md border border-gray-200 bg-white p-4'
+ data-testid='CaseStudyCard-main'
+ >
📝 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.
<div | |
className='max-w-sm h-full overflow-hidden min-h-[300px] rounded-md border border-gray-200 bg-white p-4' | |
data-testid='CaseStudyCard-main'> | |
<div | |
className='max-w-sm h-full overflow-hidden min-h-[300px] rounded-md border border-gray-200 bg-white p-4' | |
data-testid='CaseStudyCard-main' | |
> |
🧰 Tools
🪛 eslint
[error] 25-25: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 25-26: Replace ·⏎
with ⏎··
(prettier/prettier)
[error] 27-27: Replace data-testid='CaseStudyCard-main'
with ··data-testid='CaseStudyCard-main'⏎··········
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[error] 25-25: Trailing spaces not allowed
[error] 25-25: Replace spaces with proper indentation
[warning] 26-26: Invalid Tailwind CSS classnames order
[error] 27-27: Incorrect indentation for data-testid attribute
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.
Thanks @Sonalilipsa17375
/rtm |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3561 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 19 19
Lines 668 668
=========================================
Hits 668 668 ☔ View full report in Codecov by Sentry. |
Description
Mobile View -
Desktop View -
fixes #3544
Summary by CodeRabbit