fix(ui): Correct banner alignment on non-home pages (closes #4495) #4552
Conversation
raajpatre
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
We require all PRs to follow Conventional Commits specification. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
WalkthroughThe PR makes two changes: restructuring the className layout in the AnnouncementBanner component to adjust padding from py-6 to p-6 with mx-3 and reorder transition-related classes, and adding a trailing space in config/tools.json. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4552--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
components/campaigns/AnnouncementBanner.tsx(1 hunks)config/tools.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: asyncapi-bot
Repo: asyncapi/website PR: 0
File: :0-0
Timestamp: 2025-02-18T12:07:42.211Z
Learning: The following PR commands are supported in the asyncapi/website repository:
- `/please-take-a-look` or `/ptal`: Requests attention from reviewers who haven't reviewed the PR
- `/ready-to-merge` or `/rtm`: Triggers automerge when all conditions are met
- `/do-not-merge` or `/dnm`: Blocks automerge even if all conditions are met
- `/autoupdate` or `/au`: Adds autoupdate label to keep PR in sync with target branch
- `/update` or `/u`: One-time update of PR with latest changes from target branch
📚 Learning: 2025-01-19T04:51:41.255Z
Learnt from: anshgoyalevil
Repo: asyncapi/website PR: 3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
Applied to files:
config/tools.json
⏰ 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/AnnouncementBanner.tsx (1)
51-54: LGTM! Banner alignment fix looks correct.The changes successfully address the alignment inconsistency by applying
p-6andmx-3consistently regardless of thesmallprop, while preserving the intended vertical margin differences (mb-4vsmy-6). This ensures the banner has consistent horizontal spacing across all pages.
| "description": "Server API providing official AsyncAPI tools", | ||
| "links": { | ||
| "websiteUrl": "https://api.asyncapi.com/v1", | ||
| "websiteUrl": "https://api.asyncapi.com/v1", |
There was a problem hiding this comment.
Remove trailing whitespace.
There's a trailing space after the URL that appears unrelated to the banner fix. This should be removed to maintain clean formatting.
Apply this diff:
- "websiteUrl": "https://api.asyncapi.com/v1",
+ "websiteUrl": "https://api.asyncapi.com/v1",📝 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.
| "websiteUrl": "https://api.asyncapi.com/v1", | |
| "websiteUrl": "https://api.asyncapi.com/v1", |
🤖 Prompt for AI Agents
In config/tools.json around line 26, the "websiteUrl" entry contains a trailing
space after the URL; remove the extra space so the line ends with
"https://api.asyncapi.com/v1" (no trailing whitespace) and save the file to keep
formatting clean.
2 participants
Closes #4495
What this PR does
This PR fixes a visual bug where the announcement banner's alignment (padding and margin) was inconsistent on non-home pages (like /tools or /docs) compared to the homepage.
How it was fixed
The issue was in the components/campaigns/AnnouncmentBanner.tsx component.
The Tailwind CSS logic for the small prop was not applying the same horizontal margin (mx-3) and padding (p-6) as the main banner.
I updated the className logic to apply consistent p-6 and mx-3 padding/margins regardless of whether the small prop is true or false, while keeping the vertical margin (mb-4 vs my-6) logic.
How to test it:
Run the site locally (npm run dev).
Go to the homepage (/). Observe the banner's alignment.
Go to the Tools page (/tools).
Observe: The banner on /tools should now have the exact same padding and alignment as the banner on the homepage.
Summary by CodeRabbit