feat(ui): added word wrap to code viewer#2028
feat(ui): added word wrap to code viewer#2028stephansama wants to merge 1 commit intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
stephansama
changed the title
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThis pull request introduces a word wrap feature for the code viewer component. The Code Viewer component receives a new optional Suggested reviewers
🚥 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)
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.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29d80634-bca0-4cc4-8959-376aa22e6480
📒 Files selected for processing (4)
app/components/Code/Viewer.vueapp/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vuei18n/locales/en.jsoni18n/schema.json
| // Synchronize line number heights with code line heights (needed for word wrap) | ||
| function syncLineHeights() { | ||
| if (!props.wordWrap || !codeRef.value || !lineNumbersRef.value) { | ||
| // Reset heights if word wrap is disabled | ||
| if (lineNumbersRef.value) { | ||
| const nums = lineNumbersRef.value.querySelectorAll<HTMLElement>('.line-number') | ||
| nums.forEach(num => (num.style.height = '')) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| const lines = codeRef.value.querySelectorAll<HTMLElement>('code > .line') | ||
| const nums = lineNumbersRef.value.querySelectorAll<HTMLElement>('.line-number') | ||
|
|
||
| lines.forEach((line, index) => { | ||
| const num = nums[index] | ||
| if (num) { | ||
| // Use getBoundingClientRect for more precision if needed, but offsetHeight is usually enough | ||
| const height = line.offsetHeight | ||
| num.style.height = `${height}px` | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that markdown files default to preview mode, that CodeViewer is mounted with v-show,
# and that syncLineHeights currently only reruns on html/wordWrap/resize.
rg -n -C3 "const markdownViewMode|<CodeViewer|v-show=\"!fileContent.markdownHtml \\|\\| markdownViewMode === 'code'\"" "app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue"
rg -n -C3 "function syncLineHeights|offsetHeight|watch\\(|props.wordWrap|useEventListener\\(window, 'resize'" "app/components/Code/Viewer.vue"Repository: npmx-dev/npmx.dev
Length of output: 2129
Re-sync wrapped line heights after the viewer becomes visible.
The parent component mounts CodeViewer with v-show="!fileContent.markdownHtml || markdownViewMode === 'code'" and defaults to preview mode. For markdown files, this means the component is initially hidden (display: none), yet the immediate watcher on [props.selectedLines, props.html] fires during mount and calls syncLineHeights() while the viewer is not rendered. This causes offsetHeight to return 0, setting all line-number heights to 0px. The heights are never corrected when the user switches to code view with word wrap enabled, leaving the line-number column collapsed.
Add visibility detection to skip measurement while hidden, or use a ResizeObserver on the rendered code block to continuously maintain synchronisation.
🧰 Tools
🪛 Biome (2.4.6)
[error] 43-43: This callback passed to forEach() iterable method should not return a value.
(lint/suspicious/useIterableCallbackReturn)
| if (lineNumbersRef.value) { | ||
| const nums = lineNumbersRef.value.querySelectorAll<HTMLElement>('.line-number') | ||
| nums.forEach(num => (num.style.height = '')) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find app/components/Code -name "Viewer.vue" -type fRepository: npmx-dev/npmx.dev
Length of output: 91
🏁 Script executed:
cat -n app/components/Code/Viewer.vue | sed -n '35,50p'Repository: npmx-dev/npmx.dev
Length of output: 749
🏁 Script executed:
cat -n app/components/Code/Viewer.vue | head -50Repository: npmx-dev/npmx.dev
Length of output: 1935
🏁 Script executed:
# Check if there's a Biome configuration file and verify the lint rule
find . -maxdepth 2 -name "biome.json*" -o -name ".biomerc*" 2>/dev/null | head -5Repository: npmx-dev/npmx.dev
Length of output: 73
🏁 Script executed:
cat biome.jsonRepository: npmx-dev/npmx.dev
Length of output: 1835
🏁 Script executed:
# Let's verify the exact behavior of the useIterableCallbackReturn rule by checking if there are similar patterns in the file
rg "forEach" app/components/Code/Viewer.vue -A 1Repository: npmx-dev/npmx.dev
Length of output: 406
🏁 Script executed:
# Let's check if there are any other similar patterns in the codebase that might show how this is being handled elsewhere
rg "forEach.*=>.*=" --type ts --type tsx --type vue -B 1 -A 1 | head -30Repository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# Check the exact pattern and what Biome would flag
rg "forEach.*=>" --type vue --type ts -B 1 -A 1 | head -40Repository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# Check the exact pattern without type filters
rg "forEach.*=>" -B 1 -A 1 | head -50Repository: npmx-dev/npmx.dev
Length of output: 2777
🏁 Script executed:
# Let's see the exact context around lines 41-43 to understand the complete picture
cat -n app/components/Code/Viewer.vue | sed -n '38,46p'Repository: npmx-dev/npmx.dev
Length of output: 456
🏁 Script executed:
# Let's search for how similar patterns are handled elsewhere with block bodies
rg "forEach.*{" --max-count 20 | head -40Repository: npmx-dev/npmx.dev
Length of output: 144
🏁 Script executed:
# Let's look for how forEach is typically used in Vue components
rg "forEach" app/components/Code/Viewer.vue -B 1 -A 2Repository: npmx-dev/npmx.dev
Length of output: 750
Use a block body when clearing the line-number heights.
Line 43 returns the assignment value from the forEach callback, which Biome's lint/suspicious/useIterableCallbackReturn rule flags. A block body keeps the behaviour the same and resolves the lint error:
nums.forEach(num => {
num.style.height = ''
})
🧰 Tools
🪛 Biome (2.4.6)
[error] 43-43: This callback passed to forEach() iterable method should not return a value.
(lint/suspicious/useIterableCallbackReturn)
| <button | ||
| type="button" | ||
| class="px-2 py-1 font-mono text-xs text-fg-muted bg-bg-subtle border border-border rounded hover:text-fg hover:border-border-hover transition-colors items-center inline-flex gap-1" | ||
| :class="{ 'bg-accent/10 text-accent border-accent/20': wordWrap }" | ||
| @click="wordWrap = !wordWrap" | ||
| > | ||
| <span class="i-lucide:wrap-text w-3 h-3" /> | ||
| {{ $t('code.word_wrap') }} | ||
| </button> |
There was a problem hiding this comment.
Expose the wrap state on the toggle button.
This behaves as a toggle, but without aria-pressed assistive tech cannot tell whether wrapping is on or off. Please bind the pressed state here, for example with :aria-pressed="wordWrap".
|
Hi and thank you for this change! In my opinion, and for accessibility reasons (for example for people who don't use a mouse) I believe that it's always better to wrap code regardless just using CSS. For that reason I don't think we need a toggle and the wrapping can just be implemented using CSS. |
|
Perhaps we should even have a max line length in the code viewer like in many IDEs. |
i was trying to get it to work with css only however i was having a little issue. like it would mostly work but not all the way. i can take another crack at it again when i get out of work. 🫡 |
2 participants
🔗 Linked issue
resolves #2027
🧭 Context
scrolling horizontally is annoying when trying to just read code in my opinion.
📚 Description
I just added a small button in the toolbar to toggle word wrap and a class to enable and disable word wrap
I did use gemini to help with this PR however i implemented a similar feature on my personal blog on code blocks. just used the ai to quickly get up to speed