Skip to content

refactor(packages)!: simplify menus #2029

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

Draft
mihar-22 wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

refactor(packages)!: simplify menus #2029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
89f5488
refactor(packages)!: simplify menu composition
mihar-22 Aug 10, 2026
d6a6e3e
fix(packages): correct submenu backgrounds and sizing
mihar-22 Aug 10, 2026
ad88f4e
fix(skin): hide covered submenu triggers
mihar-22 Aug 10, 2026
8996697
fix(packages): hide content behind expanded submenus
mihar-22 Aug 11, 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
42 changes: 14 additions & 28 deletions apps/sandbox/templates/html-menu/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,10 @@ const checkboxItemClass = [menuItemClass, 'pl-8'].join(' ');
const subMenuTriggerClass = [menuItemClass, 'justify-between'].join(' ');

const subMenuContentClass = [
'absolute inset-0 z-10 bg-white rounded-[inherit] p-1 outline-none overflow-hidden translate-x-0',
'transition-transform duration-300 ease-in-out will-change-transform',
'[&[data-starting-style][data-direction=forward]]:translate-x-full',
'[&[data-ending-style][data-direction=forward]]:-translate-x-full',
'[&[data-starting-style][data-direction=back]]:-translate-x-full',
'[&[data-ending-style][data-direction=back]]:translate-x-full',
].join(' ');

// Root and submenu views share the same viewport so they can slide over each other.
const rootViewClass = [
'absolute inset-0 p-1 translate-x-0',
'transition-transform duration-300 ease-in-out will-change-transform',
'data-[menu-view-state=inactive]:-translate-x-full',
'absolute inset-x-0 top-0 z-10 bg-white rounded-[inherit] p-1 outline-none overflow-auto',
'translate-none transition-[translate,filter] duration-150 ease-in-out',
'data-starting-style:translate-x-full data-ending-style:translate-x-full',
'data-starting-style:blur-sm data-ending-style:blur-sm',
].join(' ');

