fix(web): add scroll to settings language menu when it overflows view…

[zhujie007]

Why

The settings language picker opens upward with 9 locale options (~360px total height). When the button is positioned in the upper half of the viewport, the top of the menu extends beyond the viewport boundary and becomes unreachable — users cannot see or click the top locale options (e.g. English, Deutsch). There is no scroll fallback.

What

• Add max-height with overflow-y: auto to the language menu so it scrolls when content exceeds viewport space.
• Switch from bottom to top + transform: translateY(-100%) positioning so the scroll constraint activates correctly with the upward expansion behavior.

Screenshots

(before/after screenshots here)

[lefarcen]

Hi @zhujie007! 🎉
Thanks for the contribution — fixing the language picker viewport overflow is exactly the kind of UX polish that makes a difference.
I will run a deep review and get back to you within 24h.

Thanks for making open-design better!
— open-design team

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b56e62e7f8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[lefarcen]

Hey @zhujie007, thanks for the fix! The viewport overflow issue is properly solved — the positioning math and scroll container work correctly.

What's solid:

• top + translateY(-100%) correctly pins menu upward
• max-height: min(360px, calc(100vh - 24px)) constrains to viewport (9 locales × ~40px)
• overscroll-behavior: contain prevents scroll bleed
• Existing resize listener already handles viewport changes

One accessibility gap to be aware of (non-blocking for this PR):

The menu now scrolls, but keyboard-only users can't navigate it with arrow keys. The current implementation only handles Escape, not up/down arrows + auto-scroll-into-view. This is a pre-existing gap (the original menu also lacked arrow key handlers), so not a regression you introduced.

Worth addressing in a follow-up or separate issue — I can open one if helpful.

Otherwise this is good to merge. Deferring final approval to a maintainer.

[lefarcen]

P1 resolved ✅ — The CSS variable approach () fixes the viewport overflow issue perfectly. The menu now dynamically constrains its height based on available space above the trigger, with scroll fallback. Exactly what was needed.

LGTM — all prior concerns (P1 viewport math, P2 keyboard nav, P3 CSS comment) are now addressed. Ready for maintainer approval.

[lefarcen]

P1 resolved ✅ — The CSS variable approach (--menu-available-h) fixes the viewport overflow issue perfectly. The menu now dynamically constrains its height based on available space above the trigger, with scroll fallback. Exactly what was needed.

LGTM — all prior concerns (P1 viewport math, P2 keyboard nav, P3 CSS comment) are now addressed. Ready for maintainer approval.

[lefarcen]

P1 resolved ✅ — The CSS variable approach (--menu-available-h) fixes the viewport overflow issue perfectly. The menu now dynamically constrains its height based on available space above the trigger, with scroll fallback. Exactly what was needed.

LGTM — all prior concerns (P1 viewport math, P2 keyboard nav, P3 CSS comment) are now addressed. Ready for maintainer approval.

[lefarcen]

Hey @zhujie007, great iteration! 🎉

You've addressed both the positioning math (P1) and the documentation gap (P2) from the prior review. The approach is solid:

What's good:

• bottom + --menu-available-h correctly constrains the menu to available space above the trigger
• CSS comment clearly explains the 360px magic number and the CSS var contract
• Scroll container setup is correct (overflow-y: auto, overscroll-behavior: contain)

One minor accessibility gap (P2, pre-existing — not a regression from this PR):
The language menu is missing aria-labelledby pointing to the trigger button. This helps screen readers announce "Language menu" when focus enters. Example fix:

// SettingsDialog.tsx line ~470
<button
  id="settings-language-trigger"
  onClick={...}
>
  {currentLocaleName}
</button>

// line ~485
<div
  aria-labelledby="settings-language-trigger"
  role="menu"
  ...
>

This is a nice-to-have, not blocking — the menu already has role="menu" so it's navigable. Up to you whether to include it in this PR or defer.

Summary: The core fix is complete and ready to merge IMO. Deferring final approval to a maintainer.

— open-design team

[lefarcen]

Approved.

[lefarcen]

Merged! 🎉 Thanks @zhujie007 for the contribution to open-design. Looking forward to the next one!