fix(ui): Sidebar scroll not showing last element #8563
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website Please review the changes when you have a chance. Thank you! 🙏 |
There was a problem hiding this comment.
Pull request overview
Updates the article sidebar container styling to ensure the sidebar can scroll fully and the last navigation item remains visible within the viewport.
Changes:
- Adjusted the article sidebar container from a fixed
100vhheight to amax-heightthat subtracts the header height CSS variable. - Bumped
@node-core/ui-componentspackage version from1.5.7to1.5.8.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/ui-components/src/Containers/Article/index.module.css | Constrains the sticky sidebar’s scrollable height to the visible viewport area (accounting for header height). |
| packages/ui-components/package.json | Patch version bump for the UI components package. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@malav2110 there's an odd effect here: 
Screencast_20260126_144928.webmThere's a weird effect where the sidebar is not full width. This is how's supposed to look: 
This is how it looks now in certain scroll positions: 
|
|
@ovflowd Thanks for the video. I want to make sure I’m looking at the right things.
|
Not really, what I'm referring is the Sidebar is not full width, the Sidebar should go all the way to the bottom bar (not the footer), but the bar that has the breadcrumbs. |
This is what Im referring to, the Sidebar should be occupying this area |
|
Ah, got it. I see the gap you’re referring to now. I’ll fix this. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
@malav2110 with the current code now the issue is at the beginning of the sidebar ;) 
|
|
Sharp eyes :) . I’ll address the top offset issue and make sure all scroll states are covered before the next push. |
| py-6 | ||
| sm:overflow-auto | ||
| sm:border-r | ||
| sm:pb-16 |
There was a problem hiding this comment.
Padding could probably be a bit smaller.
There was a problem hiding this comment.
With pb-14, the last item on the list is barely visible thus added pb-16.
There was a problem hiding this comment.
Is that really the case? This is how much padding I see
There was a problem hiding this comment.
You're probably referring to the padding when the footer isn't visible.
So this is probably not the place you want to change the padding.
There was a problem hiding this comment.
For example, if you see the beginning of the sidebar has the same margins/paddings no matter if the header is visible or not
But the same isn't truthy for the end of the sidebar:
So this means this place is probably not the right one to add padding. Or it must be a mix of two different places.
There was a problem hiding this comment.
That's weird. the Metabar is also stick and doesn't have said issue, so I'm pretty sure the same can be accomplished to the Sidebar 🤔
There was a problem hiding this comment.
I did review the MetaBar to compare.
It is also position: sticky with top: 0, but the key difference is that it does not have a height restriction or internal scrolling. Its content is short enough to fit naturally in the viewport.
The SideBar is different. On Learn page, it has many more items, so it needs max-height + overflow-y-auto to scroll internally.
That's where the issue comes from:
100vh uses the full viewport height, but when the NavBar (+ security banner) is visible, it takes up space at top. The Sidebar still assumes the full height, so the last item/s ends up getting clipped.
I tested both cases:
- Without the height constraint -> everything is visible, but user do not see the internal scrollbar and have to scroll the whole page.
- With the constraint -> internal scrolling works, last item gets cutoff when the NavBar is present.
Because the MetaBar rarely overflows, it avoids this edge case entirely.
There was a problem hiding this comment.
It is also position: sticky with top: 0, but the key difference is that it does not have a height restriction or internal scrolling. Its content is short enough to fit naturally in the viewport.
Unfortunately, you're wrong. Example: https://nodejs-api-docs-tooling.vercel.app/child_process.html / https://nodejs-api-docs-tooling.vercel.app/globals.html
There was a problem hiding this comment.
(I would recommend to avoid making assumptions, if I tell you it is possible or something the contrary, then I'm pretty sure it is possible 😛)
There was a problem hiding this comment.
(I just want to avoid giving you a solution plated, so you can try it yourself 👀)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8563 +/- ##
==========================================
+ Coverage 74.95% 74.98% +0.02%
==========================================
Files 103 103
Lines 9037 9037
Branches 312 312
==========================================
+ Hits 6774 6776 +2
+ Misses 2261 2259 -2
Partials 2 2 ☔ View full report in Codecov by Sentry. |
2 participants
Description
Fixed the Sidebar scrolling issue where last item was not visible
Validation
Before:
Before.mov
After:
After.mov
Related Issues
Fixes: #8521
Closes: #8522
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.