fix(docs): make product selector popover scrollable on small screens

[rusikv]

Problem

Bug report: the docs product selector (top of the docs sidebar) has no scroll on small screens.

The product popover (.ps-popover) is position: fixed with JS-set top/left/width but no height cap and no overflow handling. The list now holds 9 products across 3 sections (~600px tall), so on short viewports (phones, small laptop windows) the bottom entries — Mobile Application, License Server, IoT Hub — render off-screen. Since the popover is fixed and the mobile docs menu locks page scroll, those items are completely unreachable.

Fix

• positionPopover() now also sets max-height to the space remaining between the trigger and the bottom viewport edge (same 8px pad already used for horizontal clamping). Recomputed on resize via the existing repositionOpen handler.
• .ps-popover gets overflow-y: auto + overscroll-behavior: contain so the capped list scrolls instead of clipping, without chaining scroll to the page.
• .ps-section-label / .ps-item get flex-shrink: 0 so rows keep natural height and overflow into scroll instead of being compressed by the column flexbox.

Applies to both the product popover and the Cloud region popover (shared positioning + CSS).

Verification

• pnpm check — 0 errors / 0 warnings
• eslint on the changed file — clean

[vvlladd28]

Review summary

Reviewed 1 changed file in fix(docs): make product selector popover scrollable on small screens. Left 6 comment(s) inline.

The fix is the right shape and the flex-shrink: 0 coverage is complete — .ps-section-label and .ps-item are the only direct children of either popover, and nothing outside this component targets those classes. The main things worth a look: max-height can compute negative (a silent no-op that leaves the previous cap in place, re-exposing the bug in the very short-viewport case this targets), capping to the space below the trigger is the least generous placement when there's more room above, and the new scroll container has no scrollbar treatment unlike its siblings elsewhere in the repo.

---

This review was auto-generated. Findings may contain errors — please verify before applying changes.

[vvlladd28]

The 8 here is the trigger→popover gap, and it now feeds the max-height formula as well, so it's load-bearing in two computations rather than a throwaway offset. It also happens to equal viewportPad while meaning something completely different — the next person adjusting the bottom breathing room could reasonably assume the two must stay in sync. Worth naming it, e.g. const triggerGap = 8; alongside viewportPad, so the max-height line reads as window.innerHeight - (rect.bottom + triggerGap) - viewportPad.

[vvlladd28]

This is the only scrollable overlay in the repo whose height cap lives in JS — every sibling does it in CSS: starlight/MobileTableOfContents.astro:152 (max-height: calc(85vh - var(--sl-nav-height) - var(--sl-mobile-toc-height)) + overflow-y: auto + overscroll-behavior: contain, i.e. the same problem and the same CSS pair), SearchButton.astro:297, IotHub/FilterPanel.astro:389.

Since top genuinely is dynamic here a pure-CSS solution isn't available, but a middle ground keeps the geometry in one file: have JS write popover.style.setProperty('--ps-popover-top', top + 'px') and let the stylesheet own max-height: calc(100vh - var(--ps-popover-top) - 8px) right next to the overflow-y it depends on. Then the 8px pad and the scroll behaviour aren't split across two languages, and someone tweaking the popover only has to look at the <style> block.

[vvlladd28]

Can this go negative? top is anchored near the top of the viewport (the card sits at the top of the sidebar), so shrinking the window height barely moves it — drag a desktop window short enough, or land on a short landscape viewport with the region row rendered, and window.innerHeight - top - viewportPad can end up below zero. max-height doesn't accept negative lengths, so the CSSOM assignment is a silent no-op and the popover keeps whatever cap the previous repositionOpen tick left on it (or no cap at all, on a first open at that size) — which is the off-screen overflow this PR is fixing, in the short-viewport case it specifically targets. A Math.max() floor would close it.

Related: capping to the space below the trigger is the least generous placement strategy — when the trigger sits low, the ~600px 9-entry list gets squeezed into whatever sliver is left even when there's far more usable room above it. The pricing tooltip in this repo already handles the analogous case by shifting up instead (pages/pricing/index.astro:995-996: top = window.innerHeight - tipRect.height - 8). Would it be worth measuring the natural height and shifting top upward (clamped to viewportPad) before falling back to the cap, or flipping above the trigger when that space is larger? It keeps the fix, avoids the ~100px scroll window, and makes the negative case above unreachable. Trade-off: it needs the laid-out height, so it has to run once the popover is visible — the pricing code does that inside a requestAnimationFrame.

One more, less certain: since the bug is specifically about phones, is window.innerHeight the right measure? On mobile Safari the visually available height can be smaller than the layout viewport that position: fixed resolves against, and window's resize doesn't fire when the toolbar collapses or expands — visualViewport.height plus a visualViewport resize listener would track that. Likely a smaller effect than the placement question, but worth a thought.

[vvlladd28]

overflow-y: auto; overscroll-behavior: contain; is copied verbatim from starlight/MobileTableOfContents.astro:153-156 — those are the only two overscroll-behavior occurrences in the codebase. Zooming out, there are now three hand-rolled viewport-clamping implementations that each invented their own pad constant and their own strategy: this one (cap + scroll, 8px), the pricing tooltip (pages/pricing/index.astro:965-997, flip + shift, 8px), and the mega-menu submenu (Landing/Navigation.astro:254-263, horizontal clamp, 10px). There's no shared positioning helper, no floating-ui, and no use of the native popover API in the project.

The repo already extracted src/util/scroll-lock.ts out of exactly this kind of duplication — its header even calls out "the inlined copies" — so a small @util/position-floating.ts exporting something like anchorBelow(trigger, el, { gap, pad }) looks like the direction of travel. Not necessarily this PR's job, but if it stays inline, a comment pointing at the siblings would help keep the three from drifting further apart.

[vvlladd28]

Turning .ps-popover into a scroll container without any scrollbar treatment diverges from how the rest of the site handles it, and it shows the moment the list overflows: on Windows/Linux a classic ~15px scrollbar appears, eating into the row width and crowding the right-aligned check icon, and rows visibly reflow as the cap kicks in. starlight/Sidebar.astro:138 uses scrollbar-gutter: stable for precisely this (with a long comment about macOS overlay scrollbars and content sitting flush against the pane edge), and IotHub/FilterPanel.astro:401 styles a thin custom scrollbar instead. Adding scrollbar-gutter: stable (or the thin-scrollbar treatment) would stop the popover jumping between its capped and uncapped states.

Separately: with only padding: 4px inside a 12px border-radius, the first and last rows clip against the rounded corners while scrolling. A couple more px of vertical padding, or a sticky section label, would make the "there is more below" cue read better.

[vvlladd28]

These two flex-shrink: 0 declarations plus the comment explaining them exist only because .ps-popover is a column flex container — but does it need to be one? There's no gap, no align-items, nothing flex-sized among the children, and .ps-item sets its own display: flex anyway. Switching the popover to display: block (the &[hidden] { display: none } rule keeps working) would give rows their natural height for free and let both flex-shrink: 0 lines and the explanatory comment go away.

If something does rely on the flex context, .ps-popover > * { flex-shrink: 0; } expresses "all rows, not these two specific ones" more durably than patching each child rule — the bare flex-shrink: 0 on .ps-item below carries no comment and reads as deletable cruft to anyone who finds it later.

[rusikv]

Thanks — applied four of these, and pushed back on two. Details below.

Negative max-height — fixed. Floored with Math.max(0, …), with a comment recording why it isn't defensive noise (invalid value → silent CSSOM drop → stale cap from the previous call, or none at all on a first open). For the record on reachability: mobile .sidebar-pane is inset-block: var(--sl-nav-height) 0 with $header-height: 80px, so the trigger's bottom edge sits ~149px down and innerHeight - 149 - 8 - 8 only turns negative below ~165px of viewport height — an extreme window drag, not any real device. Worth closing regardless, since the failure is silent rather than visible.

The two 8s — named. triggerGap and viewportPad are now separate constants with a comment on why they aren't interchangeable despite the equal value. You were right that the gap became load-bearing in two computations once it fed the height cap.

scrollbar-gutter: stable — added, with a pointer to .sidebar-pane's reasoning in starlight/Sidebar.astro.

The column flexbox — removed. You were right that nothing needed it: no gap, nothing flex-sized, and .ps-item sets its own display: flex anyway. .ps-popover is now display: block, so rows take their natural height for free and both flex-shrink: 0 patches plus the comment explaining them are gone, rather than being made more durable via > *.

Placement rework — declining, deliberately. I measured both halves of the suggestion, and they come out differently:

Flipping above the trigger is measurably wrong for this component. Unlike the pricing tooltip, this trigger isn't free-floating — it's pinned to the top of the sidebar pane, so the space above it is only the ~105px taken by the header and the pane's padding, against ~210px below on a 375px-tall landscape viewport. Flipping would roughly halve the room in exactly the case we're trying to help.

Shifting up is the stronger version of the idea, and it does buy real height — clamping top to pad reclaims ~149px on that landscape viewport, a ~70% taller list. But it only gets that by drawing the popover over the header and over the selector itself, and that's the part we don't want: the trigger reads "You're reading docs for <product>", which is the context you need while choosing a product. Covering it at the moment of choice trades a UX regression for scroll length, on a control that is fundamentally about telling you where you are. Cap-and-scroll keeps the anchor visible, and a ~210px scroll window over 9 entries is a workable worst case. So this is a deliberate product call rather than an oversight.

Shared positioning helper — not yet, and I'd argue not in this PR. I prototyped @util/position-floating.ts with a pure computeAnchorBelow core and then dropped it, because it didn't survive its own justification:

• scroll-lock.ts, the precedent, has 12 consumers. This would have had exactly one, so it isn't deduplication — it's a file move.
• It wouldn't reduce the count you flagged: still three hand-rolled implementations, plus a fourth file.
• Neither sibling adopts it unchanged. The pricing tooltip flips and shifts; the mega-menu centres under its item, clamps horizontally only, and works in container-relative coordinates rather than viewport-fixed. Supporting all three means options for alignment, coordinate space, and flip behaviour — more configuration than code, and it would put the site header and the pricing page in the blast radius of a docs-sidebar bugfix.
• The repo also has no test runner, so "the pure core is testable without a DOM" was a theoretical benefit rather than a real one.

Agreed on the direction, though — the right trigger is a second consumer that wants anchor-below-and-cap as-is, and then it's a genuine extraction instead of a speculative one.

Two left alone, flagging rather than silently skipping: the extra vertical padding for rows clipping against the 12px corner radius, and visualViewport for mobile Safari toolbar collapse. On the latter I'm genuinely unsure it's an improvement — position: fixed resolves against the layout viewport, so capping to visualViewport.height would under-cap whenever the toolbar is expanded. Happy to take either if you'd rather they land here.