fix: package code page improvements#2217
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
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
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
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 (5)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a sticky Code Header component (breadcrumbs, copy/permalink, markdown view toggles, mobile-tree toggle, expand/collapse), a SkeletonLoader component for code loading state, and exposes a programmatic Possibly related PRs
Suggested labels
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: 5
🧹 Nitpick comments (1)
app/components/Code/Header.vue (1)
212-216: Drop the inlinefocus-visibleutility from these tab buttons.This override fights the shared button focus treatment the repo already applies globally.
Suggested fix
- class="px-2 py-1.5 font-mono text-xs rounded transition-colors duration-150 focus-visible:outline-accent/70 inline-flex items-center gap-1.5" + class="px-2 py-1.5 font-mono text-xs rounded transition-colors duration-150 inline-flex items-center gap-1.5"Based on learnings, focus-visible styling for buttons and selects is applied globally via main.css with
button:focus-visible, select:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; border-radius: 4px; }.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e125d40-6732-4e45-bc29-aec26bc64cb9
📒 Files selected for processing (9)
app/components/Code/DirectoryListing.vueapp/components/Code/Header.vueapp/components/Code/MobileTreeDrawer.vueapp/components/Code/SkeletonLoader.vueapp/components/Code/Viewer.vueapp/composables/useSettings.tsapp/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vuei18n/locales/en.jsonuno.config.ts
app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue
Show resolved
Hide resolved
app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue
Outdated
Show resolved
Hide resolved
|
(gonna mark as draft as you work on it, feel free to mark as ready when you want a review!) |
alex-key
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67cb48ea-c1f8-4b52-9336-96c0ac934347
📒 Files selected for processing (4)
app/components/Code/Header.vueapp/composables/useSettings.tsi18n/locales/en.jsontest/nuxt/a11y.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/Code/Header.vue
|
@alexdln appreciate a review from you |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
test/nuxt/a11y.spec.ts (1)
1163-1191: Add one loading-state a11y case forCodeHeader.This test covers the loaded file state well; adding a
loading: truescenario would better guard the loading UX changes in this PR.As per coding guidelines "Write unit tests for core functionality using
vitest."app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue (1)
336-336: Consider documenting the height calculation.The
h-[calc(100vh-10.5rem)]relies on a specific header height. If header dimensions change, this could break. A brief comment explaining the calculation basis (e.g.,/* 10.5rem = sticky header + tabs */) would aid future maintenance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 22686175-ed49-465f-a24a-b56f1fdc9dea
📒 Files selected for processing (32)
app/components/Code/Header.vueapp/components/Code/SkeletonLoader.vueapp/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vuei18n/locales/ar.jsoni18n/locales/az-AZ.jsoni18n/locales/bg-BG.jsoni18n/locales/bn-IN.jsoni18n/locales/cs-CZ.jsoni18n/locales/de-DE.jsoni18n/locales/en.jsoni18n/locales/es.jsoni18n/locales/fr-FR.jsoni18n/locales/hi-IN.jsoni18n/locales/hu-HU.jsoni18n/locales/id-ID.jsoni18n/locales/it-IT.jsoni18n/locales/ja-JP.jsoni18n/locales/kn-IN.jsoni18n/locales/mr-IN.jsoni18n/locales/nb-NO.jsoni18n/locales/ne-NP.jsoni18n/locales/pl-PL.jsoni18n/locales/pt-BR.jsoni18n/locales/ru-RU.jsoni18n/locales/ta-IN.jsoni18n/locales/te-IN.jsoni18n/locales/tr-TR.jsoni18n/locales/uk-UA.jsoni18n/locales/zh-CN.jsoni18n/locales/zh-TW.jsoni18n/schema.jsontest/nuxt/a11y.spec.ts
✅ Files skipped from review due to trivial changes (2)
- i18n/locales/en.json
- app/components/Code/SkeletonLoader.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/Code/Header.vue
|
@ghostdevv Why is it draft again? |
2 participants
🔗 Linked issue
Resolves #1942
🧭 Context
Code page has multiple minor issues (including mobile view). Also we want to align with existing Figma mocks.
📚 Description
Screenshots