-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(navbar): use twMerge for className merging #4813
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
Replaced template literal className concatenation with twMerge to ensure consistent Tailwind class conflict resolution and align with codebase conventions.
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughReplaced manual template-literal class composition with Changes
Sequence Diagram(s)(No sequence diagram generated: changes are localized to rendering and imports, not multi-component control flow.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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-4813--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: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/navigation/NavBar.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T14:21:28.216Z
Learnt from: sammy200-ui
Repo: asyncapi/website PR: 4804
File: components/navigation/Filter.tsx:32-41
Timestamp: 2025-12-29T14:21:28.216Z
Learning: In the asyncapi/website repository, when you intentionally omit dependencies in React hooks (e.g., useEffect, useMemo), add an eslint-disable comment with an explanatory note above the line to justify the design choice. For example: // eslint-disable-next-line react-hooks/exhaustive-deps: intentionally omitting dependencies to avoid unnecessary re-runs. This helps reviewers understand the rationale and keeps lint guidance informative.
Applied to files:
components/navigation/NavBar.tsx
🪛 GitHub Actions: PR testing - if Node project
components/navigation/NavBar.tsx
[error] 1-1: Run autofix to sort these imports! simple-import-sort/imports
[error] 22-22: tailwind-merge import should occur before import of ../AlgoliaSearch import/order
[error] 24-24: Prettier: Delete newline prettier/prettier
[error] 24-24: No-multiple-empty-lines: More than 1 blank line not allowed
[error] 149-149: Prettier: Delete whitespace character prettier/prettier
[error] 1-1: Run autofix to sort these imports! simple-import-sort/imports
⏰ 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). (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
|
|
||
| return ( | ||
| <div className={`bg-white ${className} z-50`}> | ||
| <div className={twMerge('bg-white z-50', className)}> |
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.
🧩 Analysis chain
🏁 Script executed:
# Find NavBar.tsx file
fd -e tsx -e ts "NavBar" | head -20Repository: asyncapi/website
Length of output: 92
🏁 Script executed:
# Read the NavBar.tsx file to examine line 149 and context
cat -n components/navigation/NavBar.tsx | sed -n '140,160p'Repository: asyncapi/website
Length of output: 807
🏁 Script executed:
# Search for NavBar component usage across the codebase
rg -nP --type=tsx --type=ts '<NavBar\s+' -g '!components/navigation/NavBar.tsx' -C2 | head -100Repository: asyncapi/website
Length of output: 87
🏁 Script executed:
# Check for prettier configuration in the repository
fd -e json -e js -e yml -e yaml "prettier" | head -10Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Check the import statements in NavBar.tsx to verify twMerge is properly imported
head -30 components/navigation/NavBar.tsxRepository: asyncapi/website
Length of output: 1121
🏁 Script executed:
# Find prettier config files
fd -type f \( -name ".prettierrc*" -o -name "prettier.config.*" \) | head -10Repository: asyncapi/website
Length of output: 230
🏁 Script executed:
# Check for prettier config in package.json
cat package.json | grep -A 20 '"prettier"' 2>/dev/null || echo "No prettier field found"Repository: asyncapi/website
Length of output: 723
🏁 Script executed:
# Search for NavBar usage with simpler pattern matching
rg 'NavBar' --type-list | head -5Repository: asyncapi/website
Length of output: 449
🏁 Script executed:
# Try searching for NavBar usage in JavaScript/TypeScript files
rg '<NavBar' -g '!NavBar.tsx' -C2 | head -80Repository: asyncapi/website
Length of output: 311
🏁 Script executed:
# Check the exact indentation at line 149 using od or hexdump
sed -n '149p' components/navigation/NavBar.tsx | od -c | head -20Repository: asyncapi/website
Length of output: 100
🏁 Script executed:
# Check prettier config file if it exists
cat .prettierrc .prettierrc.json .prettierrc.js 2>/dev/null | head -50Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Count exact spaces in line 149 using sed and printf
sed -n '149p' components/navigation/NavBar.tsx | sed 's/[^ ].*//' | wc -cRepository: asyncapi/website
Length of output: 61
🏁 Script executed:
# Count exact spaces in line 150 for comparison
sed -n '150p' components/navigation/NavBar.tsx | sed 's/[^ ].*//' | wc -cRepository: asyncapi/website
Length of output: 61
🏁 Script executed:
# Check lines 148-150 context with visible spaces
sed -n '148,150p' components/navigation/NavBar.tsx | cat -ARepository: asyncapi/website
Length of output: 232
Fix indentation to resolve prettier formatting error.
The twMerge usage is correct and properly merges the base Tailwind classes with passed className prop—verified against the actual usage in pages/_app.tsx which passes a className prop to NavBar.
Line 149 has incorrect indentation (5 spaces instead of 4). This is causing the prettier pipeline failure.
Fix
- <div className={twMerge('bg-white z-50', className)}>
+ <div className={twMerge('bg-white z-50', className)}>📝 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={twMerge('bg-white z-50', className)}> | |
| <div className={twMerge('bg-white z-50', className)}> |
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 149-149: Prettier: Delete whitespace character prettier/prettier
🤖 Prompt for AI Agents
In components/navigation/NavBar.tsx around line 149, the indentation of the line
containing <div className={twMerge('bg-white z-50', className)}> uses 5 spaces
instead of the project standard 4, causing Prettier to fail; fix by reducing the
leading spaces to 4 (align with surrounding JSX indentation) so the line matches
the file's indentation style and re-run Prettier to confirm the pipeline passes.
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
♻️ Duplicate comments (1)
components/navigation/NavBar.tsx (1)
152-152: Fix indentation (extra leading space).Line 152 has an extra leading space character causing prettier to fail. This was previously identified but the fix wasn't applied correctly.
🔎 Proposed fix
- <div className={twMerge('bg-white z-50', className)}> + <div className={twMerge('bg-white z-50', className)}>Reduce the leading spaces to match the surrounding JSX indentation (should be 4 spaces total).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/navigation/NavBar.tsx
🧰 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-12-29T14:21:28.216Z
Learnt from: sammy200-ui
Repo: asyncapi/website PR: 4804
File: components/navigation/Filter.tsx:32-41
Timestamp: 2025-12-29T14:21:28.216Z
Learning: In the asyncapi/website repository, when you intentionally omit dependencies in React hooks (e.g., useEffect, useMemo), add an eslint-disable comment with an explanatory note above the line to justify the design choice. For example: // eslint-disable-next-line react-hooks/exhaustive-deps: intentionally omitting dependencies to avoid unnecessary re-runs. This helps reviewers understand the rationale and keeps lint guidance informative.
Applied to files:
components/navigation/NavBar.tsx
🪛 GitHub Actions: PR testing - if Node project
components/navigation/NavBar.tsx
[error] 1-1: Run autofix to sort these imports! simple-import-sort/imports
[error] 11-11: Delete ⏎ prettier/prettier
[error] 11-11: More than 1 blank line not allowed. no-multiple-empty-lines
[error] 24-24: Delete ⏎⏎ prettier/prettier
[error] 26-26: More than 1 blank line not allowed. no-multiple-empty-lines
[error] 152-152: Delete · prettier/prettier
🔇 Additional comments (1)
components/navigation/NavBar.tsx (1)
152-152: LGTM: twMerge usage is correct and improves class composition.The switch from template literals to
twMergeproperly handles Tailwind CSS class conflicts and enables reliable className overrides. This aligns with the PR objectives and matches patterns used in other components (Button, TOC, Paragraph).Once the formatting issues are resolved, this change will improve the component's maintainability.
| import { twMerge } from 'tailwind-merge'; | ||
|
|
||
| import { defaultLanguage, i18nPaths, languages } from '@/utils/i18n'; | ||
|
|
||
|
|
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 import order and remove extra blank lines.
The pipeline is still failing due to formatting violations:
- Import sorting rules are not satisfied (line 1 error: "Run autofix to sort these imports!")
- Extra blank lines at line 11 violate the
no-multiple-empty-linesrule
🔎 Fix by running autofix
Run the project's autofix command to automatically resolve import sorting and formatting:
npm run formatOr manually ensure imports follow this order:
- External packages (React, Next.js, etc.)
- Blank line
- Internal absolute imports (@/utils/...)
- Blank line
- Internal relative imports (../, ./)
Remove extra blank lines at line 11.
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 11-11: Delete ⏎ prettier/prettier
[error] 11-11: More than 1 blank line not allowed. no-multiple-empty-lines
🤖 Prompt for AI Agents
In components/navigation/NavBar.tsx around lines 7 to 11, the imports are out of
order and there's an extra blank line causing lint/format failures; reorder
imports so external packages (e.g., tailwind-merge) come first, then a blank
line, then internal absolute imports (e.g., '@/utils/i18n'), remove the extra
blank line at line 11, and either run the project autofix (npm run format) or
apply the manual ordering described to satisfy the import-sorting and
no-multiple-empty-lines rules.
|
|
||
|
|
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.
Remove extra blank lines.
Lines 26-27 have excess blank lines causing prettier and linting failures. Only one blank line should separate the imports from the interface declaration.
🔎 Proposed fix
import ToolsPanel from './ToolsPanel';
-
-
interface NavBarProps {🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 26-26: More than 1 blank line not allowed. no-multiple-empty-lines
🤖 Prompt for AI Agents
In components/navigation/NavBar.tsx around lines 26 to 27, there are two
consecutive blank lines between the import block and the interface declaration;
remove the extra blank line so there is only a single blank line separating
imports from the interface to satisfy prettier and lint rules.
This PR updates NavBar.tsx to use
twMergefor className composition instead oftemplate literals.
Using
twMergeensures proper Tailwind CSS class conflict resolution and keepsthe component consistent with existing patterns used across the codebase
(e.g., Button, TOC, Paragraph components).
This improves maintainability and allows reliable className overrides.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.