fix: pin the sidebar only when necessary

[thasmo]

This makes the sidebar scrollable with the document, but it pins it when needed.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
npmx.dev	Ready	Preview, Comment	Feb 7, 2026 10:47pm

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
docs.npmx.dev	Ignored	Preview	Feb 7, 2026 10:47pm
npmx-lunaria	Ignored		Feb 7, 2026 10:47pm

[codecov]

Codecov Report

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

Files with missing lines	Patch %	Lines
app/components/Package/Sidebar.vue	90.90%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new Vue 3 single-file component at app/components/Package/Sidebar.vue that implements a responsive, scroll-aware sticky sidebar using the Composition API, window size/scroll hooks, template refs and computed styles. Replaces the inline area-sidebar container in app/pages/package/[[org]]/[name].vue with a PackageSidebar wrapper and moves multiple sidebar widgets into its slot (PackageSkillsCard, PackageWeeklyDownloadStats, PackagePlaygrounds, PackageCompatibility, PackageVersions, PackageInstallScripts, PackageDependencies, PackageKeywords, PackageMaintainers). Adds unit tests and an accessibility (axe) test for PackageSidebar and exports PackageSidebar from the public components surface.

Possibly related PRs

• feat: package page right sidebar scroll #805: Modifies the package page right-side sidebar markup/behaviour, replacing the inline sidebar with a scroll/sticky wrapper similar to this change.

Suggested reviewers

• danielroe

🚥 Pre-merge checks | ✅ 1

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The pull request description accurately relates to the changeset, describing the sidebar scrolling and pinning behavior implemented across the new components and tests.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

app/components/Package/Sidebar.vue (1)

12-15: Consider adding a brief comment explaining the Vue 3.4+ previous parameter pattern.