const backButtonClass = [
Expand All @@ -65,9 +56,9 @@ const backButtonClass = [
].join(' ');

const menuNavPopupClass = [
'group relative',
'relative',
menuNavSurfaceClass,
'w-(--media-menu-width) h-(--media-menu-height)',
'w-(--media-menu-width) max-w-(--media-menu-available-width) h-(--media-menu-height)',
'transition-[opacity,scale,translate,filter,width,height] duration-150 ease-in-out',
menuContentPlacementClass,
].join(' ');
Expand Down Expand Up @@ -144,11 +135,8 @@ root.innerHTML = `
</button>

<media-menu id="nav-menu" class="${menuNavPopupClass}">
<!-- Root list view — slides left when a submenu is active. -->
<media-menu-view id="nav-root-view" class="${rootViewClass}">

<!-- Quality submenu trigger -->
<media-menu-item id="nav-quality-trigger" commandfor="nav-quality-sub" class="${subMenuTriggerClass}">
<!-- Quality submenu trigger -->
<media-menu-item id="nav-quality-trigger" commandfor="nav-quality-sub" class="${subMenuTriggerClass}">
<span>Quality</span>
<span class="flex items-center gap-1">
<span id="nav-quality-hint" class="text-xs text-slate-400">auto</span>
Expand All @@ -165,16 +153,14 @@ root.innerHTML = `
</span>
</media-menu-item>

<media-menu-separator class="${menuSeparatorClass}"></media-menu-separator>
<media-menu-item id="nav-copy-item" class="${menuItemClass}">Copy link</media-menu-item>

</media-menu-view>
<media-menu-separator class="${menuSeparatorClass}"></media-menu-separator>
<media-menu-item id="nav-copy-item" class="${menuItemClass}">Copy link</media-menu-item>

<media-menu id="nav-quality-sub" class="${subMenuContentClass}">
<media-menu-back class="${backButtonClass}">
<media-menu-item class="${backButtonClass}">
<svg class="w-3.5 h-3.5 shrink-0" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round"><polyline points="15 18 9 12 15 6"/></svg>
Quality
</media-menu-back>
</media-menu-item>
<media-menu-radio-group id="nav-quality-group" aria-label="Resolution" value="auto">
<media-menu-radio-item value="auto" class="${radioItemClass}">Auto</media-menu-radio-item>
<media-menu-radio-item value="1080p" class="${radioItemClass}">1080p</media-menu-radio-item>
Expand All @@ -184,10 +170,10 @@ root.innerHTML = `
</media-menu>

<media-menu id="nav-speed-sub" class="${subMenuContentClass}">
<media-menu-back class="${backButtonClass}">
<media-menu-item class="${backButtonClass}">
<svg class="w-3.5 h-3.5 shrink-0" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round"><polyline points="15 18 9 12 15 6"/></svg>
Speed
</media-menu-back>
</media-menu-item>
<media-menu-radio-group id="nav-speed-group" aria-label="Speed" value="1">
<media-menu-radio-item value="0.5" class="${radioItemClass}">0.5x</media-menu-radio-item>
<media-menu-radio-item value="0.75" class="${radioItemClass}">0.75x</media-menu-radio-item>
Expand Down
141 changes: 65 additions & 76 deletions apps/sandbox/templates/react-menu/main.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,10 @@ const checkboxItemClass = [menuItemClass, 'pl-8'].join(' ');
const subMenuTriggerClass = [menuItemClass, 'justify-between'].join(' ');

const subMenuContentClass = [
'absolute inset-0 z-10 bg-white rounded-[inherit] p-1 outline-none overflow-hidden translate-x-0',
'transition-transform duration-300 ease-in-out will-change-transform',
'[&[data-starting-style][data-direction=forward]]:translate-x-full',
'[&[data-ending-style][data-direction=forward]]:-translate-x-full',
'[&[data-starting-style][data-direction=back]]:-translate-x-full',
'[&[data-ending-style][data-direction=back]]:translate-x-full',
].join(' ');

// Root and submenu views share the same viewport so they can slide over each other.
const rootViewClass = [
'absolute inset-0 p-1 translate-x-0',
'transition-transform duration-300 ease-in-out will-change-transform',
'data-[menu-view-state=inactive]:-translate-x-full',
'absolute inset-x-0 top-0 z-10 bg-white rounded-[inherit] p-1 outline-none overflow-auto',
'translate-none transition-[translate,filter] duration-150 ease-in-out',
'data-starting-style:translate-x-full data-ending-style:translate-x-full',
'data-starting-style:blur-sm data-ending-style:blur-sm',
].join(' ');

const backButtonClass = [
Expand All @@ -67,9 +58,9 @@ const backButtonClass = [
].join(' ');

const menuNavPopupClass = [
'group relative',
'relative',
menuNavSurfaceClass,
'w-(--media-menu-width) h-(--media-menu-height)',
'w-(--media-menu-width) max-w-(--media-menu-available-width) h-(--media-menu-height)',
'transition-[opacity,scale,translate,filter,width,height] duration-150 ease-in-out',
menuContentPlacementClass,
].join(' ');
Expand Down Expand Up @@ -235,69 +226,67 @@ function App() {
<Menu.Root>
<TriggerButton>Settings</TriggerButton>
<Menu.Content className={menuNavPopupClass}>
<Menu.View className={rootViewClass}>
{/* Quality submenu */}
<Menu.Root>
<Menu.Trigger className={subMenuTriggerClass}>
<span>Quality</span>
<span className="flex items-center gap-1">
<span className="text-xs text-slate-400">{quality}</span>
<ChevronRight />
</span>
</Menu.Trigger>
<Menu.Content className={subMenuContentClass}>
<Menu.Back className={backButtonClass}>
<ChevronLeft />
Quality
</Menu.Back>
<Menu.RadioGroup aria-label="Resolution" value={quality} onValueChange={setQuality}>
{['auto', '1080p', '720p', '480p'].map((v) => (
<Menu.RadioItem key={v} value={v} className={radioItemClass}>
{quality === v && <RadioDot />}
{v}
</Menu.RadioItem>
))}
</Menu.RadioGroup>
</Menu.Content>
</Menu.Root>
{/* Quality submenu */}
<Menu.Root>
<Menu.Trigger className={subMenuTriggerClass}>
<span>Quality</span>
<span className="flex items-center gap-1">
<span className="text-xs text-slate-400">{quality}</span>
<ChevronRight />
</span>
</Menu.Trigger>
<Menu.Content className={subMenuContentClass}>
<Menu.Item className={backButtonClass}>
<ChevronLeft />
Quality
</Menu.Item>
<Menu.RadioGroup aria-label="Resolution" value={quality} onValueChange={setQuality}>
{['auto', '1080p', '720p', '480p'].map((v) => (
<Menu.RadioItem key={v} value={v} className={radioItemClass}>
{quality === v && <RadioDot />}
{v}
</Menu.RadioItem>
))}
</Menu.RadioGroup>
</Menu.Content>
</Menu.Root>

{/* Speed submenu */}
<Menu.Root>
<Menu.Trigger className={subMenuTriggerClass}>
<span>Speed</span>
<span className="flex items-center gap-1">
<span className="text-xs text-slate-400">{speed === '1' ? 'Normal' : `${speed}x`}</span>
<ChevronRight />
</span>
</Menu.Trigger>
<Menu.Content className={subMenuContentClass}>
<Menu.Back className={backButtonClass}>
<ChevronLeft />
Speed
</Menu.Back>
<Menu.RadioGroup aria-label="Speed" value={speed} onValueChange={setSpeed}>
{[
{ value: '0.5', label: '0.5x' },
{ value: '0.75', label: '0.75x' },
{ value: '1', label: 'Normal' },
{ value: '1.25', label: '1.25x' },
{ value: '1.5', label: '1.5x' },
{ value: '2', label: '2x' },
].map(({ value, label }) => (
<Menu.RadioItem key={value} value={value} className={radioItemClass}>
{speed === value && <RadioDot />}
{label}
</Menu.RadioItem>
))}
</Menu.RadioGroup>
</Menu.Content>
</Menu.Root>
{/* Speed submenu */}
<Menu.Root>
<Menu.Trigger className={subMenuTriggerClass}>
<span>Speed</span>
<span className="flex items-center gap-1">
<span className="text-xs text-slate-400">{speed === '1' ? 'Normal' : `${speed}x`}</span>
<ChevronRight />
</span>
</Menu.Trigger>
<Menu.Content className={subMenuContentClass}>
<Menu.Item className={backButtonClass}>
<ChevronLeft />
Speed
</Menu.Item>
<Menu.RadioGroup aria-label="Speed" value={speed} onValueChange={setSpeed}>
{[
{ value: '0.5', label: '0.5x' },
{ value: '0.75', label: '0.75x' },
{ value: '1', label: 'Normal' },
{ value: '1.25', label: '1.25x' },
{ value: '1.5', label: '1.5x' },
{ value: '2', label: '2x' },
].map(({ value, label }) => (
<Menu.RadioItem key={value} value={value} className={radioItemClass}>
{speed === value && <RadioDot />}
{label}
</Menu.RadioItem>
))}
</Menu.RadioGroup>
</Menu.Content>
</Menu.Root>

<Menu.Separator className={menuSeparatorClass} />
<Menu.Item onSelect={() => console.log('copy link')} className={menuItemClass}>
Copy link
</Menu.Item>
</Menu.View>
<Menu.Separator className={menuSeparatorClass} />
<Menu.Item onSelect={() => console.log('copy link')} className={menuItemClass}>
Copy link
</Menu.Item>
</Menu.Content>
</Menu.Root>
<p className="text-[0.8125rem] text-slate-500">
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/core/ui/menu/menu-core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export class MenuCore {
role: 'menu' as const,
tabIndex: -1,
// Root menus use the Popover API for dismiss and focus handling.
// Submenus render inline inside the parent viewport — no popover.
// Submenus render inline inside their parent menu — no popover.
...(!state.isSubmenu && { popover: 'manual' as const }),
};
}
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/core/ui/menu/menu-css-vars.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/** CSS custom property names for menu layout and submenu transitions. */
/** CSS custom property names for menu layout and positioning. */
export const MenuCSSVars = {
/** Width of the incoming submenu view (px). Set on root Content before each transition. */
/** Width of the active menu panel (px). */
width: '--media-menu-width',
/** Height of the incoming submenu view (px). Set on root Content before each transition. */
/** Height of the active menu panel (px). */
height: '--media-menu-height',
/** Viewport-constrained max width for the menu (px). Set from popover positioning. */
/** Viewport-constrained max width for the menu (px). */
availableWidth: '--media-menu-available-width',
/** Viewport-constrained max height for the menu (px). Set from popover positioning. */
/** Viewport-constrained max height for the menu (px). */
availableHeight: '--media-menu-available-height',
} as const;
3 changes: 1 addition & 2 deletions packages/core/src/dom/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ export * from './ui/dismiss-layer';
export * from './ui/event';
export * from './ui/input-action';
export * from './ui/menu/create-menu';
export * from './ui/menu/create-menu-view-transition';
export * from './ui/menu/menu-viewport-transition';
export * from './ui/menu/menu-size';
export * from './ui/popover/popover';
export type { PositioningCSSVars, PositioningOptions } from './ui/popover/popover-positioning';
export * from './ui/popover/popup-group';
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/dom/ui/dismiss-layer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { State, WritableState } from '@videojs/store';
import { listen } from '@videojs/utils/dom';

import type { TransitionState } from '../../core/ui/transition';
import type { TransitionApi } from './transition';
import type { TransitionApi, TransitionElement } from './transition';

export interface DismissLayerOptions {
/** Transition API for animated open/close. */
Expand All @@ -19,7 +19,7 @@ export interface DismissLayerApi {
/** Reactive transition state for platforms to subscribe to. */
input: State<TransitionState>;
/** Start the open transition. Returns animation promise, or `null` if already open or destroyed. */
open(): Promise<void> | null;
open(element?: TransitionElement): Promise<void> | null;
/** Start the close transition. Returns animation promise, or `null` if already closed or destroyed. */
close(element: HTMLElement | null): Promise<void> | null;
/** Lifecycle signal. Aborted on destroy. */
Expand All @@ -37,7 +37,7 @@ export function createDismissLayer(options: DismissLayerOptions): DismissLayerAp

// --- Open/Close ---

function open(): Promise<void> | null {
function open(element?: TransitionElement): Promise<void> | null {
if (abort.signal.aborted) return null;

const { active, status } = state.current;
Expand All @@ -48,7 +48,7 @@ export function createDismissLayer(options: DismissLayerOptions): DismissLayerAp
transition.cancel();
}

return transition.open();
return transition.open(element);
}

function close(element: HTMLElement | null): Promise<void> | null {
Expand Down
Loading
Loading