fix: #4241 added margin to the hero section so now it is not overlapping with navbar. and removed extra space. #4279
Conversation
Priyank0703
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. |
WalkthroughA CSS class was added to the main container of the Tools index page to introduce vertical spacing, addressing layout issues. Separately, a documentation link in a TypeScript configuration file comment was updated to reflect the latest Next.js documentation URL. No functional or structural code changes were made. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4279 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 778 778
Branches 144 144
=========================================
Hits 778 778 ☔ 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-4279--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: 0
🧹 Nitpick comments (1)
pages/tools/index.tsx (1)
19-22: Verify layout scope of the newmy-6wrapperAdding an extra wrapper solely for vertical margin works, but:
GenericLayoutalready controls page-level spacing in several views—introducing an outerdivmay create double margins in the future.- The underlying issue (#4241) concerns the hero section; this fix is applied to the Tools page instead. Double-check that the margin is required here and not more appropriately applied inside the hero component or
GenericLayout.If the margin is indeed global, consider adding it to
GenericLayoutor the hero component to keep page components lean and consistent.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
next-env.d.ts(1 hunks)pages/tools/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: asyncapi-bot
PR: asyncapi/website#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
Learnt from: akshatnema
PR: asyncapi/website#3262
File: components/Avatar.tsx:35-44
Timestamp: 2024-10-11T10:46:58.882Z
Learning: In Next.js, when avoiding nested `<a>` tags due to hydration issues, use accessible non-link elements like `<button>` or `<span>` with appropriate roles, `tabIndex`, and event handlers to maintain accessibility and SEO.
Learnt from: akshatnema
PR: asyncapi/website#3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:38:35.745Z
Learning: In Next.js, nested `<a>` tags cause hydration issues due to invalid HTML. To fix this, avoid nesting `<a>` tags by replacing inner `<a>` tags with non-interactive elements like `<span>`, and use click handlers to maintain interactivity if needed.
next-env.d.ts (5)
Learnt from: anshgoyalevil
PR: #3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: akshatnema
PR: #3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., require('../scripts/build-rss.ts')). This is enforced by the project's configuration which uses moduleResolution: "bundler" in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: anshgoyalevil
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.
Learnt from: akshatnema
PR: #3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., require('../scripts/build-rss.ts')). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: akshatnema
PR: #3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In JavaScript test files within the AsyncAPI website project, TypeScript file imports must include the .ts extension to avoid lint errors, even though the files being imported are JavaScript files.
⏰ 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). (2)
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Lighthouse CI
🔇 Additional comments (1)
next-env.d.ts (1)
4-6: Avoid modifying the auto-generatednext-env.d.tsThe header explicitly states that the file should not be edited. Any manual change (even a comment URL) will be overwritten the next time
next devornext buildregenerates this file, creating noisy diffs and potential merge conflicts.Move documentation-style comments to a project README or a dedicated docs file instead, and leave this file untouched.
Priyank0703
changed the title
|
|
||
| // NOTE: This file should not be edited | ||
| // see https://nextjs.org/docs/pages/building-your-application/configuring/typescript for more information. | ||
| // see https://nextjs.org/docs/pages/api-reference/config/typescript for more information. |
There was a problem hiding this comment.
why do we have this change?
There was a problem hiding this comment.
This is a by default change and it is redirecting to the same website ( both are working same )
and if you want old one I can do that and create a new PR for that as well .
There was a problem hiding this comment.
Yes please create a new one
There was a problem hiding this comment.
I had open another new PR for this as per your given changes.
3 participants
Description
fix: #4241
Summary by CodeRabbit
Style
Documentation