The computed((previous = 'up') syntax uses Vue 3.4+'s feature where computed getters receive their previous value. While valid and clever for maintaining direction state when neither scroll direction is active, this pattern may be unfamiliar to some developers.

💡 Suggested clarifying comment

+// Vue 3.4+ computed getter receives previous value to maintain direction when scroll is idle
 const direction = computed((previous = 'up'): string => {
   if (!active.value) return 'up'
   return scroll.directions.bottom ? 'down' : scroll.directions.top ? 'up' : previous
 })

As per coding guidelines: "Add comments only to explain complex logic or non-obvious implementations".

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

app/components/Package/Sidebar.vue (2)

2-3: window shadows the global window object.

Naming the useWindowSize() result window shadows the global window identifier. While it doesn't cause a functional issue here, it can confuse readers and trip up future contributors who may expect window to refer to the browser global.

Consider renaming to something like windowSize or viewport.

♻️ Suggested rename

-const window = useWindowSize()
+const windowSize = useWindowSize()

Then update line 9:

-  return bounds.height.value > window.height.value
+  return bounds.height.value > windowSize.height.value

---

17-25: Offset calculation could yield a negative value.

When scrolling up (line 24), the offset is:

container.offsetHeight - content.offsetTop - content.offsetHeight

If content.offsetTop + content.offsetHeight > container.offsetHeight at any point during the scroll (e.g. during layout reflow or when the container hasn't expanded to fit the padded content yet), this could go negative. A negative paddingBlockEnd is treated as invalid by CSS and the declaration will be ignored, but the intent may be wrong.

Consider clamping to Math.max(0, ...):

🛡️ Suggested defensive clamp

   return direction.value === 'down'
-    ? content.value.offsetTop
-    : container.value.offsetHeight - content.value.offsetTop - content.value.offsetHeight
+    ? Math.max(0, content.value.offsetTop)
+    : Math.max(0, container.value.offsetHeight - content.value.offsetTop - content.value.offsetHeight)

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

1344-1376: Remove unused .sidebar-scroll CSS rules.

The .sidebar-scroll class (lines 1345–1376) is not used anywhere in the template. The sidebar is now implemented via the PackageSidebar component with the area-sidebar class. These styles are dead code and should be removed.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

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

1344-1376: ⚠️ Potential issue | 🟠 Major

Remove unused .sidebar-scroll CSS rules.

The .sidebar-scroll class and all associated scrollbar styling rules (lines 1344–1376) are not referenced anywhere in the template. The sidebar is now implemented using the <PackageSidebar> component with class="area-sidebar", making this CSS block dead code and safe to remove.

🧹 Nitpick comments (2)

app/components/Package/Sidebar.vue (2)

12-15: Verify that the computed getter's previous parameter behaves as intended.

Vue 3.4+ passes the previous computed value as the first argument to the getter. However, if active is false and both scroll.directions.bottom and scroll.directions.top are falsy, the function returns previous, which preserves the last direction. This is a neat pattern but subtle — worth a brief inline comment for future maintainers explaining that the default parameter 'up' only applies on the very first evaluation.

Also note: once the useWindowScroll → useScroll fix is applied, scroll.directions.bottom and scroll.directions.top will be booleans, so the ternary chain on line 14 will work correctly.

---

17-25: Potential negative offset when content is repositioned.

The "scrolling up" branch computes:

container.offsetHeight - content.offsetTop - content.offsetHeight

If the content's offsetTop plus its offsetHeight exceeds the container's offsetHeight (which can happen transiently during layout shifts or when padding is applied by this very component), the result would be negative, producing a negative paddingBlockEnd. While browsers typically clamp negative padding to 0, it may be worth guarding explicitly with Math.max(…, 0).

🛡️ Suggested defensive guard

   return direction.value === 'down'
     ? content.value.offsetTop
-    : container.value.offsetHeight - content.value.offsetTop - content.value.offsetHeight
+    : Math.max(0, container.value.offsetHeight - content.value.offsetTop - content.value.offsetHeight)

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

app/components/Package/Sidebar.vue (2)

17-25: offset can go negative — consider clamping to zero.

If the content element's position causes container.offsetHeight - content.offsetTop - content.offsetHeight to be negative (e.g. during rapid layout shifts or when content shrinks), the computed padding will become a negative pixel value, which browsers ignore but signals unexpected state. A Math.max(0, …) guard would be defensive and cheap.

🛡️ Suggested guard

-  return direction.value === 'down'
-    ? content.value.offsetTop
-    : container.value.offsetHeight - content.value.offsetTop - content.value.offsetHeight
+  const raw = direction.value === 'down'
+    ? content.value.offsetTop
+    : container.value.offsetHeight - content.value.offsetTop - content.value.offsetHeight
+
+  return Math.max(0, raw)

---

12-15: Narrow the return type and parameter type annotation for type safety.

The computed getter correctly uses Vue 3.4+'s previous-value parameter with a default of 'up'. However, the return type : string is overly broad—the function only ever returns 'down' or 'up' (or the previous value), and downstream usage in offset and style computeds, as well as the template's data-direction binding, all expect one of these two literal values.

Suggested change

-const direction = computed((previous = 'up'): string => {
+const direction = computed((previous: 'up' | 'down' = 'up'): 'up' | 'down' => {

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

1344-1376: Remove the unused .sidebar-scroll CSS block.

The .sidebar-scroll class defined at lines 1344–1376 is not referenced anywhere in the codebase and can be safely removed. This scoped CSS is dead code that's no longer needed.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (4)

app/components/Package/Sidebar.vue (2)

17-25: offsetTop / offsetHeight reads are not reactive — offset may go stale.

The offset computed reads content.value.offsetTop and container.value.offsetHeight directly from the DOM, which are not tracked by Vue's reactivity system. The computed only re-evaluates when active or direction changes, so the captured offset reflects the DOM state at the moment of the last direction change. This works for the scroll-pinning use case (offset is "frozen" until the next direction flip), but if the sidebar content resizes dynamically (e.g. lazy-loaded sections, expandable accordions), the offset won't update.

Worth considering whether useElementBounding (which is reactive) could provide the necessary values instead, or whether a manual update trigger on content resize would be prudent. If the sidebar content is effectively static once rendered, this is fine as-is.

---

12-15: Add an inline comment explaining the Vue 3.4+ computed previous-value pattern.

The computed((previous = 'up') => ...) pattern on line 12 uses the Vue 3.4+ feature where the getter receives the previous computed value as its first argument. The = 'up' default handles the initial undefined case. While functionally correct, a brief comment would clarify this non-obvious pattern for future readers:

// previous parameter receives the prior direction value (Vue 3.4+)
const direction = computed((previous = 'up'): string => {
  if (!active.value) return previous
  return scroll.directions.bottom ? 'down' : scroll.directions.top ? 'up' : previous
})

test/nuxt/components/PackageSidebar.spec.ts (2)

22-61: Test suite lacks coverage for the component's core scroll-direction behaviour.

The sidebar's primary feature — switching between direction="up" and direction="down" based on scroll, and computing offset/style accordingly — has no test coverage. Consider adding tests for:

1. data-active="true" when mockContentHeight > mockViewportHeight
2. data-direction toggling to "down" when mockScrollDirections.bottom = true
3. Offset/style computation (padding applied in the correct direction)

🧪 Example additional tests

it('sets active=true when content is taller than viewport', async () => {
  mockContentHeight.value = 1500
  const wrapper = await mountSuspended(Sidebar)
  expect(wrapper.attributes('data-active')).toBe('true')
})

it('sets direction to down when scrolling down while active', async () => {
  mockContentHeight.value = 1500
  const wrapper = await mountSuspended(Sidebar)
  expect(wrapper.attributes('data-direction')).toBe('up')

  mockScrollDirections.bottom = true
  await wrapper.vm.$nextTick()
  expect(wrapper.attributes('data-direction')).toBe('down')
})

---

11-20: Mock for useElementBounding omits most return properties.

The mock only returns { height, update }, but useElementBounding returns many more reactive properties (width, top, left, right, bottom, x, y). This works today because the component only reads height, but if the component ever accesses another property, this mock will cause a confusing test failure. Consider spreading a minimal set of default refs to future-proof.

[coderabbitai]

Actionable comments posted: 1

[vercel]

@thasmo is attempting to deploy a commit to the serhalp's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

Actionable comments posted: 1

[43081j]

Unsure if my approach with the test rework is exactly right but we can always tweak it later if it isn't.

Looks good to me 👍

[thasmo]

One issue still needs to be fixed.

Fixed.