From c5f75166fd9e97cdec32335ae90bde46262d9465 Mon Sep 17 00:00:00 2001 From: Sam Potts Date: Tue, 2 Jun 2026 20:26:19 +1000 Subject: [PATCH] feat(packages): update menu group labels (#1643) --- apps/sandbox/templates/html-menu/main.ts | 4 +- apps/sandbox/templates/react-menu/main.tsx | 4 +- internal/design/ui/menus.md | 28 +++--- packages/html/src/define/ui/compounds.ts | 4 +- packages/html/src/define/ui/menu.ts | 4 +- packages/html/src/index.ts | 4 +- packages/html/src/ui/menu/context.ts | 6 ++ .../html/src/ui/menu/menu-group-controller.ts | 62 +++++++++++++ .../html/src/ui/menu/menu-group-element.ts | 15 +--- .../src/ui/menu/menu-group-label-element.ts | 51 +++++++++++ .../html/src/ui/menu/menu-label-element.ts | 5 -- .../src/ui/menu/menu-radio-group-element.ts | 13 ++- .../src/ui/menu/tests/menu-element.test.ts | 70 +++++++++++++-- .../playback-rate-options-element.ts | 7 +- .../presets/audio/minimal-skin.tailwind.tsx | 2 +- .../react/src/presets/audio/minimal-skin.tsx | 2 +- .../react/src/presets/audio/skin.tailwind.tsx | 2 +- packages/react/src/presets/audio/skin.tsx | 2 +- .../presets/video/minimal-skin.tailwind.tsx | 2 +- .../react/src/presets/video/minimal-skin.tsx | 2 +- .../react/src/presets/video/skin.tailwind.tsx | 2 +- packages/react/src/presets/video/skin.tsx | 2 +- packages/react/src/ui/menu/context.tsx | 16 ++++ packages/react/src/ui/menu/index.parts.ts | 2 +- .../react/src/ui/menu/menu-group-label.tsx | 41 +++++++++ packages/react/src/ui/menu/menu-group.tsx | 30 ++++--- packages/react/src/ui/menu/menu-label.tsx | 33 ------- .../react/src/ui/menu/menu-radio-group.tsx | 29 ++++--- .../react/src/ui/menu/tests/menu.test.tsx | 87 ++++++++++++++++++- packages/react/src/ui/menu/use-menu-group.tsx | 44 ++++++++++ .../tests/playback-rate-menu.test.tsx | 2 +- 31 files changed, 449 insertions(+), 128 deletions(-) create mode 100644 packages/html/src/ui/menu/menu-group-controller.ts create mode 100644 packages/html/src/ui/menu/menu-group-label-element.ts delete mode 100644 packages/html/src/ui/menu/menu-label-element.ts create mode 100644 packages/react/src/ui/menu/menu-group-label.tsx delete mode 100644 packages/react/src/ui/menu/menu-label.tsx create mode 100644 packages/react/src/ui/menu/use-menu-group.tsx diff --git a/apps/sandbox/templates/html-menu/main.ts b/apps/sandbox/templates/html-menu/main.ts index 6c86d5c3..b5bf115d 100644 --- a/apps/sandbox/templates/html-menu/main.ts +++ b/apps/sandbox/templates/html-menu/main.ts @@ -98,7 +98,7 @@ root.innerHTML = ` - Resolution + Resolution Auto 1080p 720p @@ -121,7 +121,7 @@ root.innerHTML = ` - Playback + Playback Loop Autoplay diff --git a/apps/sandbox/templates/react-menu/main.tsx b/apps/sandbox/templates/react-menu/main.tsx index 4eb514a7..f0a3395b 100644 --- a/apps/sandbox/templates/react-menu/main.tsx +++ b/apps/sandbox/templates/react-menu/main.tsx @@ -180,7 +180,7 @@ function App() { Quality - Resolution + Resolution {['auto', '1080p', '720p', '480p'].map((value) => ( {quality === value && } @@ -205,7 +205,7 @@ function App() { Settings - Playback + Playback {loop && } Loop diff --git a/internal/design/ui/menus.md b/internal/design/ui/menus.md index f05daea9..5a4f2a83 100644 --- a/internal/design/ui/menus.md +++ b/internal/design/ui/menus.md @@ -47,6 +47,7 @@ import { Menu } from '@videojs/react'; + Quality Auto 1080p 720p @@ -59,6 +60,7 @@ import { Menu } from '@videojs/react'; + Speed 0.5× Normal 2× @@ -98,6 +100,7 @@ import '@videojs/html/ui/menu'; + Quality Auto 1080p 720p @@ -107,6 +110,7 @@ import '@videojs/html/ui/menu'; + Speed 0.5× Normal 2× @@ -123,7 +127,7 @@ All parts are exported under `Menu.*` (React) or as `` elements (H ```ts import { Menu } from '@videojs/react'; // Menu.Root, Menu.Trigger, Menu.Content, Menu.Back, -// Menu.Item, Menu.Label, Menu.Separator, Menu.Group, +// Menu.Item, Menu.GroupLabel, Menu.Separator, Menu.Group, // Menu.RadioGroup, Menu.RadioItem, Menu.CheckboxItem, Menu.ItemIndicator ``` @@ -269,13 +273,13 @@ Standard menu item for actions. Activating fires `onSelect` and closes the menu. --- -#### Label +#### GroupLabel Non-interactive heading within a group. Not keyboard-navigable. **Props:** `render`. -**ARIA (automatic):** `role="presentation"`. +**ARIA (automatic):** Provides an `id` and registers it with the nearest `Group` or `RadioGroup`. The group receives `aria-labelledby` unless the consumer passed `aria-label` or `aria-labelledby` directly. --- @@ -295,10 +299,9 @@ Groups related items for assistive technology. | Prop | Type | Description | |------|------|-------------| -| `label` | `string` | Accessible label (`aria-label`). | | `render` | `RenderProp` | Custom render element. | -**ARIA (automatic):** `role="group"`, `aria-label`. +**ARIA (automatic):** `role="group"`, `aria-labelledby` from child `GroupLabel` when present. Consumers may pass `aria-label` or `aria-labelledby` directly. --- @@ -313,10 +316,9 @@ Single-selection group. Manages value state — controlled or uncontrolled. In a | `value` | `string` | — | Controlled selected value. | | `defaultValue` | `string` | — | Initial value (uncontrolled). | | `onValueChange` | `(value: string) => void` | — | Fired when selection changes. | -| `label` | `string` | — | Accessible group label. | | `render` | `RenderProp` | — | Custom render element. | -**ARIA (automatic):** `role="group"`, `aria-label`. +**ARIA (automatic):** `role="group"`, `aria-labelledby` from child `GroupLabel` when present. Consumers may pass `aria-label` or `aria-labelledby` directly. --- @@ -338,8 +340,6 @@ Item within a RadioGroup. Represents one selectable option. **Behavior:** In a submenu, selecting a RadioItem auto-pops back to the parent view after calling `onValueChange`. ---- - #### CheckboxItem Toggle item with independent checked/unchecked state. @@ -378,7 +378,7 @@ Visual indicator that renders when the parent RadioItem or CheckboxItem is check | View | `` | | Back | `` | | Item | `` | -| Label | `` | +| GroupLabel | `` | | Separator | `` | | Group | `` | | RadioGroup | `` | @@ -617,7 +617,8 @@ The menu follows the [WAI-ARIA Menu Pattern](https://www.w3.org/WAI/ARIA/apg/pat
-
+
+
Quality
Auto
1080p
@@ -634,7 +635,6 @@ The menu follows the [WAI-ARIA Menu Pattern](https://www.w3.org/WAI/ARIA/apg/pat | RadioItem | `menuitemradio` | | CheckboxItem | `menuitemcheckbox` | | Group / RadioGroup | `group` | -| Label | `presentation` | | Separator | `separator` | **Focus management:** @@ -781,7 +781,7 @@ menu-content.tsx menu-view.tsx menu-back.tsx menu-item.tsx -menu-label.tsx +menu-group-label.tsx menu-separator.tsx menu-group.tsx menu-radio-group.tsx @@ -797,7 +797,7 @@ menu-element.ts menu-view-element.ts menu-back-element.ts menu-item-element.ts -menu-label-element.ts +menu-group-label-element.ts menu-separator-element.ts menu-group-element.ts menu-radio-group-element.ts diff --git a/packages/html/src/define/ui/compounds.ts b/packages/html/src/define/ui/compounds.ts index 79f2dada..f1aa652e 100644 --- a/packages/html/src/define/ui/compounds.ts +++ b/packages/html/src/define/ui/compounds.ts @@ -8,9 +8,9 @@ import { MenuBackElement } from '../../ui/menu/menu-back-element'; import { MenuCheckboxItemElement } from '../../ui/menu/menu-checkbox-item-element'; import { MenuElement } from '../../ui/menu/menu-element'; import { MenuGroupElement } from '../../ui/menu/menu-group-element'; +import { MenuGroupLabelElement } from '../../ui/menu/menu-group-label-element'; import { MenuItemElement } from '../../ui/menu/menu-item-element'; import { MenuItemIndicatorElement } from '../../ui/menu/menu-item-indicator-element'; -import { MenuLabelElement } from '../../ui/menu/menu-label-element'; import { MenuRadioGroupElement } from '../../ui/menu/menu-radio-group-element'; import { MenuRadioItemElement } from '../../ui/menu/menu-radio-item-element'; import { MenuSeparatorElement } from '../../ui/menu/menu-separator-element'; @@ -45,7 +45,7 @@ export function defineMenu(): void { safeDefine(MenuElement); safeDefine(MenuBackElement); safeDefine(MenuItemElement); - safeDefine(MenuLabelElement); + safeDefine(MenuGroupLabelElement); safeDefine(MenuSeparatorElement); safeDefine(MenuGroupElement); safeDefine(MenuRadioGroupElement); diff --git a/packages/html/src/define/ui/menu.ts b/packages/html/src/define/ui/menu.ts index 0e795778..02632a79 100644 --- a/packages/html/src/define/ui/menu.ts +++ b/packages/html/src/define/ui/menu.ts @@ -2,9 +2,9 @@ import { MenuBackElement } from '../../ui/menu/menu-back-element'; import { MenuCheckboxItemElement } from '../../ui/menu/menu-checkbox-item-element'; import { MenuElement } from '../../ui/menu/menu-element'; import { MenuGroupElement } from '../../ui/menu/menu-group-element'; +import { MenuGroupLabelElement } from '../../ui/menu/menu-group-label-element'; import { MenuItemElement } from '../../ui/menu/menu-item-element'; import { MenuItemIndicatorElement } from '../../ui/menu/menu-item-indicator-element'; -import { MenuLabelElement } from '../../ui/menu/menu-label-element'; import { MenuRadioGroupElement } from '../../ui/menu/menu-radio-group-element'; import { MenuRadioItemElement } from '../../ui/menu/menu-radio-item-element'; import { MenuSeparatorElement } from '../../ui/menu/menu-separator-element'; @@ -18,7 +18,7 @@ declare global { [MenuElement.tagName]: MenuElement; [MenuBackElement.tagName]: MenuBackElement; [MenuItemElement.tagName]: MenuItemElement; - [MenuLabelElement.tagName]: MenuLabelElement; + [MenuGroupLabelElement.tagName]: MenuGroupLabelElement; [MenuSeparatorElement.tagName]: MenuSeparatorElement; [MenuGroupElement.tagName]: MenuGroupElement; [MenuRadioGroupElement.tagName]: MenuRadioGroupElement; diff --git a/packages/html/src/index.ts b/packages/html/src/index.ts index add2a55c..813a8a80 100644 --- a/packages/html/src/index.ts +++ b/packages/html/src/index.ts @@ -49,17 +49,19 @@ export * from './ui/media-element'; export { MediaUIElement } from './ui/media-ui-element'; export { type MenuContextValue, + type MenuGroupContextValue, type MenuRadioGroupContextValue, menuContext, + menuGroupContext, menuRadioGroupContext, } from './ui/menu/context'; export { MenuBackElement } from './ui/menu/menu-back-element'; export { MenuCheckboxItemElement } from './ui/menu/menu-checkbox-item-element'; export { MenuElement } from './ui/menu/menu-element'; export { MenuGroupElement } from './ui/menu/menu-group-element'; +export { MenuGroupLabelElement } from './ui/menu/menu-group-label-element'; export { MenuItemElement } from './ui/menu/menu-item-element'; export { MenuItemIndicatorElement } from './ui/menu/menu-item-indicator-element'; -export { MenuLabelElement } from './ui/menu/menu-label-element'; export { MenuRadioGroupElement } from './ui/menu/menu-radio-group-element'; export { MenuRadioItemElement } from './ui/menu/menu-radio-item-element'; export { MenuSeparatorElement } from './ui/menu/menu-separator-element'; diff --git a/packages/html/src/ui/menu/context.ts b/packages/html/src/ui/menu/context.ts index 810b914a..1408298c 100644 --- a/packages/html/src/ui/menu/context.ts +++ b/packages/html/src/ui/menu/context.ts @@ -16,8 +16,14 @@ export interface MenuRadioGroupContextValue { onValueChange: (value: string) => void; } +export interface MenuGroupContextValue { + registerLabel: (id: string) => () => void; +} + const MENU_CONTEXT_KEY = Symbol('@videojs/menu'); const MENU_RADIO_GROUP_CONTEXT_KEY = Symbol('@videojs/menu-radio-group'); +const MENU_GROUP_CONTEXT_KEY = Symbol('@videojs/menu-group'); export const menuContext = createContext(MENU_CONTEXT_KEY); export const menuRadioGroupContext = createContext(MENU_RADIO_GROUP_CONTEXT_KEY); +export const menuGroupContext = createContext(MENU_GROUP_CONTEXT_KEY); diff --git a/packages/html/src/ui/menu/menu-group-controller.ts b/packages/html/src/ui/menu/menu-group-controller.ts new file mode 100644 index 00000000..742bb2c7 --- /dev/null +++ b/packages/html/src/ui/menu/menu-group-controller.ts @@ -0,0 +1,62 @@ +import { applyElementProps } from '@videojs/core/dom'; +import { ContextProvider } from '@videojs/element/context'; + +import type { MediaElement } from '../media-element'; +import { menuGroupContext } from './context'; + +interface MenuGroupHost extends MediaElement { + requestUpdate(): void; +} + +export class MenuGroupController { + readonly #host: MenuGroupHost; + readonly #provider: ContextProvider; + readonly #contextValue = { + registerLabel: (id: string) => this.#registerLabel(id), + }; + + #labelId: string | undefined; + #appliedLabelId: string | undefined; + + constructor(host: MenuGroupHost) { + this.#host = host; + this.#provider = new ContextProvider(host, { + context: menuGroupContext, + initialValue: this.#contextValue, + }); + } + + applyProps(): void { + const currentLabelledBy = this.#host.getAttribute('aria-labelledby') ?? undefined; + const hasExplicitLabelledBy = currentLabelledBy !== undefined && currentLabelledBy !== this.#appliedLabelId; + const hasExplicitLabel = this.#host.hasAttribute('aria-label') || hasExplicitLabelledBy; + + if (hasExplicitLabel) { + if (this.#appliedLabelId && currentLabelledBy === this.#appliedLabelId) { + this.#host.removeAttribute('aria-labelledby'); + } + + this.#appliedLabelId = undefined; + applyElementProps(this.#host, { role: 'group' }); + return; + } + + this.#appliedLabelId = this.#labelId; + applyElementProps(this.#host, { + role: 'group', + 'aria-labelledby': this.#labelId, + }); + } + + #registerLabel(id: string): () => void { + this.#labelId = id; + this.#provider.setValue(this.#contextValue); + this.#host.requestUpdate(); + + return () => { + if (this.#labelId !== id) return; + this.#labelId = undefined; + this.#host.requestUpdate(); + }; + } +} diff --git a/packages/html/src/ui/menu/menu-group-element.ts b/packages/html/src/ui/menu/menu-group-element.ts index d7c28f6b..d29428a8 100644 --- a/packages/html/src/ui/menu/menu-group-element.ts +++ b/packages/html/src/ui/menu/menu-group-element.ts @@ -1,23 +1,16 @@ -import { applyElementProps } from '@videojs/core/dom'; -import type { PropertyDeclarationMap, PropertyValues } from '@videojs/element'; +import type { PropertyValues } from '@videojs/element'; import { MediaElement } from '../media-element'; +import { MenuGroupController } from './menu-group-controller'; export class MenuGroupElement extends MediaElement { static readonly tagName = 'media-menu-group'; - static override properties = { - label: { type: String }, - } satisfies PropertyDeclarationMap<'label'>; - - label: string | undefined = undefined; + readonly #group = new MenuGroupController(this); protected override update(_changed: PropertyValues): void { super.update(_changed); - applyElementProps(this, { - role: 'group', - 'aria-label': this.label, - }); + this.#group.applyProps(); } } diff --git a/packages/html/src/ui/menu/menu-group-label-element.ts b/packages/html/src/ui/menu/menu-group-label-element.ts new file mode 100644 index 00000000..eee09f31 --- /dev/null +++ b/packages/html/src/ui/menu/menu-group-label-element.ts @@ -0,0 +1,51 @@ +import type { PropertyValues } from '@videojs/element'; +import { ContextConsumer } from '@videojs/element/context'; + +import { MediaElement } from '../media-element'; +import { menuGroupContext } from './context'; + +let idCounter = 0; + +export class MenuGroupLabelElement extends MediaElement { + static readonly tagName = 'media-menu-group-label'; + + readonly #groupCtx = new ContextConsumer(this, { context: menuGroupContext, subscribe: true }); + readonly #generatedId = `vjs-menu-group-label-${idCounter++}`; + + #cleanupRegistration: (() => void) | null = null; + #registeredId: string | null = null; + + override disconnectedCallback(): void { + super.disconnectedCallback(); + this.#cleanupRegistration?.(); + this.#cleanupRegistration = null; + this.#registeredId = null; + } + + protected override update(_changed: PropertyValues): void { + super.update(_changed); + + if (!this.id) { + this.id = this.#generatedId; + } + + this.#registerLabel(); + } + + #registerLabel(): void { + const groupCtx = this.#groupCtx.value; + + if (!groupCtx) { + this.#cleanupRegistration?.(); + this.#cleanupRegistration = null; + this.#registeredId = null; + return; + } + + if (this.#registeredId === this.id) return; + + this.#cleanupRegistration?.(); + this.#registeredId = this.id; + this.#cleanupRegistration = groupCtx.registerLabel(this.id); + } +} diff --git a/packages/html/src/ui/menu/menu-label-element.ts b/packages/html/src/ui/menu/menu-label-element.ts deleted file mode 100644 index 299a0b50..00000000 --- a/packages/html/src/ui/menu/menu-label-element.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { MediaElement } from '../media-element'; - -export class MenuLabelElement extends MediaElement { - static readonly tagName = 'media-menu-label'; -} diff --git a/packages/html/src/ui/menu/menu-radio-group-element.ts b/packages/html/src/ui/menu/menu-radio-group-element.ts index d3d7fe46..26ce75eb 100644 --- a/packages/html/src/ui/menu/menu-radio-group-element.ts +++ b/packages/html/src/ui/menu/menu-radio-group-element.ts @@ -1,30 +1,27 @@ -import { applyElementProps } from '@videojs/core/dom'; import type { PropertyDeclarationMap, PropertyValues } from '@videojs/element'; import { ContextProvider } from '@videojs/element/context'; import { MediaElement } from '../media-element'; import { menuRadioGroupContext } from './context'; +import { MenuGroupController } from './menu-group-controller'; export class MenuRadioGroupElement extends MediaElement { static readonly tagName: string = 'media-menu-radio-group'; static override properties = { value: { type: String }, - label: { type: String }, - } satisfies PropertyDeclarationMap<'value' | 'label'>; + } satisfies PropertyDeclarationMap<'value'>; value = ''; - label: string | undefined = undefined; readonly #provider = new ContextProvider(this, { context: menuRadioGroupContext }); + readonly #group = new MenuGroupController(this); protected override update(_changed: PropertyValues): void { super.update(_changed); - applyElementProps(this, { - role: 'group', - 'aria-label': this.label, - }); + this.#group.applyProps(); + this.#provider.setValue({ value: this.value, onValueChange: (next: string) => { diff --git a/packages/html/src/ui/menu/tests/menu-element.test.ts b/packages/html/src/ui/menu/tests/menu-element.test.ts index a3f57149..ebf3b648 100644 --- a/packages/html/src/ui/menu/tests/menu-element.test.ts +++ b/packages/html/src/ui/menu/tests/menu-element.test.ts @@ -11,9 +11,9 @@ import { MenuBackElement } from '../menu-back-element'; import { MenuCheckboxItemElement } from '../menu-checkbox-item-element'; import { MenuElement } from '../menu-element'; import { MenuGroupElement } from '../menu-group-element'; +import { MenuGroupLabelElement } from '../menu-group-label-element'; import { MenuItemElement } from '../menu-item-element'; import { MenuItemIndicatorElement } from '../menu-item-indicator-element'; -import { MenuLabelElement } from '../menu-label-element'; import { MenuRadioGroupElement } from '../menu-radio-group-element'; import { MenuRadioItemElement } from '../menu-radio-item-element'; import { MenuSeparatorElement } from '../menu-separator-element'; @@ -113,7 +113,7 @@ afterEach(() => { describe('MenuElement', () => { it('scopes menu state data attributes to menu elements', async () => { const root = createElement(MenuElement); - const label = createElement(MenuLabelElement); + const label = createElement(MenuGroupLabelElement); const group = createElement(MenuGroupElement); const item = createElement(MenuItemElement); const checkboxItem = createElement(MenuCheckboxItemElement); @@ -131,10 +131,8 @@ describe('MenuElement', () => { root.side = 'top'; root.align = 'end'; label.textContent = 'Playback'; - group.label = 'Playback'; item.textContent = 'Copy link'; checkboxItem.textContent = 'Autoplay'; - radioGroup.label = 'Quality'; radioGroup.value = 'auto'; radioItem.value = 'auto'; radioItem.textContent = 'Auto'; @@ -148,10 +146,10 @@ describe('MenuElement', () => { radioItem.append(indicator); radioGroup.append(radioItem); - group.append(item, checkboxItem, radioGroup); + group.append(label, item, checkboxItem, radioGroup); rootView.append(trigger); child.append(back, childItem); - root.append(label, group, separator, rootView, child); + root.append(group, separator, rootView, child); document.body.append(root); await root.updateComplete; @@ -367,6 +365,66 @@ describe('MenuElement', () => { ); }); + it('wires group labels to group elements with aria-labelledby', async () => { + const root = createElement(MenuElement); + const group = createElement(MenuGroupElement); + const radioGroup = createElement(MenuRadioGroupElement); + const groupLabel = createElement(MenuGroupLabelElement); + const radioLabel = createElement(MenuGroupLabelElement); + + root.open = true; + groupLabel.textContent = 'Playback'; + radioLabel.textContent = 'Quality'; + + group.append(groupLabel); + radioGroup.append(radioLabel); + root.append(group, radioGroup); + document.body.append(root); + + await root.updateComplete; + await group.updateComplete; + await radioGroup.updateComplete; + await groupLabel.updateComplete; + await radioLabel.updateComplete; + + await waitForAssertion(() => { + expect(group.getAttribute('aria-labelledby')).toBe(groupLabel.id); + expect(radioGroup.getAttribute('aria-labelledby')).toBe(radioLabel.id); + }); + }); + + it('lets explicit group labels override generated aria-labelledby', async () => { + const root = createElement(MenuElement); + const ariaLabelGroup = createElement(MenuGroupElement); + const ariaLabelledByGroup = createElement(MenuRadioGroupElement); + const ariaLabel = createElement(MenuGroupLabelElement); + const ariaLabelledByLabel = createElement(MenuGroupLabelElement); + + root.open = true; + ariaLabelGroup.setAttribute('aria-label', 'Playback'); + ariaLabelledByGroup.setAttribute('aria-labelledby', 'external-label'); + + ariaLabelGroup.append(ariaLabel); + ariaLabelledByGroup.append(ariaLabelledByLabel); + root.append(ariaLabelGroup, ariaLabelledByGroup); + document.body.append(root); + + await root.updateComplete; + await ariaLabelGroup.updateComplete; + await ariaLabelledByGroup.updateComplete; + await ariaLabel.updateComplete; + await ariaLabelledByLabel.updateComplete; + + await waitForAssertion(() => { + expect(ariaLabel.id).not.toBe(''); + expect(ariaLabelledByLabel.id).not.toBe(''); + }); + + expect(ariaLabelGroup.getAttribute('aria-label')).toBe('Playback'); + expect(ariaLabelGroup.hasAttribute('aria-labelledby')).toBe(false); + expect(ariaLabelledByGroup.getAttribute('aria-labelledby')).toBe('external-label'); + }); + it('highlights pointer-entered items without moving focus', async () => { const root = createElement(MenuElement); const item = createElement(MenuItemElement); diff --git a/packages/html/src/ui/playback-rate-menu/playback-rate-options-element.ts b/packages/html/src/ui/playback-rate-menu/playback-rate-options-element.ts index 4b37fda4..32056fe2 100644 --- a/packages/html/src/ui/playback-rate-menu/playback-rate-options-element.ts +++ b/packages/html/src/ui/playback-rate-menu/playback-rate-options-element.ts @@ -14,7 +14,7 @@ export class PlaybackRateOptionsElement extends MenuRadioGroupElement { static override properties = { ...MenuRadioGroupElement.properties, disabled: { type: Boolean }, - } satisfies PropertyDeclarationMap<'value' | 'label' | 'disabled'>; + } satisfies PropertyDeclarationMap<'value' | 'disabled'>; disabled = false; formatRate = PlaybackRateMenuCore.defaultProps.formatRate; @@ -53,7 +53,10 @@ export class PlaybackRateOptionsElement extends MenuRadioGroupElement { state = this.#core.getState(); this.value = this.#core.getRateValue(state.rate); - this.label = this.label || 'Playback rate'; + if (!this.hasAttribute('aria-label') && !this.hasAttribute('aria-labelledby')) { + this.setAttribute('aria-label', 'Playback rate'); + } + this.#syncContent(state); } diff --git a/packages/react/src/presets/audio/minimal-skin.tailwind.tsx b/packages/react/src/presets/audio/minimal-skin.tailwind.tsx index 536816b8..4757b1d4 100644 --- a/packages/react/src/presets/audio/minimal-skin.tailwind.tsx +++ b/packages/react/src/presets/audio/minimal-skin.tailwind.tsx @@ -124,7 +124,7 @@ function PlaybackRateMenuItems(): ReactNode { const { options, setValue, value } = usePlaybackRateMenu(); return ( - + {options.map((option) => ( {option.label} diff --git a/packages/react/src/presets/audio/minimal-skin.tsx b/packages/react/src/presets/audio/minimal-skin.tsx index b49dadb5..f2c35b29 100644 --- a/packages/react/src/presets/audio/minimal-skin.tsx +++ b/packages/react/src/presets/audio/minimal-skin.tsx @@ -73,7 +73,7 @@ function PlaybackRateMenuItems(): ReactNode { const { options, setValue, value } = usePlaybackRateMenu(); return ( - + {options.map((option) => ( {option.label} diff --git a/packages/react/src/presets/audio/skin.tailwind.tsx b/packages/react/src/presets/audio/skin.tailwind.tsx index 430866d0..2da1c678 100644 --- a/packages/react/src/presets/audio/skin.tailwind.tsx +++ b/packages/react/src/presets/audio/skin.tailwind.tsx @@ -126,7 +126,7 @@ function PlaybackRateMenuItems(): ReactNode { const { options, setValue, value } = usePlaybackRateMenu(); return ( - + {options.map((option) => ( {option.label} diff --git a/packages/react/src/presets/audio/skin.tsx b/packages/react/src/presets/audio/skin.tsx index 369f1a0d..97db7275 100644 --- a/packages/react/src/presets/audio/skin.tsx +++ b/packages/react/src/presets/audio/skin.tsx @@ -73,7 +73,7 @@ function PlaybackRateMenuItems(): ReactNode { const { options, setValue, value } = usePlaybackRateMenu(); return ( - + {options.map((option) => ( {option.label} diff --git a/packages/react/src/presets/video/minimal-skin.tailwind.tsx b/packages/react/src/presets/video/minimal-skin.tailwind.tsx index 2ca3c031..9c1be2dc 100644 --- a/packages/react/src/presets/video/minimal-skin.tailwind.tsx +++ b/packages/react/src/presets/video/minimal-skin.tailwind.tsx @@ -161,7 +161,7 @@ function PlaybackRateMenuItems(): ReactNode { const { options, setValue, value } = usePlaybackRateMenu(); return ( - + {options.map((option) => ( {option.label} diff --git a/packages/react/src/presets/video/minimal-skin.tsx b/packages/react/src/presets/video/minimal-skin.tsx index cc4e9293..4995d502 100644 --- a/packages/react/src/presets/video/minimal-skin.tsx +++ b/packages/react/src/presets/video/minimal-skin.tsx @@ -102,7 +102,7 @@ function PlaybackRateMenuItems(): ReactNode { const { options, setValue, value } = usePlaybackRateMenu(); return ( - + {options.map((option) => ( {option.label} diff --git a/packages/react/src/presets/video/skin.tailwind.tsx b/packages/react/src/presets/video/skin.tailwind.tsx index 08bf5814..bf39ede1 100644 --- a/packages/react/src/presets/video/skin.tailwind.tsx +++ b/packages/react/src/presets/video/skin.tailwind.tsx @@ -161,7 +161,7 @@ function PlaybackRateMenuItems(): ReactNode { const { options, setValue, value } = usePlaybackRateMenu(); return ( - + {options.map((option) => ( {option.label} diff --git a/packages/react/src/presets/video/skin.tsx b/packages/react/src/presets/video/skin.tsx index fea890bb..6d129612 100644 --- a/packages/react/src/presets/video/skin.tsx +++ b/packages/react/src/presets/video/skin.tsx @@ -102,7 +102,7 @@ function PlaybackRateMenuItems(): ReactNode { const { options, setValue, value } = usePlaybackRateMenu(); return ( - + {options.map((option) => ( {option.label} diff --git a/packages/react/src/ui/menu/context.tsx b/packages/react/src/ui/menu/context.tsx index dd9657fd..1ba7e4d8 100644 --- a/packages/react/src/ui/menu/context.tsx +++ b/packages/react/src/ui/menu/context.tsx @@ -60,6 +60,22 @@ export function useSubMenuContext(): SubMenuContextValue | null { return useContext(SubMenuContext); } +// --------------------------------------------------------------------------- +// Group context — shared by group-like parts and MenuGroupLabel +// --------------------------------------------------------------------------- + +export interface MenuGroupContextValue { + registerLabel: (id: string) => () => void; +} + +const MenuGroupContext = createContext(null); + +export const MenuGroupContextProvider = MenuGroupContext.Provider; + +export function useMenuGroupContext(): MenuGroupContextValue | null { + return useContext(MenuGroupContext); +} + // --------------------------------------------------------------------------- // Radio group context — shared between MenuRadioGroup and MenuRadioItem // --------------------------------------------------------------------------- diff --git a/packages/react/src/ui/menu/index.parts.ts b/packages/react/src/ui/menu/index.parts.ts index 56f33bc8..3c6561ad 100644 --- a/packages/react/src/ui/menu/index.parts.ts +++ b/packages/react/src/ui/menu/index.parts.ts @@ -5,12 +5,12 @@ export { } from './menu-checkbox-item'; export { MenuContent as Content, type MenuContentProps as ContentProps } from './menu-content'; export { MenuGroup as Group, type MenuGroupProps as GroupProps } from './menu-group'; +export { MenuGroupLabel as GroupLabel, type MenuGroupLabelProps as GroupLabelProps } from './menu-group-label'; export { MenuItem as Item, type MenuItemProps as ItemProps } from './menu-item'; export { MenuItemIndicator as ItemIndicator, type MenuItemIndicatorProps as ItemIndicatorProps, } from './menu-item-indicator'; -export { MenuLabel as Label, type MenuLabelProps as LabelProps } from './menu-label'; export { MenuRadioGroup as RadioGroup, type MenuRadioGroupProps as RadioGroupProps } from './menu-radio-group'; export { MenuRadioItem as RadioItem, type MenuRadioItemProps as RadioItemProps } from './menu-radio-item'; export { MenuRoot as Root, type MenuRootProps as RootProps } from './menu-root'; diff --git a/packages/react/src/ui/menu/menu-group-label.tsx b/packages/react/src/ui/menu/menu-group-label.tsx new file mode 100644 index 00000000..5fe5cfa7 --- /dev/null +++ b/packages/react/src/ui/menu/menu-group-label.tsx @@ -0,0 +1,41 @@ +'use client'; + +import type { MenuState } from '@videojs/core'; +import { forwardRef, useLayoutEffect } from 'react'; + +import type { UIComponentProps } from '../../utils/types'; +import { renderElement } from '../../utils/use-render'; +import { useSafeId } from '../../utils/use-safe-id'; +import { useMenuContext, useMenuGroupContext } from './context'; + +export interface MenuGroupLabelProps extends UIComponentProps<'div', MenuState> {} + +/** Non-interactive label for a group of items. Renders a `
`. */ +export const MenuGroupLabel = forwardRef(function MenuGroupLabel( + { render, className, style, id: idProp, ...elementProps }, + forwardedRef +) { + const { state } = useMenuContext(); + const group = useMenuGroupContext(); + const generatedId = useSafeId('menu-group-label'); + const id = idProp ?? generatedId; + + useLayoutEffect(() => { + return group?.registerLabel(id); + }, [group, id]); + + return renderElement( + 'div', + { render, className, style }, + { + state, + ref: [forwardedRef], + props: [{ id }, elementProps], + } + ); +}); + +export namespace MenuGroupLabel { + export type Props = MenuGroupLabelProps; + export type State = MenuState; +} diff --git a/packages/react/src/ui/menu/menu-group.tsx b/packages/react/src/ui/menu/menu-group.tsx index 8c58c47c..adf0c6ea 100644 --- a/packages/react/src/ui/menu/menu-group.tsx +++ b/packages/react/src/ui/menu/menu-group.tsx @@ -6,27 +6,31 @@ import { forwardRef } from 'react'; import type { UIComponentProps } from '../../utils/types'; import { renderElement } from '../../utils/use-render'; import { useMenuContext } from './context'; +import { getMenuGroupProps, MenuGroupProvider } from './use-menu-group'; -export interface MenuGroupProps extends UIComponentProps<'div', MenuState> { - /** Accessible label for the group. */ - label?: string; -} +export interface MenuGroupProps extends UIComponentProps<'div', MenuState> {} /** Groups related menu items. Renders a `
` with `role="group"`. */ export const MenuGroup = forwardRef(function MenuGroup( - { render, className, style, label, ...elementProps }, + { render, className, style, ...elementProps }, forwardedRef ) { const { state } = useMenuContext(); - return renderElement( - 'div', - { render, className, style }, - { - state, - ref: [forwardedRef], - props: [{ role: 'group' as const, 'aria-label': label }, elementProps], - } + return ( + + {(labelId) => + renderElement( + 'div', + { render, className, style }, + { + state, + ref: [forwardedRef], + props: [getMenuGroupProps(labelId, elementProps), elementProps], + } + ) + } + ); }); diff --git a/packages/react/src/ui/menu/menu-label.tsx b/packages/react/src/ui/menu/menu-label.tsx deleted file mode 100644 index 48f8b1f0..00000000 --- a/packages/react/src/ui/menu/menu-label.tsx +++ /dev/null @@ -1,33 +0,0 @@ -'use client'; - -import type { MenuState } from '@videojs/core'; -import { forwardRef } from 'react'; - -import type { UIComponentProps } from '../../utils/types'; -import { renderElement } from '../../utils/use-render'; -import { useMenuContext } from './context'; - -export interface MenuLabelProps extends UIComponentProps<'div', MenuState> {} - -/** Non-interactive label for a group of items. Renders a `
`. */ -export const MenuLabel = forwardRef(function MenuLabel( - { render, className, style, ...elementProps }, - forwardedRef -) { - const { state } = useMenuContext(); - - return renderElement( - 'div', - { render, className, style }, - { - state, - ref: [forwardedRef], - props: [elementProps], - } - ); -}); - -export namespace MenuLabel { - export type Props = MenuLabelProps; - export type State = MenuState; -} diff --git a/packages/react/src/ui/menu/menu-radio-group.tsx b/packages/react/src/ui/menu/menu-radio-group.tsx index 764b1f55..e3b0e8ba 100644 --- a/packages/react/src/ui/menu/menu-radio-group.tsx +++ b/packages/react/src/ui/menu/menu-radio-group.tsx @@ -6,35 +6,38 @@ import { forwardRef } from 'react'; import type { UIComponentProps } from '../../utils/types'; import { renderElement } from '../../utils/use-render'; import { MenuRadioGroupContextProvider, useMenuContext } from './context'; +import { getMenuGroupProps, MenuGroupProvider } from './use-menu-group'; export interface MenuRadioGroupProps extends UIComponentProps<'div', MenuState> { /** The currently selected value. */ value: string; /** Called when the user selects a radio item. */ onValueChange: (value: string) => void; - /** Accessible label for the group. */ - label?: string; } /** A group of mutually exclusive radio items. Renders a `
` with `role="group"`. */ export const MenuRadioGroup = forwardRef(function MenuRadioGroup( - { render, className, style, value, onValueChange, label, ...elementProps }, + { render, className, style, value, onValueChange, ...elementProps }, forwardedRef ) { const { state } = useMenuContext(); return ( - - {renderElement( - 'div', - { render, className, style }, - { - state, - ref: [forwardedRef], - props: [{ role: 'group' as const, 'aria-label': label }, elementProps], - } + + {(labelId) => ( + + {renderElement( + 'div', + { render, className, style }, + { + state, + ref: [forwardedRef], + props: [getMenuGroupProps(labelId, elementProps), elementProps], + } + )} + )} - + ); }); diff --git a/packages/react/src/ui/menu/tests/menu.test.tsx b/packages/react/src/ui/menu/tests/menu.test.tsx index 1649dd9b..16aa0f4d 100644 --- a/packages/react/src/ui/menu/tests/menu.test.tsx +++ b/packages/react/src/ui/menu/tests/menu.test.tsx @@ -7,9 +7,9 @@ import { MenuBack } from '../menu-back'; import { MenuCheckboxItem } from '../menu-checkbox-item'; import { MenuContent } from '../menu-content'; import { MenuGroup } from '../menu-group'; +import { MenuGroupLabel } from '../menu-group-label'; import { MenuItem } from '../menu-item'; import { MenuItemIndicator } from '../menu-item-indicator'; -import { MenuLabel } from '../menu-label'; import { MenuRadioGroup } from '../menu-radio-group'; import { MenuRadioItem } from '../menu-radio-item'; import { MenuRoot } from '../menu-root'; @@ -253,6 +253,56 @@ function CheckboxFixture({ ); } +function GroupLabelFixture() { + return ( + + Settings + + + Playback + Copy link + + + + ); +} + +function RadioGroupLabelFixture() { + return ( + + Settings + + + Quality + Auto + + + + ); +} + +function ExplicitGroupLabelFixture() { + return ( + + Settings + + + Ignored + + + Ignored + Auto + + + + ); +} + function FocusOutFixture({ onRootOpenChange }: { onRootOpenChange: NonNullable }) { return ( <> @@ -285,13 +335,13 @@ describe('MenuContent', () => { Settings - Playback - + + Playback Copy link Autoplay - + Auto @@ -653,6 +703,35 @@ describe('MenuContent', () => { }); }); + it('wires GroupLabel to Group with aria-labelledby', async () => { + render(); + + await waitFor(() => { + expect(screen.getByTestId('group').getAttribute('aria-labelledby')).toBe(screen.getByTestId('label').id); + }); + }); + + it('wires GroupLabel to RadioGroup with aria-labelledby', async () => { + render(); + + await waitFor(() => { + expect(screen.getByTestId('group').getAttribute('aria-labelledby')).toBe(screen.getByTestId('label').id); + }); + }); + + it('lets explicit group labels override generated aria-labelledby', async () => { + render(); + + await waitFor(() => { + expect(screen.getByTestId('aria-label-label').id).not.toBe(''); + expect(screen.getByTestId('aria-labelledby-label').id).not.toBe(''); + }); + + expect(screen.getByTestId('aria-label-group').getAttribute('aria-label')).toBe('Playback'); + expect(screen.getByTestId('aria-label-group').hasAttribute('aria-labelledby')).toBe(false); + expect(screen.getByTestId('aria-labelledby-group').getAttribute('aria-labelledby')).toBe('external-label'); + }); + it('keeps the menu open when a checkbox item is toggled', () => { const onCheckedChange = vi.fn(); const onRootOpenChange = vi.fn(); diff --git a/packages/react/src/ui/menu/use-menu-group.tsx b/packages/react/src/ui/menu/use-menu-group.tsx new file mode 100644 index 00000000..42c5712b --- /dev/null +++ b/packages/react/src/ui/menu/use-menu-group.tsx @@ -0,0 +1,44 @@ +'use client'; + +import { type ReactElement, useCallback, useMemo, useState } from 'react'; + +import { MenuGroupContextProvider } from './context'; + +interface MenuGroupProviderProps { + children: (labelId: string | undefined) => ReactElement | null; +} + +interface MenuGroupElementProps { + 'aria-label'?: unknown; + 'aria-labelledby'?: unknown; +} + +function hasExplicitLabel(elementProps: MenuGroupElementProps): boolean { + return elementProps['aria-label'] !== undefined || elementProps['aria-labelledby'] !== undefined; +} + +export function getMenuGroupProps( + labelId: string | undefined, + elementProps: MenuGroupElementProps +): { role: 'group'; 'aria-labelledby'?: string | undefined } { + return { + role: 'group', + 'aria-labelledby': hasExplicitLabel(elementProps) ? undefined : labelId, + }; +} + +export function MenuGroupProvider({ children }: MenuGroupProviderProps): ReactElement | null { + const [labelId, setLabelId] = useState(); + + const registerLabel = useCallback((id: string) => { + setLabelId(id); + + return () => { + setLabelId((current) => (current === id ? undefined : current)); + }; + }, []); + + const value = useMemo(() => ({ registerLabel }), [registerLabel]); + + return {children(labelId)}; +} diff --git a/packages/react/src/ui/playback-rate-menu/tests/playback-rate-menu.test.tsx b/packages/react/src/ui/playback-rate-menu/tests/playback-rate-menu.test.tsx index f0d0fbf0..78f469db 100644 --- a/packages/react/src/ui/playback-rate-menu/tests/playback-rate-menu.test.tsx +++ b/packages/react/src/ui/playback-rate-menu/tests/playback-rate-menu.test.tsx @@ -38,7 +38,7 @@ function PlaybackRateMenuItems(): ReactNode { const { options, setValue, value } = usePlaybackRateMenu(); return ( - + {options.map((option) => ( {option.label}