fix: table of contents links not working #3560
Conversation
cs7-shrey
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe changes in the Changes
Assessment against linked issues
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
|
![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.
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3560--asyncapi-website.netlify.app/ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3560 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 19 19
Lines 668 668
=========================================
Hits 668 668 ☔ View full report in Codecov by Sentry. |
![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 (2)
components/TOC.tsx (2)
Line range hint
34-40: Enhance slug transformation for better compatibilityThe current slug transformation might not handle all edge cases correctly. Consider these improvements:
- Handle consecutive spaces to avoid multiple hyphens
- Add support for non-ASCII characters
- Ensure compatibility with manually added anchor tags mentioned in the comment
Consider this enhanced transformation:
slugWithATag: item.content - .replace(/[<>?!:`'."\\/=]/gi, '') - .replace(/\s/gi, '-') - .toLowerCase() + .normalize('NFKD') // Normalize Unicode characters + .replace(/[\u0300-\u036f]/g, '') // Remove diacritics + .replace(/[^a-zA-Z0-9\s-]/g, '') // Keep only alphanumeric and spaces + .trim() + .replace(/\s+/g, '-') // Replace one or more spaces with single hyphen + .toLowerCase()
Line range hint
34-93: Consider extracting slug transformation logicThe current implementation embeds the slug transformation logic directly in the component. For better maintainability and reusability, consider:
- Extracting this logic into a utility function
- Adding it to a shared utilities module
- Writing dedicated tests for the transformation logic
This would make it easier to maintain consistent slug generation across the application and improve test coverage.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/TOC.tsx(2 hunks)
🔇 Additional comments (2)
components/TOC.tsx (2)
78-78: LGTM! Consistent usage of transformed slugsThe change ensures that Scrollspy uses the same transformed slugs as the anchor links, maintaining consistency in navigation tracking.
88-88: Verify the fix for non-working TOC linksThe change correctly uses the transformed slugs in anchor links. Let's verify that this matches the actual heading IDs in the rendered content.
✅ Verification successful
Slug transformation correctly matches heading IDs
The change to use
slugWithATagis verified to be correct. The transformation logic follows standard markdown heading ID generation rules by removing special characters, converting spaces to hyphens, and making the text lowercase. This ensures the TOC links will correctly match the auto-generated heading IDs in the rendered content.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the heading IDs in MDX files match our transformed slugs # Find all MDX files echo "Checking heading patterns in MDX files..." rg -A 1 '^#{1,6}\s+.*' --type md # Check for manually added anchor tags in spec files echo "Checking for manual anchor tags in spec files..." rg '<a\s+name="[^"]*">' --type mdLength of output: 70547
Script:
#!/bin/bash # Search for slug transformation logic in the codebase echo "Searching for slug transformation code..." rg -A 5 "slugWithATag" --type ts echo "Searching for related slug utility functions..." rg -A 5 "slug" --type ts --glob '!**/node_modules/**' --glob '!**/dist/**'Length of output: 21610
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/TOC.tsx (2)
78-78: LGTM! Consider using nullish coalescing for clarity.The fallback logic for handling missing slugs is correct. However, you could make it more explicit using the nullish coalescing operator.
-items={tocItems.map((item) => (item.slug ? item.slug : item.slugWithATag))} +items={tocItems.map((item) => item.slug ?? item.slugWithATag)}
Line range hint
6-13: Update interface to reflect optional slug property.The interface should indicate that
slugmight be undefined since the implementation handles missing slugs.interface ITOCProps { className?: string; cssBreakingPoint?: string; toc: { lvl: number; content: string; - slug: string; + slug?: string; + slugWithATag?: string; }[]; contentSelector?: string; depth?: number; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/TOC.tsx(2 hunks)
🧰 Additional context used
🪛 eslint
components/TOC.tsx
[error] 88-88: Replace (item.slug·?·item.slug·:·item.slugWithATag) with item.slug·?·item.slug·:·item.slugWithATag
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
components/TOC.tsx
[error] 88-88: Prettier formatting error: Extra parentheses in expression '(item.slug ? item.slug : item.slugWithATag)'
components/TOC.tsx
Outdated
| @@ -85,7 +85,7 @@ export default function TOC({ className, cssBreakingPoint = 'xl', toc, contentSe | |||
| <a | |||
| className={`pl-${2 ** (item.lvl - 1)} font-normal mb-1 block font-sans text-sm | |||
| text-gray-900 antialiased transition duration-100 ease-in-out hover:underline`} | |||
| href={`#${item.slug}`} | |||
| href={`#${(item.slug ? item.slug : item.slugWithATag)}`} | |||
There was a problem hiding this comment.
Remove unnecessary parentheses to fix formatting.
The conditional logic is correct, but the extra parentheses are causing formatting issues.
-href={`#${(item.slug ? item.slug : item.slugWithATag)}`}
+href={`#${item.slug ? item.slug : item.slugWithATag}`}📝 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.
| href={`#${(item.slug ? item.slug : item.slugWithATag)}`} | |
| href={`#${item.slug ? item.slug : item.slugWithATag}`} |
🧰 Tools
🪛 eslint
[error] 88-88: Replace (item.slug·?·item.slug·:·item.slugWithATag) with item.slug·?·item.slug·:·item.slugWithATag
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[error] 88-88: Prettier formatting error: Extra parentheses in expression '(item.slug ? item.slug : item.slugWithATag)'
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Description
Related issue(s)
Fixes #3540
Summary by CodeRabbit