fix: resolve broken TOC sidebar links for headings containing Markdown links#5530
Conversation
hasithasandunlakshan
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg,
princerajpoot20 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces heading-content normalization utilities and slug generation refinements for table-of-contents processing. A new ChangesTOC Heading Normalization and Slug Generation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
hasithasandunlakshan
changed the title
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5530--asyncapi-website.netlify.app/ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5530 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 23 23
Lines 931 976 +45
Branches 180 192 +12
=========================================
+ Hits 931 976 +45 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/TOC.tsx (1)
116-127: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRedundant character removal due to layered processing.
Line 118's character removal
.replace(/[<>?!:'."\/=@#$%^&*()[]{}+,;]/gi, '')` creates inconsistency:
- When
getSlugContentnormalizes content (has markdown links): it already applies/[^\w\s-]/gwhich removes most of these characters → line 118 is redundant- When
getSlugContentdoesn't normalize (no links): line 118 removes delimiters like<>but doesn't strip the HTML tag names → insufficient normalizationThis dual-layer processing pattern suggests the code handles two cases but does so inconsistently. Fixing the asymmetric behavior in
getSlugContent(previous comment) would make this processing more predictable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/TOC.tsx` around lines 116 - 127, The slug creation in slugWithATag is compensating for inconsistent behavior in getSlugContent: update getSlugContent to always perform full normalization (strip HTML tags, remove all non-word/space/hyphen characters using /[^\w\s-]/g, collapse whitespace to single hyphens, and toLowerCase) so it produces a canonical slug for both linked and plain content, then remove the redundant character-removal .replace(/[<>?!:`'."\\/=@#$%^&*()[\]{}+,;]/gi, '') usage inside slugWithATag (leaving only the whitespace-to-hyphen and seen suffix logic) to avoid asymmetry and duplication between getSlugContent and slugWithATag.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@components/TOC.tsx`:
- Around line 116-127: The slug creation in slugWithATag is compensating for
inconsistent behavior in getSlugContent: update getSlugContent to always perform
full normalization (strip HTML tags, remove all non-word/space/hyphen characters
using /[^\w\s-]/g, collapse whitespace to single hyphens, and toLowerCase) so it
produces a canonical slug for both linked and plain content, then remove the
redundant character-removal .replace(/[<>?!:`'."\\/=@#$%^&*()[\]{}+,;]/gi, '')
usage inside slugWithATag (leaving only the whitespace-to-hyphen and seen suffix
logic) to avoid asymmetry and duplication between getSlugContent and
slugWithATag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 70a8ca0d-e872-4c59-a6fc-e9d8e66dfbaa
📒 Files selected for processing (1)
components/TOC.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
components/TOC.tsx (1)
24-24: ⚡ Quick winSimplify redundant character class in trailing heading-ID regex.
Line 24 can be simplified:
\walready covers digits and_, so[\w\d\-_]is redundant and harder to read.Proposed cleanup
- .replace(/\s?\{#[\w\d\-_]+\}$/, '') + .replace(/\s?\{#[\w-]+\}$/, '')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/TOC.tsx` at line 24, The trailing heading-ID regex in the replace call is overly verbose; update the pattern used in the .replace call (the expression that currently reads .replace(/\s?\{#[\w\d\-_]+\}$/, '')) to use a simplified character class like [-\w] (e.g., /\s?\{#[-\w]+\}$/) so it still matches digits, letters, and underscores plus the hyphen but is clearer and less redundant.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@components/TOC.tsx`:
- Line 24: The trailing heading-ID regex in the replace call is overly verbose;
update the pattern used in the .replace call (the expression that currently
reads .replace(/\s?\{#[\w\d\-_]+\}$/, '')) to use a simplified character class
like [-\w] (e.g., /\s?\{#[-\w]+\}$/) so it still matches digits, letters, and
underscores plus the hyphen but is clearer and less redundant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0fa8770a-f753-47b3-b1a0-08bf7ade6118
📒 Files selected for processing (1)
components/TOC.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/build-post-list.ts (1)
52-84: 💤 Low valueCognitive complexity is acceptable for this parser.
SonarCloud flags this function's cognitive complexity (18 vs. 15 allowed). The manual character-by-character parsing is complex but correct and handles edge cases (incomplete patterns, images vs. links) appropriately. Refactoring to reduce complexity would require significant restructuring for minimal benefit, as the function logic is clear and self-contained.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/build-post-list.ts` around lines 52 - 84, SonarCloud flags stripMarkdownLinks for high cognitive complexity; replace the manual character-by-character parser with a simple regex-based approach to reduce complexity: in the stripMarkdownLinks function, first run a replace for image syntax (match /!\[([^\]]*)\]\((?:[^)]*)\)/) and substitute with group 1, then run a replace for regular links (match /\[([^\]]*)\]\((?:[^)]*)\)/) and substitute with group 1, and return the resulting string; keep the function name stripMarkdownLinks and ensure the regexes tolerate empty alt/text or missing URL characters to preserve behavior for incomplete patterns.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/build-post-list.ts`:
- Around line 52-84: SonarCloud flags stripMarkdownLinks for high cognitive
complexity; replace the manual character-by-character parser with a simple
regex-based approach to reduce complexity: in the stripMarkdownLinks function,
first run a replace for image syntax (match /!\[([^\]]*)\]\((?:[^)]*)\)/) and
substitute with group 1, then run a replace for regular links (match
/\[([^\]]*)\]\((?:[^)]*)\)/) and substitute with group 1, and return the
resulting string; keep the function name stripMarkdownLinks and ensure the
regexes tolerate empty alt/text or missing URL characters to preserve behavior
for incomplete patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c0cf5b48-3522-45dc-a878-de5b7c07aead
📒 Files selected for processing (3)
components/TOC.tsxscripts/build-post-list.tstests/build-post-list.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/build-post-list.ts (1)
90-100: 💤 Low valueConsider flipping the negated condition for readability.
SonarCloud flags the
if (!markdownLink)as an unexpected negated condition. Inverting the check puts the "action" case first, which is a common idiom.♻️ Proposed refactor
while (index < content.length) { const markdownLink = getMarkdownLinkAt(content, index); - if (!markdownLink) { - result += content[index]; - index += 1; - } else { + if (markdownLink) { result += markdownLink.label; index = markdownLink.nextIndex; + } else { + result += content[index]; + index += 1; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/build-post-list.ts` around lines 90 - 100, The loop currently checks a negated condition if (!markdownLink) making the fallback branch first; invert the condition to check if (markdownLink) first so the primary action (appending markdownLink.label and advancing index to markdownLink.nextIndex) is handled in the if branch, and the fallback (appending content[index] and incrementing index by 1) goes in the else branch; update the block around getMarkdownLinkAt, markdownLink, result, index, and content accordingly for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/build-post-list.ts`:
- Around line 90-100: The loop currently checks a negated condition if
(!markdownLink) making the fallback branch first; invert the condition to check
if (markdownLink) first so the primary action (appending markdownLink.label and
advancing index to markdownLink.nextIndex) is handled in the if branch, and the
fallback (appending content[index] and incrementing index by 1) goes in the else
branch; update the block around getMarkdownLinkAt, markdownLink, result, index,
and content accordingly for clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 98c0aadf-474a-42a3-98d9-61b2c04d074d
📒 Files selected for processing (1)
scripts/build-post-list.ts
|
@CodeRabbit give me a review. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== components/TOC.tsx ==="
cat components/TOC.tsx
echo ""
echo "=== scripts/build-post-list.ts ==="
cat scripts/build-post-list.ts
echo ""
echo "=== tests/build-post-list.test.ts (slugifyToC section) ==="
cat tests/build-post-list.test.tsLength of output: 35129 🏁 Script executed: #!/bin/bash
# Check the remark/rehype/markdown configuration for slug generation
rg -n "rehype-slug\|remark-slug\|github-slugger\|slugify" --type ts --type js --type json -g '!**/node_modules/**' -g '!**/.next/**' | head -60
echo "---"
# Check next.config.js/ts for remark plugins
fd "next.config" --type f | xargs cat 2>/dev/null
echo "---"
# Check for any MDX/remark configuration
fd "mdx\|remark\|rehype" --type f -g '*.{ts,js,mjs,cjs}' -g '!node_modules' | xargs cat 2>/dev/null | head -100Length of output: 2135
SummaryThe approach is sound: moving TOC content normalization to build time (in Issues Found1. 🐛 Indentation inconsistency in
|
|
@CodeRabbit give me a review. |
|
If you've pushed new commits since then and want a fresh incremental review, I'll trigger one now: ✅ Actions performedReview triggered.
|
@CodeRabbit I have fixed them.Take a look |
|
(ﾉ◕ヮ◕)ﾉ*:･ﾟ✧ ✅ Actions performedReview triggered.
|
|
@hasithasandunlakshan sorry..from your testing evidence, i did not get what's the issue and what's the fix that you have done. can you please try again explaining that |
|
Hi @princerajpoot20 I updated the description video.Basically issue was https://www.asyncapi.com/docs/community/050-mentorship-program/asyncapi-mentorship-2024-project-ideas#10-performance--accessibility-improvement-of-websitehttpsgithubcomasyncapiwebsiteissues3186 checked fixed version - https://deploy-preview-5530--asyncapi-website.netlify.app/docs/community/050-mentorship-program/asyncapi-mentorship-2024-project-ideas#asyncapi-mentorship-program-2024 |
|
@hasithasandunlakshan oh okay. got it. Since these changes also affect the "On this page" section across all our docs, can you do a sanity check on the other docs as well to ensure everything is working as expected there? Please attach evidence of that as part of this effort. This is to make sure we don't introduce any new regressions. |
hasithasandunlakshan
force-pushed
the
fix/scrolling-issue-in-right-sidebar
branch
from
6a1463b to
d947bc5
Compare
|
3 participants
Description
Related issue(s)
#5465 (comment)
Issue 17
Screen.Recording.2026-06-02.at.22.30.43.mov
Screen.Recording.2026-06-01.at.17.05.34.mov
Reprodcue
Summary by CodeRabbit
Chores
Tests