Skip to content

fix(ui): file tree scroll position is stable when browsing#2288

Open
jhroemer wants to merge 4 commits intonpmx-dev:mainfrom
jhroemer:fix/code-file-tree-stable-position-when-browsing
Open

fix(ui): file tree scroll position is stable when browsing#2288
jhroemer wants to merge 4 commits intonpmx-dev:mainfrom
jhroemer:fix/code-file-tree-stable-position-when-browsing

Conversation

@jhroemer
Copy link
Copy Markdown
Contributor

@jhroemer jhroemer commented Mar 26, 2026

🔗 Linked issue

#2286

🧭 Context

Currently browsing the code view can be somewhat annoying, since the scroll position resets on navigation.

📚 Description

It's just nice that scroll position is stable when clicking around :-)

The fix:

  • adds a ref to the scrollable aside
  • saves the scroll position (w. useState ) before unmounting
  • sets the scroll position when the aside is re-rendered
  • edit: pushed an update, so that now the scroll position is only saved when you're navigating within the same package+version

As far as I can see on the Nuxt docs flush: 'post'; is good practice when you want to watch the DOM, but I rarely touch Vue code so would appreciate feedback. Same goes for the useState composable, it's the first time I'm using it 😄 But the main part is that the state should persist across navigation.

Regarding testing: I would personally not get into testing scroll position. I've done this in the past which caused lots of flaky tests.

package-code-view-scroll-fix.mov

I did not find PRs that directly overlap. I did find this one but it's not the same thing + I believe it would be complimentary.

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 26, 2026

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

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Mar 27, 2026 8:28am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Mar 27, 2026 8:28am
npmx-lunaria Ignored Ignored Mar 27, 2026 8:28am

Request Review

@jhroemer jhroemer changed the title Package code view: file tree scroll position is stable when browsing fix(package code view): file tree scroll position is stable when browsing Mar 26, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 26, 2026

📝 Walkthrough

Walkthrough

The page component persists the file-tree sidebar’s vertical scroll per package/version. It adds a template ref file-tree-sidebar, saves the sidebar element’s scrollTop into a shared useState keyed by ${packageName}@${version} on onBeforeUnmount, and restores that scrollTop once on mount using a one-time watcher with flush: 'post'. The sidebar <aside> template now includes the new ref.

Suggested labels

needs review

Suggested reviewers

  • danielroe
  • graphieros
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description clearly relates to the changeset, detailing the scroll position persistence feature and implementation approach.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jhroemer jhroemer changed the title fix(package code view): file tree scroll position is stable when browsing fix(ui): file tree scroll position is stable when browsing Mar 26, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

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.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c8c1d70c-6026-4a1a-bb36-b59cca79e631

📥 Commits

Reviewing files that changed from the base of the PR and between 3186091 and c5b0b66.

📒 Files selected for processing (1)
  • app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue

Copy link
Copy Markdown
Contributor

@ghostdevv ghostdevv left a comment

Choose a reason for hiding this comment

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

So the issue seems to be that Nuxt is re-rendering the whole page as there is a navigation that happens when you click the file? I wonder if instead we could do something like this to prevent nuxt re-rendering completely?

definePageMeta({
    // ...
	key: (route) => `/package-code/${route.params.org ?? ''}/${route.params.packageName}/v/${route.params.version}`,
});

@jhroemer
Copy link
Copy Markdown
Contributor Author

jhroemer commented Apr 5, 2026

So the issue seems to be that Nuxt is re-rendering the whole page as there is a navigation that happens when you click the file? I wonder if instead we could do something like this to prevent nuxt re-rendering completely?

definePageMeta({
    // ...
	key: (route) => `/package-code/${route.params.org ?? ''}/${route.params.packageName}/v/${route.params.version}`,
});

Thanks @ghostdevv. Yeah I didn't really consider/question the page-reload, I kinda assumed it was intentional. I just tried it quickly and it does the trick, but there's no type inference on the route object params, and I don't know if it's possible without having to do type assertions manually or cast it?

And then I'm not entirely sure about the behavior when you change versions: if you change version, maybe the collapse/expand state should be reset too, because the file-tree might change. That's the current behavior when the entire page re-renders, but not if things stay in place. Wdyt?

@ghostdevv
Copy link
Copy Markdown
Contributor

And then I'm not entirely sure about the behavior when you change versions: if you change version, maybe the collapse/expand state should be reset too, because the file-tree might change. That's the current behavior when the entire page re-renders, but not if things stay in place. Wdyt?

So I think the code snippet I suggested (if it works, I haven't had a chance to test it) would mean that it wouldn't cause a reload if it's just the file you selected changes but would if it's the package/version. Theoretically, that would mean that Vue should be reactive and update but it's entirely possible that it won't or we're relying on the reload. I can take a look into it tomorrow? 🙏

@ghostdevv
Copy link
Copy Markdown
Contributor

If we can't figure it out we can call in some Nuxt experts, it's the perfect project for it 😄 I think this will help with the weirdness I was seeing in #1977 which has prevented it from being merged (I was trying to avoid a solution like what you've got here, but I didn't have the energy to look into or really articulate my theory - but now we can!)

@jhroemer
Copy link
Copy Markdown
Contributor Author

jhroemer commented Apr 5, 2026

And then I'm not entirely sure about the behavior when you change versions: if you change version, maybe the collapse/expand state should be reset too, because the file-tree might change. That's the current behavior when the entire page re-renders, but not if things stay in place. Wdyt?

So I think the code snippet I suggested (if it works, I haven't had a chance to test it) would mean that it wouldn't cause a reload if it's just the file you selected changes but would if it's the package/version. Theoretically, that would mean that Vue should be reactive and update but it's entirely possible that it won't or we're relying on the reload. I can take a look into it tomorrow? 🙏

I currently have guests so I need to follow-up later (probably tomorrow) too 😀 But I'm happy to dig into it and understand the rendering story here (using react at work..) I just tested locally and the effect at least is that the expand/collapse state persists across versions.

But conceptually: browsing files? Don't re-render. Changing version? Do re-render. Right?

@ghostdevv
Copy link
Copy Markdown
Contributor

I currently have guests so I need to follow-up later (probably tomorrow) too 😀

sounds good! no rush!

But conceptually: browsing files? Don't re-render. Changing version? Do re-render. Right?

ideally, yes! where re-rendering means re-rendering the whole page, or even just the file tree, ergo loosing scroll

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.

2 participants