feat(ContentToc): add highlight-variant prop#5746
Conversation
commit: |
Co-authored-by: Benjamin Canac <canacb1@gmail.com>
Co-authored-by: Benjamin Canac <canacb1@gmail.com>
📝 WalkthroughWalkthroughAdds a new Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/runtime/components/content/ContentToc.vue (1)
136-146: Variable shadowing:linkHeightdefined twice with different values.The outer
linkHeight = 28(pixels, used for circuit mask) is shadowed by the innerconst linkHeight = 1.75(rem, used for indicator positioning). While functionally correct, this shadowing is confusing and could lead to maintenance issues.Additionally,
gapSizeon line 146 is defined but never used.♻️ Proposed fix to clarify variable naming
-const linkHeight = 28 +const linkHeightPx = 28 const indicatorStyle = computed(() => { if (!activeHeadings.value?.length) { return } const flatLinks = flattenLinks(props.links || []) const activeIndex = flatLinks.findIndex(link => activeHeadings.value.includes(link.id)) - const linkHeight = 1.75 // text-sm line-height (1.25rem) + py-1 (0.5rem) - const gapSize = 0 + const linkHeightRem = 1.75 // text-sm line-height (1.25rem) + py-1 (0.5rem) return { - '--indicator-size': `${linkHeight * activeHeadings.value.length}rem`, - '--indicator-position': activeIndex >= 0 ? `${activeIndex * linkHeight}rem` : '0rem' + '--indicator-size': `${linkHeightRem * activeHeadings.value.length}rem`, + '--indicator-position': activeIndex >= 0 ? `${activeIndex * linkHeightRem}rem` : '0rem' } })And update circuit mask references:
- const totalHeight = flatLinks.length * linkHeight + const totalHeight = flatLinks.length * linkHeightPx ... - const nextY = y + linkHeight + const nextY = y + linkHeightPx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/content/ContentToc.vue` around lines 136 - 146, The computed indicatorStyle currently shadows the top-level linkHeight by redeclaring const linkHeight = 1.75; rename the inner value to a clear, distinct name (e.g., indicatorLineHeightRem or indicatorLineHeight) and update all usages inside the indicatorStyle computed (including calculations for indicator positioning) so it no longer hides the top-level linkHeight used for the circuit mask; also remove the unused gapSize or use it where intended (or rename it if it has a planned purpose). Locate these changes around the indicatorStyle computed, activeHeadings, flattenLinks(props.links || []), and any circuit mask references that rely on the outer linkHeight and update those references to the outer symbol accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/2.components/content-toc.md`:
- Around line 157-198: The component-code block for the "Highlight Variant"
section is missing the externalTypes declaration; update the ::component-code
block (the one with props: highlightVariant: 'circuit' and links: [...] ) to
include externalTypes: - ContentTocLink[] so it matches the other sections
(e.g., the "Highlight" block that already defines externalTypes) and ensures the
ContentTocLink[] type is available for the links children.
---
Nitpick comments:
In `@src/runtime/components/content/ContentToc.vue`:
- Around line 136-146: The computed indicatorStyle currently shadows the
top-level linkHeight by redeclaring const linkHeight = 1.75; rename the inner
value to a clear, distinct name (e.g., indicatorLineHeightRem or
indicatorLineHeight) and update all usages inside the indicatorStyle computed
(including calculations for indicator positioning) so it no longer hides the
top-level linkHeight used for the circuit mask; also remove the unused gapSize
or use it where intended (or rename it if it has a planned purpose). Locate
these changes around the indicatorStyle computed, activeHeadings,
flattenLinks(props.links || []), and any circuit mask references that rely on
the outer linkHeight and update those references to the outer symbol
accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/components/content/__snapshots__/ContentToc.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
docs/app/pages/docs/[...slug].vuedocs/content/docs/2.components/content-toc.mdsrc/runtime/components/content/ContentToc.vuesrc/theme/content/content-toc.ts
highlight-variant prophighlight-variant prop
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/runtime/components/content/ContentToc.vue (1)
127-134: HardcodedlinkHeightis tightly coupled to CSS values.The
linkHeightconstant assumes fixed CSS measurements (text-sm + py-1). While the comment documents the calculation, any future CSS changes to link padding or font-size would silently break the indicator positioning.Consider extracting this as a configurable constant or CSS custom property if link sizing needs to vary.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/content/ContentToc.vue` around lines 127 - 134, The hardcoded linkHeight constant tightly couples indicator positioning to CSS; replace it with a configurable value (e.g., a prop or module-level config) or derive it from a CSS custom property at runtime so changes to styles won't break layout: remove the hardcoded linkHeight and instead expose a prop like linkHeightRem (defaulting to current 1.75) or read a CSS variable (e.g., --content-link-height) via getComputedStyle in the component's mounted hook, and update any uses of linkHeight to reference the new prop/derived value (refer to the linkHeight symbol and ensure any logic using flattenLinksWithLevel or the indicator positioning reads the new value).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/2.components/content-toc.md`:
- Around line 103-108: The TOC entries use camelCase IDs (highlightColor,
highlightVariant) which are inconsistent with other sections; update the id
values for these entries to kebab-case and quoted strings ('highlight-color' and
'highlight-variant') while leaving depth and text unchanged so anchor links
match the actual heading IDs.
---
Nitpick comments:
In `@src/runtime/components/content/ContentToc.vue`:
- Around line 127-134: The hardcoded linkHeight constant tightly couples
indicator positioning to CSS; replace it with a configurable value (e.g., a prop
or module-level config) or derive it from a CSS custom property at runtime so
changes to styles won't break layout: remove the hardcoded linkHeight and
instead expose a prop like linkHeightRem (defaulting to current 1.75) or read a
CSS variable (e.g., --content-link-height) via getComputedStyle in the
component's mounted hook, and update any uses of linkHeight to reference the new
prop/derived value (refer to the linkHeight symbol and ensure any logic using
flattenLinksWithLevel or the indicator positioning reads the new value).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09a5dd75-5728-4ecc-b195-3e7443ddeae2
📒 Files selected for processing (3)
docs/content/docs/2.components/content-toc.mdplaygrounds/nuxt/app/pages/components/content/content-toc.vuesrc/runtime/components/content/ContentToc.vue
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/components/content/ContentToc.spec.ts (1)
48-48: Consider a more specific type assertion.The
as anycast works but loses type safety. A more specific type would improve maintainability.♻️ Proposed refinement
- const highlightVariants = Object.keys(theme.variants.highlightVariant) as any + const highlightVariants = Object.keys(theme.variants.highlightVariant) as Array<keyof typeof theme.variants.highlightVariant>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/components/content/ContentToc.spec.ts` at line 48, Replace the broad "as any" cast on the highlightVariants declaration with a precise key-type cast to preserve type safety: use Object.keys(theme.variants.highlightVariant) typed as Array<keyof typeof theme.variants.highlightVariant> (or the equivalent keyof union) so highlightVariants has the correct string-literal key type instead of any; update the declaration referencing highlightVariants and theme.variants.highlightVariant accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/components/content/ContentToc.spec.ts`:
- Line 48: Replace the broad "as any" cast on the highlightVariants declaration
with a precise key-type cast to preserve type safety: use
Object.keys(theme.variants.highlightVariant) typed as Array<keyof typeof
theme.variants.highlightVariant> (or the equivalent keyof union) so
highlightVariants has the correct string-literal key type instead of any; update
the declaration referencing highlightVariants and
theme.variants.highlightVariant accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 44696e59-da95-4af5-8189-eb5b34886151
⛔ Files ignored due to path filters (1)
test/components/content/__snapshots__/ContentToc.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
docs/content/docs/2.components/content-toc.mdsrc/runtime/components/content/ContentToc.vuesrc/theme/content/content-toc.tstest/components/content/ContentToc.spec.ts
🔗 Linked issue
❓ Type of change
📚 Description
CleanShot.2025-12-23.at.12.28.04.mp4