Skip to content

feat: package header UI#2035

Merged
alexdln merged 29 commits intonpmx-dev:mainfrom
alexdln:feat/package-header-ui
Mar 12, 2026
Merged

feat: package header UI#2035
alexdln merged 29 commits intonpmx-dev:mainfrom
alexdln:feat/package-header-ui

Conversation

@alexdln
Copy link
Copy Markdown
Member

@alexdln alexdln commented Mar 11, 2026

🔗 Linked issue
Closes #2031 Closes #2033

🧭 Context
Updated UI in the package header - more stable, more versatile, more properly separated, and with other minor updates to improve usability. Now we can quickly select a version and switch between tabs on all package pages from header.

This is a reusable component that is used on all pages in the package. This further reduces the amount of code in the pages themselves

Updated the UI for the code view page - lines and symbols have moved down, the path has moved to a panel above the code and this panel is always displayed, and a scroll bar appears for long paths (I hope we will improve it further in next prs), removed internal code scrolling and improved stickiness

Updated the UI for the diff-view page - the version selector has been moved to the right, added shortcuts for diff and main

Also fixed a bug with the version-selector in diff-view - it basically didn't work
And a couple more bugs with layout on header, nav, breadcrumbs

in one train with #2030

There are still some details to improve (for example, in this discord thread). But for now, this PR is already large and stable. We can try to implement the rest in future iterations

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 11, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs.npmx.dev Ready Ready Preview, Comment Mar 12, 2026 9:06pm
npmx.dev Ready Ready Preview, Comment Mar 12, 2026 9:06pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
npmx-lunaria Ignored Ignored Mar 12, 2026 9:06pm

Request Review

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 11, 2026

Codecov Report

❌ Patch coverage is 47.15026% with 102 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/pages/package/[[org]]/[name].vue 26.85% 71 Missing and 8 partials ⚠️
app/components/Package/Header.vue 53.06% 17 Missing and 6 partials ⚠️

📢 Thoughts on this report? Let us know!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 11, 2026

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

File Note
i18n/locales/ar.json Localization changed, will be marked as complete.
i18n/locales/az-AZ.json Localization changed, will be marked as complete.
i18n/locales/bg-BG.json Localization changed, will be marked as complete.
i18n/locales/bn-IN.json Localization changed, will be marked as complete.
i18n/locales/cs-CZ.json Localization changed, will be marked as complete.
i18n/locales/de-DE.json Localization changed, will be marked as complete.
i18n/locales/en.json Source changed, localizations will be marked as outdated.
i18n/locales/es.json Localization changed, will be marked as complete.
i18n/locales/fr-FR.json Localization changed, will be marked as complete.
i18n/locales/hi-IN.json Localization changed, will be marked as complete.
i18n/locales/hu-HU.json Localization changed, will be marked as complete.
i18n/locales/id-ID.json Localization changed, will be marked as complete.
i18n/locales/it-IT.json Localization changed, will be marked as complete.
i18n/locales/ja-JP.json Localization changed, will be marked as complete.
i18n/locales/kn-IN.json Localization changed, will be marked as complete.
i18n/locales/mr-IN.json Localization changed, will be marked as complete.
i18n/locales/nb-NO.json Localization changed, will be marked as complete.
i18n/locales/ne-NP.json Localization changed, will be marked as complete.
i18n/locales/pl-PL.json Localization changed, will be marked as complete.
i18n/locales/pt-BR.json Localization changed, will be marked as complete.
i18n/locales/ru-RU.json Localization changed, will be marked as complete.
i18n/locales/ta-IN.json Localization changed, will be marked as complete.
i18n/locales/te-IN.json Localization changed, will be marked as complete.
i18n/locales/tr-TR.json Localization changed, will be marked as complete.
i18n/locales/uk-UA.json Localization changed, will be marked as complete.
i18n/locales/zh-CN.json Localization changed, will be marked as complete.
i18n/locales/zh-TW.json Localization changed, will be marked as complete.
Warnings reference
Icon Description
🔄️ The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
app/pages/package/[[org]]/[name].vue (1)

520-523: Misleading comment.

The comment states "includes file path if present", but this computed property doesn't include any file path logic. This comment appears to be copy-pasted from the code view page. Consider updating it to accurately describe this URL pattern for the main package page.

✏️ Suggested fix
-// URL pattern for version selector - includes file path if present
+// URL pattern for version selector
 const versionUrlPattern = computed(
   () => `/package/${pkg.value?.name || packageName.value}/v/{version}`,
 )

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fa7bacad-7947-4ccd-a764-4281272f9f08

📥 Commits

Reviewing files that changed from the base of the PR and between f09c274 and 8ce16a1.

📒 Files selected for processing (2)
  • app/pages/package-docs/[...path].vue
  • app/pages/package/[[org]]/[name].vue

@graphieros
Copy link
Copy Markdown
Contributor

I like a lot the resulting flexibility.
I have a single remark, that is probably just personal 'preferences' of mine, so to be taken with a grain of salt.

Package name hidden on scroll

For some reason not seeing the name of the package feels to me like it removes some quality to the page, like it suddenly anonymises the content. We need to conquer real estate on mobile, but probably a solution can be found to keep it visible on desktop ?

@graphieros
Copy link
Copy Markdown
Contributor

@graphieros Not only yours, and I generally agree too. There will just still be edge cases that need to be processed somehow 🫠

I know... 😬
Maybe halving the font size above a given number of chars ?
Anyway, the solution can be addressed later, as you noted in the PR description.

Copy link
Copy Markdown
Contributor

@graphieros graphieros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

@alexdln
Copy link
Copy Markdown
Member Author

alexdln commented Mar 12, 2026

@graphieros created an issue, hope we will find good option #2053

@alexdln alexdln added this pull request to the merge queue Mar 12, 2026
Merged via the queue into npmx-dev:main with commit 0d0707f Mar 12, 2026
22 checks passed
@alexdln alexdln deleted the feat/package-header-ui branch March 12, 2026 22:49
@github-actions github-actions bot mentioned this pull request Mar 12, 2026
@btea
Copy link
Copy Markdown
Contributor

btea commented Mar 17, 2026

@alexdln Hi, I noticed that this PR removed the feature added in #1686, which I personally think is very useful. However, I can't find a reason for its removal, or perhaps I've overlooked it?

@alexdln
Copy link
Copy Markdown
Member Author

alexdln commented Mar 17, 2026

@btea hi, I reused the logic that was on the main page on all pages - the scroll appears when needed in the package header

Like here - https://npmx.dev/package-code/nuxt/v/4.4.2/dist%2Fapp%2Fcomponents%2Fclient-fallback.server.js

@btea
Copy link
Copy Markdown
Contributor

btea commented Mar 18, 2026

@alexdln I'm sorry, I didn't quite understand what you meant.

the scroll appears when needed in the package header

Accessing the page via the link you provided above, I did not see a "Scroll to Top" button (used to quickly scroll the current file's code to the top). 😕

@alexdln
Copy link
Copy Markdown
Member Author

alexdln commented Mar 18, 2026

@btea sorry, here it is. It becomes visible after scrolling for a bit
image

@btea
Copy link
Copy Markdown
Contributor

btea commented Mar 18, 2026

image

This is very strange, I haven't been able to see the relevant button on my page.

@alexdln
Copy link
Copy Markdown
Member Author

alexdln commented Mar 18, 2026

Really weird...
Probably a bug with https://github.com/npmx-dev/npmx.dev/blob/main/app/components/ScrollToTop.client.vue. Maybe browser related...

@alexdln
Copy link
Copy Markdown
Member Author

alexdln commented Mar 18, 2026

Oh no, I think I found it - I'm not using this component (I dont know why) - I just used a button with the required logic...

The component has a fallback option in case the logic isn't supported, and apparently yours isn't. So button in header don't have fallback option and that's why you don't see anything... Probably

@btea
Copy link
Copy Markdown
Contributor

btea commented Mar 18, 2026

() => isTouchDeviceClient.value && scrollY.value > SCROLL_TO_TOP_THRESHOLD,

It seems to be a problem with this logic, which is causing the button not to be displayed in the PC browser. When I switched to mobile mode, the buttons were displayed correctly.

@alexdln
Copy link
Copy Markdown
Member Author

alexdln commented Mar 18, 2026

Oh... I should get used that when you have a touchscreen laptop, you can get similar artifacts. Thanks a lot for finding this 🤍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

version select overflows on mobile at the code page Improve available real estate on mobile

3 participants