From 01a2115aabe201d8aa293a4ad30badd7e1b69666 Mon Sep 17 00:00:00 2001 From: Sam Potts Date: Fri, 29 May 2026 16:26:47 +1000 Subject: [PATCH] fix(packages): scope menu data attributes (#1628) --- .../html/src/ui/menu/menu-back-element.ts | 4 +- .../src/ui/menu/menu-checkbox-item-element.ts | 4 +- packages/html/src/ui/menu/menu-element.ts | 3 + .../html/src/ui/menu/menu-group-element.ts | 9 +- .../html/src/ui/menu/menu-item-element.ts | 4 +- .../html/src/ui/menu/menu-label-element.ts | 10 +- .../src/ui/menu/menu-radio-group-element.ts | 11 +- .../src/ui/menu/menu-radio-item-element.ts | 4 +- .../src/ui/menu/menu-separator-element.ts | 9 +- .../src/ui/menu/tests/menu-element.test.ts | 104 ++++++++++++++++++ packages/react/src/ui/menu/menu-back.tsx | 1 - .../react/src/ui/menu/menu-checkbox-item.tsx | 3 +- packages/react/src/ui/menu/menu-content.tsx | 1 - packages/react/src/ui/menu/menu-group.tsx | 3 +- .../react/src/ui/menu/menu-item-indicator.tsx | 3 +- packages/react/src/ui/menu/menu-item.tsx | 3 +- packages/react/src/ui/menu/menu-label.tsx | 3 +- .../react/src/ui/menu/menu-radio-group.tsx | 3 +- .../react/src/ui/menu/menu-radio-item.tsx | 3 +- packages/react/src/ui/menu/menu-separator.tsx | 3 +- packages/react/src/ui/menu/menu-trigger.tsx | 4 +- .../react/src/ui/menu/tests/menu.test.tsx | 92 ++++++++++++++++ .../react/src/utils/tests/use-render.test.tsx | 27 +++-- packages/react/src/utils/use-render.tsx | 5 +- 24 files changed, 237 insertions(+), 79 deletions(-) diff --git a/packages/html/src/ui/menu/menu-back-element.ts b/packages/html/src/ui/menu/menu-back-element.ts index b3d4a375..754f8708 100644 --- a/packages/html/src/ui/menu/menu-back-element.ts +++ b/packages/html/src/ui/menu/menu-back-element.ts @@ -1,4 +1,4 @@ -import { applyElementProps, applyStateDataAttrs } from '@videojs/core/dom'; +import { applyElementProps } from '@videojs/core/dom'; import type { PropertyDeclarationMap, PropertyValues } from '@videojs/element'; import { ContextConsumer } from '@videojs/element/context'; @@ -57,7 +57,5 @@ export class MenuBackElement extends MediaElement { role: 'button', 'aria-label': this.label, }); - - if (ctx) applyStateDataAttrs(this, ctx.state, ctx.stateAttrMap); } } diff --git a/packages/html/src/ui/menu/menu-checkbox-item-element.ts b/packages/html/src/ui/menu/menu-checkbox-item-element.ts index 78b23c3e..bd2bd1b0 100644 --- a/packages/html/src/ui/menu/menu-checkbox-item-element.ts +++ b/packages/html/src/ui/menu/menu-checkbox-item-element.ts @@ -1,4 +1,4 @@ -import { applyElementProps, applyStateDataAttrs } from '@videojs/core/dom'; +import { applyElementProps } from '@videojs/core/dom'; import type { PropertyDeclarationMap, PropertyValues } from '@videojs/element'; import { ContextConsumer } from '@videojs/element/context'; @@ -72,7 +72,5 @@ export class MenuCheckboxItemElement extends MediaElement { 'aria-checked': String(this.checked), 'aria-disabled': this.disabled ? 'true' : undefined, }); - - applyStateDataAttrs(this, ctx.state, ctx.stateAttrMap); } } diff --git a/packages/html/src/ui/menu/menu-element.ts b/packages/html/src/ui/menu/menu-element.ts index 83fabaa0..d699b0fc 100644 --- a/packages/html/src/ui/menu/menu-element.ts +++ b/packages/html/src/ui/menu/menu-element.ts @@ -282,6 +282,9 @@ export class MenuElement extends MediaElement { // Apply base submenu attributes regardless of phase. const transitionState = this.#menuViewTransition.input.current; + this.removeAttribute(MenuDataAttrs.side); + this.removeAttribute(MenuDataAttrs.align); + applyElementProps(this, { ...getMenuViewTransitionAttrs(transitionState), role: 'menu', diff --git a/packages/html/src/ui/menu/menu-group-element.ts b/packages/html/src/ui/menu/menu-group-element.ts index d566f12f..d7c28f6b 100644 --- a/packages/html/src/ui/menu/menu-group-element.ts +++ b/packages/html/src/ui/menu/menu-group-element.ts @@ -1,9 +1,7 @@ -import { applyElementProps, applyStateDataAttrs } from '@videojs/core/dom'; +import { applyElementProps } from '@videojs/core/dom'; import type { PropertyDeclarationMap, PropertyValues } from '@videojs/element'; -import { ContextConsumer } from '@videojs/element/context'; import { MediaElement } from '../media-element'; -import { menuContext } from './context'; export class MenuGroupElement extends MediaElement { static readonly tagName = 'media-menu-group'; @@ -14,8 +12,6 @@ export class MenuGroupElement extends MediaElement { label: string | undefined = undefined; - readonly #ctx = new ContextConsumer(this, { context: menuContext, subscribe: true }); - protected override update(_changed: PropertyValues): void { super.update(_changed); @@ -23,8 +19,5 @@ export class MenuGroupElement extends MediaElement { role: 'group', 'aria-label': this.label, }); - - const ctx = this.#ctx.value; - if (ctx) applyStateDataAttrs(this, ctx.state, ctx.stateAttrMap); } } diff --git a/packages/html/src/ui/menu/menu-item-element.ts b/packages/html/src/ui/menu/menu-item-element.ts index 6ce24a1c..958eddd6 100644 --- a/packages/html/src/ui/menu/menu-item-element.ts +++ b/packages/html/src/ui/menu/menu-item-element.ts @@ -1,4 +1,4 @@ -import { applyElementProps, applyStateDataAttrs, completeMenuItemSelection } from '@videojs/core/dom'; +import { applyElementProps, completeMenuItemSelection } from '@videojs/core/dom'; import type { PropertyDeclarationMap, PropertyValues } from '@videojs/element'; import { ContextConsumer } from '@videojs/element/context'; @@ -100,7 +100,5 @@ export class MenuItemElement extends MediaElement { 'data-has-submenu': '', }), }); - - applyStateDataAttrs(this, ctx.state, ctx.stateAttrMap); } } diff --git a/packages/html/src/ui/menu/menu-label-element.ts b/packages/html/src/ui/menu/menu-label-element.ts index 7a7c6684..299a0b50 100644 --- a/packages/html/src/ui/menu/menu-label-element.ts +++ b/packages/html/src/ui/menu/menu-label-element.ts @@ -1,11 +1,5 @@ -import type { MenuState } from '@videojs/core'; -import { ContextConsumer } from '@videojs/element/context'; +import { MediaElement } from '../media-element'; -import { ContextPartElement } from '../context-part-element'; -import { menuContext } from './context'; - -export class MenuLabelElement extends ContextPartElement { +export class MenuLabelElement extends MediaElement { static readonly tagName = 'media-menu-label'; - - protected readonly consumer = new ContextConsumer(this, { context: menuContext, subscribe: true }); } 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 e5d4e35a..d3d7fe46 100644 --- a/packages/html/src/ui/menu/menu-radio-group-element.ts +++ b/packages/html/src/ui/menu/menu-radio-group-element.ts @@ -1,9 +1,9 @@ -import { applyElementProps, applyStateDataAttrs } from '@videojs/core/dom'; +import { applyElementProps } from '@videojs/core/dom'; import type { PropertyDeclarationMap, PropertyValues } from '@videojs/element'; -import { ContextConsumer, ContextProvider } from '@videojs/element/context'; +import { ContextProvider } from '@videojs/element/context'; import { MediaElement } from '../media-element'; -import { menuContext, menuRadioGroupContext } from './context'; +import { menuRadioGroupContext } from './context'; export class MenuRadioGroupElement extends MediaElement { static readonly tagName: string = 'media-menu-radio-group'; @@ -16,7 +16,6 @@ export class MenuRadioGroupElement extends MediaElement { value = ''; label: string | undefined = undefined; - readonly #menuCtx = new ContextConsumer(this, { context: menuContext, subscribe: true }); readonly #provider = new ContextProvider(this, { context: menuRadioGroupContext }); protected override update(_changed: PropertyValues): void { @@ -26,10 +25,6 @@ export class MenuRadioGroupElement extends MediaElement { role: 'group', 'aria-label': this.label, }); - - const ctx = this.#menuCtx.value; - if (ctx) applyStateDataAttrs(this, ctx.state, ctx.stateAttrMap); - this.#provider.setValue({ value: this.value, onValueChange: (next: string) => { diff --git a/packages/html/src/ui/menu/menu-radio-item-element.ts b/packages/html/src/ui/menu/menu-radio-item-element.ts index 0b664aab..fd84141d 100644 --- a/packages/html/src/ui/menu/menu-radio-item-element.ts +++ b/packages/html/src/ui/menu/menu-radio-item-element.ts @@ -1,4 +1,4 @@ -import { applyElementProps, applyStateDataAttrs, completeMenuItemSelection } from '@videojs/core/dom'; +import { applyElementProps, completeMenuItemSelection } from '@videojs/core/dom'; import type { PropertyDeclarationMap, PropertyValues } from '@videojs/element'; import { ContextConsumer } from '@videojs/element/context'; @@ -77,7 +77,5 @@ export class MenuRadioItemElement extends MediaElement { 'aria-checked': String(checked), 'aria-disabled': this.disabled ? 'true' : undefined, }); - - applyStateDataAttrs(this, menuCtx.state, menuCtx.stateAttrMap); } } diff --git a/packages/html/src/ui/menu/menu-separator-element.ts b/packages/html/src/ui/menu/menu-separator-element.ts index 1f584815..7fc75e0a 100644 --- a/packages/html/src/ui/menu/menu-separator-element.ts +++ b/packages/html/src/ui/menu/menu-separator-element.ts @@ -1,21 +1,14 @@ -import { applyElementProps, applyStateDataAttrs } from '@videojs/core/dom'; +import { applyElementProps } from '@videojs/core/dom'; import type { PropertyValues } from '@videojs/element'; -import { ContextConsumer } from '@videojs/element/context'; import { MediaElement } from '../media-element'; -import { menuContext } from './context'; export class MenuSeparatorElement extends MediaElement { static readonly tagName = 'media-menu-separator'; - readonly #ctx = new ContextConsumer(this, { context: menuContext, subscribe: true }); - protected override update(_changed: PropertyValues): void { super.update(_changed); applyElementProps(this, { role: 'separator' }); - - const ctx = this.#ctx.value; - if (ctx) applyStateDataAttrs(this, ctx.state, ctx.stateAttrMap); } } 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 bf011465..a3f57149 100644 --- a/packages/html/src/ui/menu/tests/menu-element.test.ts +++ b/packages/html/src/ui/menu/tests/menu-element.test.ts @@ -7,9 +7,16 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { playerContext } from '../../../player/context'; import { ControlsElement } from '../../controls/controls-element'; import { MediaElement } from '../../media-element'; +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 { 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'; import { MenuViewElement } from '../menu-view-element'; let tagCounter = 0; @@ -91,11 +98,108 @@ async function waitForAssertion(assertion: () => void): Promise { throw error; } +const menuStateAttrs = ['data-open', 'data-side', 'data-align', 'data-starting-style', 'data-ending-style'] as const; + +function expectNoMenuStateAttrs(element: HTMLElement): void { + for (const attr of menuStateAttrs) { + expect(element.hasAttribute(attr), `${element.localName} should not have ${attr}`).toBe(false); + } +} + afterEach(() => { document.body.innerHTML = ''; }); describe('MenuElement', () => { + it('scopes menu state data attributes to menu elements', async () => { + const root = createElement(MenuElement); + const label = createElement(MenuLabelElement); + const group = createElement(MenuGroupElement); + const item = createElement(MenuItemElement); + const checkboxItem = createElement(MenuCheckboxItemElement); + const radioGroup = createElement(MenuRadioGroupElement); + const radioItem = createElement(MenuRadioItemElement); + const indicator = createElement(MenuItemIndicatorElement); + const separator = createElement(MenuSeparatorElement); + const rootView = createElement(MenuViewElement); + const trigger = createElement(MenuItemElement); + const child = createElement(MenuElement); + const back = createElement(MenuBackElement); + const childItem = createElement(MenuItemElement); + + root.open = true; + 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'; + indicator.checked = true; + trigger.id = 'child-trigger'; + trigger.commandfor = 'child-menu'; + trigger.textContent = 'Quality'; + child.id = 'child-menu'; + back.textContent = 'Back'; + childItem.textContent = 'Auto'; + + radioItem.append(indicator); + radioGroup.append(radioItem); + group.append(item, checkboxItem, radioGroup); + rootView.append(trigger); + child.append(back, childItem); + root.append(label, group, separator, rootView, child); + document.body.append(root); + + await root.updateComplete; + await label.updateComplete; + await group.updateComplete; + await item.updateComplete; + await checkboxItem.updateComplete; + await radioGroup.updateComplete; + await radioItem.updateComplete; + await indicator.updateComplete; + await separator.updateComplete; + await rootView.updateComplete; + await trigger.updateComplete; + await child.updateComplete; + await back.updateComplete; + await childItem.updateComplete; + + expect(root.hasAttribute('data-open')).toBe(true); + expect(root.getAttribute('data-side')).toBe('top'); + expect(root.getAttribute('data-align')).toBe('end'); + + for (const element of [label, group, separator, item, checkboxItem, radioGroup, radioItem, indicator, trigger]) { + expectNoMenuStateAttrs(element); + } + + expect(item.hasAttribute('data-item')).toBe(true); + expect(checkboxItem.hasAttribute('data-item')).toBe(true); + expect(radioItem.hasAttribute('data-item')).toBe(true); + expect(trigger.hasAttribute('data-item')).toBe(true); + + trigger.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + + await root.updateComplete; + await child.updateComplete; + await back.updateComplete; + await waitForAssertion(() => { + expect(child.getAttribute('data-menu-view-state')).toBe('active'); + }); + + expect(child.hasAttribute('data-submenu')).toBe(true); + expect(child.hasAttribute('data-menu-view')).toBe(true); + expect(child.hasAttribute('data-open')).toBe(true); + expect(child.hasAttribute('data-side')).toBe(false); + expect(child.hasAttribute('data-align')).toBe(false); + expectNoMenuStateAttrs(back); + expectNoMenuStateAttrs(childItem); + }); + it('marks root and nested menu views with generic view attributes', async () => { const root = createElement(MenuElement); const rootView = createElement(MenuViewElement); diff --git a/packages/react/src/ui/menu/menu-back.tsx b/packages/react/src/ui/menu/menu-back.tsx index 1e730994..e1c9c590 100644 --- a/packages/react/src/ui/menu/menu-back.tsx +++ b/packages/react/src/ui/menu/menu-back.tsx @@ -33,7 +33,6 @@ export const MenuBack = forwardRef(function Me { render, className, style }, { state: parentMenu?.state ?? ({} as MenuState), - stateAttrMap: parentMenu?.stateAttrMap, ref: forwardedRef, props: [ { diff --git a/packages/react/src/ui/menu/menu-checkbox-item.tsx b/packages/react/src/ui/menu/menu-checkbox-item.tsx index e0e3624b..d8fddce0 100644 --- a/packages/react/src/ui/menu/menu-checkbox-item.tsx +++ b/packages/react/src/ui/menu/menu-checkbox-item.tsx @@ -21,7 +21,7 @@ export const MenuCheckboxItem = forwardRef(null); useEffect(() => { @@ -50,7 +50,6 @@ export const MenuCheckboxItem = forwardRef(function { render, className, style }, { state, - stateAttrMap, ref: menuViewComposedRef, props: [ { diff --git a/packages/react/src/ui/menu/menu-group.tsx b/packages/react/src/ui/menu/menu-group.tsx index 5b51cec5..8c58c47c 100644 --- a/packages/react/src/ui/menu/menu-group.tsx +++ b/packages/react/src/ui/menu/menu-group.tsx @@ -17,14 +17,13 @@ export const MenuGroup = forwardRef(function Men { render, className, style, label, ...elementProps }, forwardedRef ) { - const { state, stateAttrMap } = useMenuContext(); + const { state } = useMenuContext(); return renderElement( 'div', { render, className, style }, { state, - stateAttrMap, ref: [forwardedRef], props: [{ role: 'group' as const, 'aria-label': label }, elementProps], } diff --git a/packages/react/src/ui/menu/menu-item-indicator.tsx b/packages/react/src/ui/menu/menu-item-indicator.tsx index 9ead0ca1..9cd81bef 100644 --- a/packages/react/src/ui/menu/menu-item-indicator.tsx +++ b/packages/react/src/ui/menu/menu-item-indicator.tsx @@ -19,7 +19,7 @@ export const MenuItemIndicator = forwardRef(function MenuI { render, className, style, onSelect, disabled, onClick, ...elementProps }, forwardedRef ) { - const { menu, state, stateAttrMap } = useMenuContext(); + const { menu, state } = useMenuContext(); const subMenuCtx = useSubMenuContext(); const parentMenu = subMenuCtx?.parentMenu.menu ?? null; const elementRef = useRef(null); @@ -52,7 +52,6 @@ export const MenuItem = forwardRef(function MenuI { render, className, style }, { state, - stateAttrMap, ref: [forwardedRef, elementRef], props: [ { diff --git a/packages/react/src/ui/menu/menu-label.tsx b/packages/react/src/ui/menu/menu-label.tsx index 8599d632..48f8b1f0 100644 --- a/packages/react/src/ui/menu/menu-label.tsx +++ b/packages/react/src/ui/menu/menu-label.tsx @@ -14,14 +14,13 @@ export const MenuLabel = forwardRef(function Men { render, className, style, ...elementProps }, forwardedRef ) { - const { state, stateAttrMap } = useMenuContext(); + const { state } = useMenuContext(); return renderElement( 'div', { render, className, style }, { state, - stateAttrMap, ref: [forwardedRef], props: [elementProps], } diff --git a/packages/react/src/ui/menu/menu-radio-group.tsx b/packages/react/src/ui/menu/menu-radio-group.tsx index b60b226c..764b1f55 100644 --- a/packages/react/src/ui/menu/menu-radio-group.tsx +++ b/packages/react/src/ui/menu/menu-radio-group.tsx @@ -21,7 +21,7 @@ export const MenuRadioGroup = forwardRef(fu { render, className, style, value, onValueChange, label, ...elementProps }, forwardedRef ) { - const { state, stateAttrMap } = useMenuContext(); + const { state } = useMenuContext(); return ( @@ -30,7 +30,6 @@ export const MenuRadioGroup = forwardRef(fu { render, className, style }, { state, - stateAttrMap, ref: [forwardedRef], props: [{ role: 'group' as const, 'aria-label': label }, elementProps], } diff --git a/packages/react/src/ui/menu/menu-radio-item.tsx b/packages/react/src/ui/menu/menu-radio-item.tsx index 7d459a7d..887aa0cb 100644 --- a/packages/react/src/ui/menu/menu-radio-item.tsx +++ b/packages/react/src/ui/menu/menu-radio-item.tsx @@ -20,7 +20,7 @@ export const MenuRadioItem = forwardRef(func { render, className, style, value, disabled, onClick, ...elementProps }, forwardedRef ) { - const { menu, state, stateAttrMap } = useMenuContext(); + const { menu, state } = useMenuContext(); const { value: groupValue, onValueChange } = useMenuRadioGroupContext(); const subMenuCtx = useSubMenuContext(); const parentMenu = subMenuCtx?.parentMenu.menu ?? null; @@ -54,7 +54,6 @@ export const MenuRadioItem = forwardRef(func { render, className, style }, { state, - stateAttrMap, ref: [forwardedRef, elementRef], props: [ { diff --git a/packages/react/src/ui/menu/menu-separator.tsx b/packages/react/src/ui/menu/menu-separator.tsx index b0fc7dbd..9ef74550 100644 --- a/packages/react/src/ui/menu/menu-separator.tsx +++ b/packages/react/src/ui/menu/menu-separator.tsx @@ -14,14 +14,13 @@ export const MenuSeparator = forwardRef(func { render, className, style, ...elementProps }, forwardedRef ) { - const { state, stateAttrMap } = useMenuContext(); + const { state } = useMenuContext(); return renderElement( 'div', { render, className, style }, { state, - stateAttrMap, ref: [forwardedRef], props: [{ role: 'separator' as const }, elementProps], } diff --git a/packages/react/src/ui/menu/menu-trigger.tsx b/packages/react/src/ui/menu/menu-trigger.tsx index dce292cd..bf174d73 100644 --- a/packages/react/src/ui/menu/menu-trigger.tsx +++ b/packages/react/src/ui/menu/menu-trigger.tsx @@ -49,7 +49,7 @@ export const MenuTrigger = forwardRef, elementRef as React.Ref], props: [ { @@ -145,7 +144,6 @@ export const MenuTrigger = forwardRef, triggerRef], props: [ { type: 'button' as const, ...core.getTriggerAttrs(state, contentId) }, diff --git a/packages/react/src/ui/menu/tests/menu.test.tsx b/packages/react/src/ui/menu/tests/menu.test.tsx index bb98bed1..1649dd9b 100644 --- a/packages/react/src/ui/menu/tests/menu.test.tsx +++ b/packages/react/src/ui/menu/tests/menu.test.tsx @@ -6,8 +6,14 @@ import { ControlsContextProvider } from '../../controls/context'; import { MenuBack } from '../menu-back'; import { MenuCheckboxItem } from '../menu-checkbox-item'; import { MenuContent } from '../menu-content'; +import { MenuGroup } from '../menu-group'; 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'; +import { MenuSeparator } from '../menu-separator'; import { MenuTrigger } from '../menu-trigger'; import { MenuView } from '../menu-view'; @@ -263,7 +269,93 @@ function FocusOutFixture({ onRootOpenChange }: { onRootOpenChange: NonNullable { + it('scopes menu state data attributes to content elements', async () => { + render( + + Settings + + Playback + + Copy link + + Autoplay + + + + Auto + + Checked + + + + + + + + Quality + + Back + Auto + + + + + + ); + + const rootContent = screen.getByTestId('root-content'); + + expect(rootContent.hasAttribute('data-open')).toBe(true); + expect(rootContent.getAttribute('data-side')).toBe('top'); + expect(rootContent.getAttribute('data-align')).toBe('end'); + + for (const testId of [ + 'trigger', + 'label', + 'group', + 'separator', + 'item', + 'checkbox-item', + 'radio-group', + 'radio-item', + 'indicator', + 'submenu-trigger', + ]) { + expectNoMenuStateAttrs(screen.getByTestId(testId)); + } + + expect(screen.getByTestId('item').hasAttribute('data-item')).toBe(true); + expect(screen.getByTestId('radio-item').hasAttribute('data-item')).toBe(true); + expect(screen.getByTestId('checkbox-item').hasAttribute('data-item')).toBe(true); + expect(screen.getByTestId('submenu-trigger').hasAttribute('data-item')).toBe(true); + + fireEvent.click(screen.getByTestId('submenu-trigger')); + + await waitFor(() => { + expect(screen.getByTestId('submenu-content').getAttribute('data-menu-view-state')).toBe('active'); + }); + + const submenuContent = screen.getByTestId('submenu-content'); + + expect(submenuContent.hasAttribute('data-submenu')).toBe(true); + expect(submenuContent.hasAttribute('data-menu-view')).toBe(true); + expect(submenuContent.hasAttribute('data-open')).toBe(true); + expect(submenuContent.hasAttribute('data-side')).toBe(false); + expect(submenuContent.hasAttribute('data-align')).toBe(false); + expectNoMenuStateAttrs(screen.getByTestId('back')); + }); + it('marks the root view inactive while a submenu view is active', async () => { render(); diff --git a/packages/react/src/utils/tests/use-render.test.tsx b/packages/react/src/utils/tests/use-render.test.tsx index 78b4795e..3b815b27 100644 --- a/packages/react/src/utils/tests/use-render.test.tsx +++ b/packages/react/src/utils/tests/use-render.test.tsx @@ -419,21 +419,14 @@ describe('renderElement', () => { }); describe('state data attributes', () => { - it('generates data-* attributes from state boolean true values', () => { + it('does not generate data-* attributes without an explicit mapping', () => { const { container } = render(); const element = container.firstElementChild; - expect(element?.getAttribute('data-active')).toBe(''); - }); - - it('does not generate data-* attributes from state boolean false values', () => { - const { container } = render(); - const element = container.firstElementChild; - expect(element?.hasAttribute('data-active')).toBe(false); }); - it('generates data-* attributes from state with multiple properties', () => { + it('generates mapped data-* attributes from state with multiple properties', () => { interface MultiState { paused: boolean; ended: boolean; @@ -454,8 +447,17 @@ describe('renderElement', () => { ...elementProps } = props; const state: MultiState = { paused, ended, volume }; + const stateAttrMap = { + paused: 'data-paused', + ended: 'data-ended', + volume: 'data-volume', + } as const; - return renderElement('div', { className, style, render: renderProp }, { state, ref, props: [elementProps] }); + return renderElement( + 'div', + { className, style, render: renderProp }, + { state, ref, props: [elementProps], stateAttrMap } + ); }); const { container } = render(); @@ -466,7 +468,7 @@ describe('renderElement', () => { expect(element?.getAttribute('data-volume')).toBe('0.5'); }); - it('converts state keys to lowercase for data attributes', () => { + it('does not expose unmapped state keys as data attributes', () => { interface CamelCaseState { isPaused: boolean; } @@ -484,7 +486,7 @@ describe('renderElement', () => { const { container } = render(); const element = container.firstElementChild; - expect(element?.getAttribute('data-ispaused')).toBe(''); + expect(element?.hasAttribute('data-ispaused')).toBe(false); }); it('supports explicit state attribute mapping', () => { @@ -534,6 +536,7 @@ describe('renderElement', () => { { state, ref, + stateAttrMap: { active: 'data-active' }, // Explicit prop comes after state in merge order, so it wins props: [elementProps], } diff --git a/packages/react/src/utils/use-render.tsx b/packages/react/src/utils/use-render.tsx index 37032c69..8bcbc8b9 100644 --- a/packages/react/src/utils/use-render.tsx +++ b/packages/react/src/utils/use-render.tsx @@ -85,8 +85,9 @@ export function renderElement< const className = resolveClassName(classNameProp, state); const style = resolveStyle(styleProp, state); - // Generate data attributes from state - const stateDataAttrs = getStateDataAttrs(state, stateAttrMap); + // Generate data attributes only when a component explicitly opts in with a + // mapping. State is still passed to render/className/style callbacks. + const stateDataAttrs = stateAttrMap ? getStateDataAttrs(state, stateAttrMap) : {}; // Merge: state data attrs first, then props (so props can override) const propsArray = Array.isArray(props) ? props : props ? [props] : [];