Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vvlladd28 merged 2 commits into from
Jul 29, 2026
Merged

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e5188a
fix(docs): make product selector popover scrollable on small screens
rusikv Jul 29, 2026
2a2b35f
fix(docs): clamp product selector popover height and reserve its gutter
rusikv Jul 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions src/components/VersionSwitcher.astro
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,13 +343,30 @@ const activeRegion = currentFamily.regions?.options.find((r) => r.product === cu
const rect = trigger.getBoundingClientRect();
const minWidth = 320;
const width = Math.max(rect.width, minWidth);
// Breathing space kept between the popover and the viewport edges.
const viewportPad = 8;
// Space between the trigger's bottom edge and the popover's top
// edge. Deliberately separate from `viewportPad` despite the equal
// value — it feeds both the top offset and the height cap, so the
// two aren't interchangeable.
const triggerGap = 8;
// Anchor to trigger.left, but keep popover inside viewport on the right.
const maxLeft = window.innerWidth - width - viewportPad;
const left = Math.min(rect.left, Math.max(viewportPad, maxLeft));
popover.style.top = `${rect.bottom + 8}px`;
const top = rect.bottom + triggerGap;
popover.style.top = `${top}px`;
popover.style.left = `${left}px`;
popover.style.width = `${width}px`;
// Cap the popover to the room under the trigger so an over-tall
// list scrolls (see `overflow-y` on `.ps-popover`) instead of
// running off-screen on short viewports.
//
// The `Math.max(0, …)` floor is load-bearing rather than defensive:
// a negative `max-height` is invalid CSS, and an invalid value
// assigned through the CSSOM is dropped silently, which would leave
// whatever cap the previous call set — or none at all on a first
// open, i.e. exactly the overflow this is fixing.
popover.style.maxHeight = `${Math.max(0, window.innerHeight - top - viewportPad)}px`;
};

triggers.forEach((trigger) => {
Expand Down Expand Up @@ -644,18 +661,30 @@ const activeRegion = currentFamily.regions?.options.find((r) => r.product === cu
// ─── Popovers ────────────────────────────────────────────────────────────
.ps-popover {
position: fixed;
// top / left / width are set by JS so the popover can extend past the
// sidebar boundary into the content area (matches the design).
// top / left / width / max-height are set by JS (see `positionPopover`)
// so the popover can extend past the sidebar boundary into the content
// area (matches the design) while staying inside the viewport.
z-index: 1000;
display: flex;
flex-direction: column;
// Plain block flow: rows take their natural height, so a capped
// popover overflows into scroll instead of compressing them. (A column
// flexbox would need `flex-shrink: 0` on every child to behave.)
display: block;
padding: 4px;
background: var(--sl-color-bg-sidebar, var(--sl-color-bg));
border: 1px solid var(--sl-color-hairline-light);
border-radius: 12px;
box-shadow:
0 4px 12px rgba(0, 0, 0, 0.08),
0 1px 3px rgba(0, 0, 0, 0.06);
// JS caps max-height to the space below the trigger; scroll the rest
// instead of clipping it off-screen on short viewports.
overflow-y: auto;

@vvlladd28 vvlladd28 Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

overscroll-behavior: contain;

@vvlladd28 vvlladd28 Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

// Reserve the scrollbar gutter up front (same reasoning as
// `.sidebar-pane` in starlight/Sidebar.astro) so rows don't reflow and
// the right-aligned check icon doesn't get crowded when the cap
// engages and a classic scrollbar appears.
scrollbar-gutter: stable;

&[hidden] {
display: none;
Expand Down