-
-
Notifications
You must be signed in to change notification settings - Fork 1k
style: missing vertical spacing between navbar and announcement banner on tools, community, case studies, blog, and roadmap pages #4549 #4550
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
base: master
Are you sure you want to change the base?
Conversation
…r on Tools, Community, Case Studies, Blog, and Roadmap pages asyncapi#4549
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis PR adds vertical spacing to the AnnouncementHero component and applies styling classes to three card containers on the Roadmap page. Both changes are purely presentational, addressing visual spacing and design consistency without modifying any logic or data flow. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4550 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 780 780
Branches 144 144
=========================================
Hits 780 780 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4550--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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/campaigns/AnnouncementHero.tsx(1 hunks)pages/roadmap.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lighthouse CI
🔇 Additional comments (1)
components/campaigns/AnnouncementHero.tsx (1)
64-64: Spacing change verified across all affected pages.The
mt-4addition to the internal container div in AnnouncementHero is a component-level change that consistently applies to all affected pages. All five pages mentioned in the PR—Tools, Community, Case Studies, Blog, and Roadmap—useGenericLayout, which renders theAnnouncementHerocomponent. This design ensures uniform spacing without needing page-specific modifications.
| <div className='relative mt-20'> | ||
| <div className='grid lg:grid-cols-3 lg:gap-8'> | ||
| <div> | ||
| <div className='bg-white border border-gray-200 rounded-lg shadow-md p-6 flex flex-col justify-start lg:h-full'> |
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.
Clarify the relationship between goal card styling and PR objective.
These changes add visual styling (white background, border, shadow, padding, flex layout) to the three goal cards, which improves their presentation. However, the PR objective states the purpose is to fix "missing vertical spacing between navbar and announcement banner."
It's unclear how styling these goal cards relates to the navbar/announcement spacing issue. Consider whether these changes should be in a separate PR for better change tracking and clarity.
If these changes are addressing a separate styling concern on the Roadmap page, please update the PR description to reflect the additional scope. Otherwise, consider moving these changes to a dedicated PR.
Also applies to: 552-552, 575-575
🤖 Prompt for AI Agents
In pages/roadmap.tsx around lines 524, 552 and 575, the added card styling
(bg-white, border, shadow, p-6, flex, etc.) is unrelated to the stated PR
objective (fixing vertical spacing between navbar and announcement banner).
Either remove/revert these styling changes from this PR (restore the previous
classNames at those lines and move the styling edits into a new branch/PR), or
keep them but update the PR description to explicitly state the additional scope
(styling changes to the goal cards) and why they are included; implement one of
these two options and push the corresponding commit(s).
Fixes #4549

Summary by CodeRabbit