fix: twoslash popup rendering and styling#2274
Conversation
Twoslash popups render through MDC, so the embedded code/pre nodes were mapped to Nuxt UI Prose components (bordered code blocks with a copy button). Reset those styles so popups show plain highlighted code, drop the prose classMarkdown, patch nuxt-content-twoslash to resolve config globals like defineNuxtConfig, and enable twoslash in dev.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adjusts Twoslash code popup rendering and type resolution. It adds a TypeScript reference directive patch to the Twoslash transformer to resolve Nuxt types correctly, registers the patch in workspace dependencies, updates the base code font size from Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/assets/css/twoslash.css (1)
56-59: 💤 Low valueConsider adding a comment to explain the line-height calculation.
The
calc(1.25 / 0.875)expression (≈1.4286) works correctly, but a brief comment explaining why this specific calculation is needed would improve maintainability.For example:
/* Maintain consistent line-height ratio adjusted for the reduced font size */ line-height: calc(1.25 / 0.875);🤖 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 `@app/assets/css/twoslash.css` around lines 56 - 59, Add a brief explanatory comment above the .twoslash-popup-docs p rule explaining why line-height uses calc(1.25 / 0.875) (e.g., that it maintains a consistent line-height ratio when font-size is reduced to 0.875) so future maintainers understand the rationale; update the comment near the .twoslash-popup-docs p selector and keep it concise and specific to the adjustment.
🤖 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 `@app/assets/css/twoslash.css`:
- Around line 56-59: Add a brief explanatory comment above the
.twoslash-popup-docs p rule explaining why line-height uses calc(1.25 / 0.875)
(e.g., that it maintains a consistent line-height ratio when font-size is
reduced to 0.875) so future maintainers understand the rationale; update the
comment near the .twoslash-popup-docs p selector and keep it concise and
specific to the adjustment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8e8f133-b1fb-4aee-8a18-e2777ead39b2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
app/assets/css/twoslash.cssnuxt.config.tspatches/nuxt-content-twoslash@0.4.0.patchpnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- nuxt.config.ts
| ) { | ||
| const prepend = [ | ||
| + // `nuxt.d.ts` (app context) no longer references the `nuxt` package types since | ||
| + // Nuxt's type-context split, so config-level globals like `defineNuxtConfig` are |
There was a problem hiding this comment.
this shouldn't be needed. instead we should set the nuxt.config.ts file name for any code examples that use defineNuxtConfig, so the node types are pulled in
which ones were causing an issue?
There was a problem hiding this comment.
Well all code-blocks, on nuxt.com twoslash is not working at all at the moment and has doubled the build time. So we either need to fix it or disable it.
I don't think this patch should be merged, this PR is an experiment to fix twoslash upstream. I should have marked it as draft.
There was a problem hiding this comment.
thank you for diving into this ❤️
There was a problem hiding this comment.
What should we do then? Should I apply this patch on nuxt-content-twoslash or you don't think it's correct?
2 participants
Summary
Twoslash popups are rendered through MDC, so the
code/prenodes they embed get mapped to Nuxt UI's Prose components (ProseCodeInline/ProsePre). That wrapped popups in bordered, padded code blocks with a copy button instead of plain highlighted code..twoslash-popup-container(margins, borders, padding,bg-muted, copy button) so popups show plain highlighted code, for both the inline and multi-line (pre) type cases as well as inline code in the docs sectionnuxt-content-twoslashso config-level globals likedefineNuxtConfigresolve in untagged config snippets (Nuxt's type-context split meansnuxt.d.tsno longer references thenuxtpackage types)Test plan
twoslashcode block, confirm the popup shows plain highlighted code with no border, copy button, or extra padding