feat: add word wrap toggle to code viewer#2207
feat: add word wrap toggle to code viewer#2207thealxlabs wants to merge 4 commits 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
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThis PR adds word-wrap support to the code viewer. The CodeViewer component gains an 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
i18n/locales/en.json (1)
1-2:⚠️ Potential issue | 🟡 MinorRegenerate the i18n schema file.
The
i18n/schema.jsonfile is stale and missing newly added keys (toggle_word_wrap,dependents, and others). Runpnpm run generate:i18n-schemato regenerate it fromi18n/locales/en.json.
🧹 Nitpick comments (3)
app/components/Code/Viewer.vue (1)
167-180: Remove duplicative/dead CSS selectors.Lines 167-173 define styles for
.code-content.word-wrap-activeand.code-content:has(.word-wrap), whilst lines 175-180 target.code-lines.word-wrap. Based on the template at line 125, only the.code-lineselement receives theword-wrapclass, so:
.code-content.word-wrap-activeis dead code (never applied).code-content:has(.word-wrap)and.code-lines.word-wrapare duplicative (both achieve the same result)Consider keeping only the direct selector for clarity.
♻️ Suggested simplification
-.code-content.word-wrap-active :deep(.line), -.code-content:has(.word-wrap) :deep(.line) { - white-space: pre-wrap; - overflow-wrap: break-word; - max-height: none; - overflow: visible; -} - .code-lines.word-wrap :deep(.line) { white-space: pre-wrap; overflow-wrap: break-word; max-height: none; overflow: visible; }test/nuxt/components/CodeViewer.spec.ts (2)
18-18: Weak assertion in basic rendering test.The OR condition
wrapper.find('pre').exists() || wrapper.html().includes('line')is always true becauseSAMPLE_HTMLcontains the string'line'. This test doesn't meaningfully verify that the component rendered correctly.♻️ Suggested improvement
- expect(wrapper.find('pre').exists() || wrapper.html().includes('line')).toBe(true) + // Verify the code-lines container exists and contains our sample HTML + const codeLines = wrapper.find('.code-lines') + expect(codeLines.exists()).toBe(true) + expect(codeLines.html()).toContain('const x = 1')
31-33: Prefer consistent assertion approach for class checking.The test at line 33 checks for
'word-wrap'in the raw HTML string, whilst the test at line 48 useswrapper.find('.code-lines').classes(). Using the same approach in both tests improves consistency and reliability.♻️ Suggested improvement for consistency
- const html = wrapper.html() - // When wordWrap is true, the code-lines div should have the 'word-wrap' class - expect(html).toContain('word-wrap') + const codeLines = wrapper.find('.code-lines') + // When wordWrap is true, the code-lines div should have the 'word-wrap' class + expect(codeLines.classes()).toContain('word-wrap')
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a03437c-19a6-460f-a9e3-03164a3ccfa5
📒 Files selected for processing (4)
app/components/Code/Viewer.vueapp/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vuei18n/locales/en.jsontest/nuxt/components/CodeViewer.spec.ts
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
knowler
left a comment
knowler
left a comment
There was a problem hiding this comment.
This approach doesn’t work because when the line wraps it’s adding new line numbers for the wrapped content causing the line numbers to become inaccurate for subsequent lines in a file. For a CSS solution, I think we’ll need to create some kind of grid layout where the line numbers are one column and the code is another, then when the code wraps it’ll cause the entire row to grow.
|
For this one, considering that there are two other PRs open on it that others have spent considerable effort on, let’s also put this on hold until I can check in with the others. |
|
Closing this PR. You've opened 14 PRs in a matter of minutes, which is a clear sign of automated bulk submissions with no genuine engagement. While AI tools can be helpful for coding assistance, our project requires genuine human involvement. We've outlined this clearly in our contribution guidelines. If you're genuinely interested in contributing to npmx.dev:
Mass-generated PRs like these won't be accepted. We welcome real contributions from developers who want to be part of our community. |
3 participants
🔗 Linked issue
resolves #2027
🧭 Context
The code viewer displayed long lines without any wrapping option, forcing horizontal scrolling. Users viewing files with very long lines (generated code, minified files, etc.) had no way to toggle word wrap.
📚 Description
wordWrapboolean prop toCode/Viewer.vuethat toggles aword-wrapCSS class on the code-lines container, enablingwhite-space: pre-wrapandoverflow-wrap: break-wordfor long lines.package-code/.../[...filePath].vue) in the toolbar, using anaria-pressedattribute to reflect the active state.code.toggle_word_wrapi18n key.A regression test (
test/nuxt/components/CodeViewer.spec.ts) verifies that theword-wrapclass is applied whenwordWrapis true and absent when false.