Skip to content

fix(menu): add max-height and scroll to settings menu panel#2893

Open
terminalchai wants to merge 1 commit intosampotts:developfrom
terminalchai:fix/menu-scroll-overflow
Open

fix(menu): add max-height and scroll to settings menu panel#2893
terminalchai wants to merge 1 commit intosampotts:developfrom
terminalchai:fix/menu-scroll-overflow

Conversation

@terminalchai
Copy link
Copy Markdown

Summary

Fixes #1420

Problem

When a player has many quality options or caption/language tracks, the settings menu panel has no height constraint — it grows as tall as needed and clips off-screen, with no way to scroll to the hidden items.

A previous attempt (comment) solved this by adding a new [role='menu-captions']\ element, but as @sampotts noted that's not a valid ARIA role.

Fix

Two small changes, no JS touched:

*\src/sass/settings/menus.scss* — add a new variable following the existing pattern:
\\scss
-menu-max-height: var(--plyr-menu-max-height, 240px) !default;
\\

*\src/sass/components/menus.scss* — apply it to the existing [role='menu']\ selector:
\\diff
[role='menu'] {

  • max-height: -menu-max-height;
  • overflow-y: auto;
    padding: -control-padding;
    }
    \\

The \240px\ default shows roughly 7–8 items before scrolling kicks in. Both the SCSS variable and the --plyr-menu-max-height\ CSS custom property can be overridden by consumers who need a different height.

… items

When a player has many quality options or caption tracks (e.g. 30
languages), the settings menu panel grows beyond the viewport with no
way to scroll, clipping the excess items.

Add a \\-menu-max-height\ SCSS variable (backed by the
\--plyr-menu-max-height\ CSS custom property, defaulting to 240px)
and apply it to the \[role='menu']\ container along with
\overflow-y: auto\.

This caps the panel height at 240px and lets it scroll, without
touching any ARIA roles or JS logic.

Fixes sampotts#1420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant