feat(packages): update menu group labels (#1643)

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<MenuContextValue>(MENU_CONTEXT_KEY);
 export const menuRadioGroupContext = createContext<MenuRadioGroupContextValue>(MENU_RADIO_GROUP_CONTEXT_KEY);
+export const menuGroupContext = createContext<MenuGroupContextValue>(MENU_GROUP_CONTEXT_KEY);

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<typeof menuGroupContext, MenuGroupHost>;
+  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();
+    };
+  }
+}

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();
   }
 }

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);
+  }
+}

packages/html/src/ui/menu/menu-label-element.ts

@@ -1,5 +0,0 @@
-import { MediaElement } from '../media-element';
-
-export class MenuLabelElement extends MediaElement {
-  static readonly tagName = 'media-menu-label';
-}

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) => {

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);

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);
     }