From 80c7c2b7a367c479f5d09ce71283846f342be499 Mon Sep 17 00:00:00 2001 From: Rahim Date: Tue, 1 Sep 2026 16:34:18 -0700 Subject: [PATCH 1/3] fix(vjsc): preserve component host props --- packages/vjsc/src/plugins/html-runtime.ts | 16 ++++++++++- .../plugins/tests/component-target.test.ts | 17 +++++++++++- .../src/plugins/tests/html-runtime.test.ts | 5 ++-- packages/vjsc/src/target/render.ts | 27 +++++++++++++++---- packages/vjsc/src/target/source.ts | 11 +++++++- 5 files changed, 66 insertions(+), 10 deletions(-) diff --git a/packages/vjsc/src/plugins/html-runtime.ts b/packages/vjsc/src/plugins/html-runtime.ts index c23991565b..a18b40c872 100644 --- a/packages/vjsc/src/plugins/html-runtime.ts +++ b/packages/vjsc/src/plugins/html-runtime.ts @@ -58,7 +58,21 @@ export function Host(props) { } const current = child[element]; - return htmlElement(current.type, { ...current.attributes, ...attributes }, current.children); + return htmlElement(current.type, mergeHostAttributes(current.attributes, attributes), current.children); +} + +function mergeHostAttributes(current, forwarded) { + const attributes = { ...current, ...forwarded }; + const className = [current.class, current.className, forwarded.class, forwarded.className] + .flat(Infinity) + .filter(Boolean); + + delete attributes.className; + + if (className.length > 0) attributes.class = className; + else delete attributes.class; + + return attributes; } function renderAttributes(attributes, context) { diff --git a/packages/vjsc/src/plugins/tests/component-target.test.ts b/packages/vjsc/src/plugins/tests/component-target.test.ts index 97c7a49cb9..dfbf4696cd 100644 --- a/packages/vjsc/src/plugins/tests/component-target.test.ts +++ b/packages/vjsc/src/plugins/tests/component-target.test.ts @@ -3,7 +3,7 @@ import { describe, expect, it } from 'vite-plus/test'; import { defineComponent, defineSchema } from '../../components/definition'; import { defineComponentTarget } from '../../target/definition'; -import { jsx } from '../../target/jsx-runtime'; +import { Host, jsx } from '../../target/jsx-runtime'; import { readComponentSource } from '../component-meta'; import { componentSourcePlugin } from '../component-source'; import { type ComponentTargetSelection, componentTargetPlugin } from '../component-target'; @@ -99,6 +99,7 @@ const htmlTarget = defineComponentTarget()(({ target, element, un }), rules: { OptionGroup: { Root: unwrap() }, + Poster: ({ props, children }) => jsx(Host, { ...props, className: 'poster', children }), PlayButton: () => jsx(Svg, { viewBox: '0 0 18 18', @@ -274,6 +275,20 @@ describe('componentTargetPlugin', () => { ); }); + it('preserves component prop names when forwarding HTML host props', async () => { + const source = await transform( + ` + import * as $ from '@fixture/components'; + const CustomPoster = (props) => ; + export const poster = <$.Poster src="poster.jpg">; + `, + { targets: [htmlTarget] } + ); + + expect(source).toMatch(//); + expect(source).not.toContain('class="poster"'); + }); + it('lowers canonical components retained by an outer rewrite', async () => { const source = await transform(` import * as $ from '@fixture/components'; diff --git a/packages/vjsc/src/plugins/tests/html-runtime.test.ts b/packages/vjsc/src/plugins/tests/html-runtime.test.ts index 0ab73edea7..db521fca29 100644 --- a/packages/vjsc/src/plugins/tests/html-runtime.test.ts +++ b/packages/vjsc/src/plugins/tests/html-runtime.test.ts @@ -30,11 +30,12 @@ describe('htmlRuntimePlugin', () => { it('forwards host attributes to one dynamic element child', async () => { const runtime = await loadRuntime(); const output = runtime.jsx(runtime.Host, { + class: ['trigger', 'active'], id: 'trigger', - children: runtime.jsx('button', { className: ['button', 'active'] }), + children: runtime.jsx('button', { className: 'button' }), }); - expect(String(output)).toBe(''); + expect(String(output)).toBe(''); }); it('flattens class arrays after HTML attribute normalization', async () => { diff --git a/packages/vjsc/src/target/render.ts b/packages/vjsc/src/target/render.ts index 82886f8077..e8eb664d83 100644 --- a/packages/vjsc/src/target/render.ts +++ b/packages/vjsc/src/target/render.ts @@ -82,6 +82,14 @@ function renderTargetNode(node: TargetNode, context: TargetRenderContext): strin } export function renderTargetAttributes(node: TargetNode, context: TargetRenderContext): string[] { + return renderAttributesFor(node, context, context.target.jsx.attributes); +} + +function renderAttributesFor( + node: TargetNode, + context: TargetRenderContext, + attributeMode: ComponentTarget['jsx']['attributes'] +): string[] { const attributes: string[] = []; const props = node.props as Readonly>; @@ -93,7 +101,7 @@ export function renderTargetAttributes(node: TargetNode, context: TargetRenderCo const value = props[property]; if (property === SOURCE_PROPS) { - if (isSourcePropsToken(value)) attributes.push(...renderSourceProps(value, context.target.jsx.attributes)); + if (isSourcePropsToken(value)) attributes.push(...renderSourceProps(value, attributeMode)); continue; } @@ -108,7 +116,7 @@ export function renderTargetAttributes(node: TargetNode, context: TargetRenderCo if (typeof property !== 'string' || value === undefined) continue; - const attribute = renderAttribute(property, value, context); + const attribute = renderAttribute(property, value, context, attributeMode); if (attribute) attributes.push(attribute); } @@ -150,8 +158,13 @@ function renderTargetReplacement(replacement: TargetReplacement, context: Target return renderSourceRange(source, replacement.branchStart, replacement.branchEnd).value; } -function renderAttribute(name: string, value: unknown, context: TargetRenderContext): string | undefined { - const targetName = targetAttributeName(name, context.target.jsx.attributes); +function renderAttribute( + name: string, + value: unknown, + context: TargetRenderContext, + attributeMode: ComponentTarget['jsx']['attributes'] +): string | undefined { + const targetName = targetAttributeName(name, attributeMode); if (isSourcePropToken(value)) return renderSourcePropAttribute(targetName, value); @@ -263,7 +276,11 @@ function renderWithProps( const attributes = isExpressionNode(props) ? [`{...${renderTargetExpression(props, context)}}`] - : renderTargetAttributes({ [TARGET_NODE]: true, type: TARGET_HOST, props: { ...props }, key: null }, context); + : renderAttributesFor( + { [TARGET_NODE]: true, type: TARGET_HOST, props: { ...props }, key: null }, + context, + children.rootComponent ? 'react' : context.target.jsx.attributes + ); if (children.rootOpeningEnd === undefined) { const host = context.target.jsx.host; diff --git a/packages/vjsc/src/target/source.ts b/packages/vjsc/src/target/source.ts index 16d7d4b98f..12c6a88cf8 100644 --- a/packages/vjsc/src/target/source.ts +++ b/packages/vjsc/src/target/source.ts @@ -1,4 +1,4 @@ -import type { JSXAttribute, JSXElement, JSXOpeningElement } from '@oxc-project/types'; +import type { JSXAttribute, JSXElement, JSXElementName, JSXOpeningElement } from '@oxc-project/types'; import { createSourceText, renderSourceRange, type SourceText } from '../ast'; import { type SourceProps, type TargetOutput, type TargetReplacement, TARGET_REPLACEMENT } from './definition'; @@ -27,6 +27,8 @@ export interface SourceChildrenToken { readonly value: string; /** Opening-tag offset when the children contain exactly one JSX element. */ readonly rootOpeningEnd?: number | undefined; + /** Whether the single root is a component invocation rather than an intrinsic element. */ + readonly rootComponent?: boolean | undefined; } export function singleJsxElementChild(node: JSXElement): JSXElement | undefined { @@ -115,9 +117,16 @@ export function createSourceChildren( source: normalized, value: rendered.value, ...(rootOpeningEnd !== undefined ? { rootOpeningEnd } : {}), + ...(rootOpening ? { rootComponent: isComponentName(rootOpening.name) } : {}), }; } +function isComponentName(name: JSXElementName): boolean { + if (name.type === 'JSXIdentifier') return /^[A-Z]/.test(name.name); + + return name.type === 'JSXMemberExpression'; +} + export function isSourcePropsToken(value: unknown): value is SourcePropsToken { return Boolean(value && typeof value === 'object' && (value as Partial)[SOURCE_PROPS] === true); } From 3bcf2c6759a54d4eeb7f3d800e97202ec226eabb Mon Sep 17 00:00:00 2001 From: Rahim Date: Tue, 1 Sep 2026 16:34:39 -0700 Subject: [PATCH 2/3] fix(core): stabilize popup interactions --- packages/core/src/core/ui/menu/core.ts | 16 +++--- .../core/src/core/ui/menu/menu-component.ts | 7 +-- packages/core/src/core/ui/popover/core.ts | 14 ++++-- packages/core/src/core/ui/tooltip/core.ts | 14 ++++-- .../core/src/core/ui/volume-popover/core.ts | 2 +- packages/core/src/dom/ui/menu/menu-popup.ts | 21 +++++++- .../src/dom/ui/menu/tests/menu-popup.test.ts | 50 +++++++++++++++++++ packages/core/src/dom/ui/popover/popover.ts | 1 + .../src/dom/ui/popover/tests/popover.test.ts | 15 ++++++ packages/react/src/ui/menu/menu-root.tsx | 2 +- .../react/src/ui/menu/tests/menu.test.tsx | 8 +-- .../react/src/ui/popover/popover-root.tsx | 2 +- .../react/src/ui/tooltip/tooltip-root.tsx | 2 +- .../react/src/utils/tests/use-render.test.tsx | 11 ++++ packages/react/src/utils/use-render.tsx | 15 ++++-- 15 files changed, 147 insertions(+), 33 deletions(-) create mode 100644 packages/core/src/dom/ui/menu/tests/menu-popup.test.ts diff --git a/packages/core/src/core/ui/menu/core.ts b/packages/core/src/core/ui/menu/core.ts index 479646377e..47036120f1 100644 --- a/packages/core/src/core/ui/menu/core.ts +++ b/packages/core/src/core/ui/menu/core.ts @@ -1,7 +1,7 @@ import { defaults } from '@videojs/utils/object'; import type { NonNullableObject } from '@videojs/utils/types'; -import type { PopoverAlign, PopoverSide } from '../popover/core'; +import type { PopoverAlign, PopoverBoundary, PopoverSide } from '../popover/core'; import type { TransitionFlags, TransitionState, TransitionStatus } from '../transition'; import { getTransitionFlags } from '../transition'; @@ -12,6 +12,8 @@ export interface MenuProps { side?: PopoverSide | undefined; /** Alignment along the trigger's edge. Root menus only. */ align?: PopoverAlign | undefined; + /** Boundary used to constrain the root menu popup. */ + boundary?: PopoverBoundary | undefined; /** Controlled open state. */ open?: boolean | undefined; /** Initial open state (uncontrolled). */ @@ -22,6 +24,8 @@ export interface MenuProps { closeOnOutsideClick?: boolean | undefined; } +type MenuCoreProps = Omit; + export interface MenuTriggerProps { disabled?: boolean | undefined; } @@ -91,7 +95,7 @@ export interface MenuState extends TransitionFlags { /** Base menu logic: ARIA attributes and open/close state computation. */ export class MenuCore { - static readonly defaultProps: NonNullableObject = { + static readonly defaultProps: NonNullableObject = { side: 'bottom', align: 'start', open: false, @@ -103,15 +107,15 @@ export class MenuCore { #props = { ...MenuCore.defaultProps }; #input: MenuInput | null = null; - get props(): Readonly> { + get props(): Readonly> { return this.#props; } - constructor(props?: MenuProps) { + constructor(props?: MenuCoreProps) { if (props) this.setProps(props); } - setProps(props: MenuProps): void { + setProps(props: MenuCoreProps): void { this.#props = defaults(props, MenuCore.defaultProps); } @@ -155,7 +159,7 @@ export class MenuCore { } export namespace MenuCore { - export type Props = MenuProps; + export type Props = MenuCoreProps; export type State = MenuState; export type Input = MenuInput; } diff --git a/packages/core/src/core/ui/menu/menu-component.ts b/packages/core/src/core/ui/menu/menu-component.ts index 128bb15dd1..faca6b4fe4 100644 --- a/packages/core/src/core/ui/menu/menu-component.ts +++ b/packages/core/src/core/ui/menu/menu-component.ts @@ -3,16 +3,11 @@ import { defineComponent } from 'vjsc/components'; import type { MenuItemIndicatorProps, MenuItemProps, MenuPopupProps, MenuProps, MenuTriggerProps } from './core'; import { MenuDataAttrs } from './data'; -interface MenuRootProps extends MenuProps { - /** Boundary used to constrain the root menu popup size. */ - boundary?: 'viewport' | 'container' | (string & {}) | undefined; -} - export default defineComponent({ name: 'Menu', root: 'Root', parts: { - Root: defineComponent(), + Root: defineComponent(), Trigger: defineComponent(), Popup: defineComponent(), Content: defineComponent(), diff --git a/packages/core/src/core/ui/popover/core.ts b/packages/core/src/core/ui/popover/core.ts index 68c311b020..b4d1d948f9 100644 --- a/packages/core/src/core/ui/popover/core.ts +++ b/packages/core/src/core/ui/popover/core.ts @@ -8,11 +8,15 @@ export type PopoverSide = 'top' | 'bottom' | 'left' | 'right'; export type PopoverAlign = 'start' | 'center' | 'end'; +export type PopoverBoundary = 'viewport' | 'container' | (string & {}); + export interface PopoverProps { /** Preferred side of the trigger for the popup. */ side?: PopoverSide | undefined; /** Alignment of the popup along the trigger's edge. */ align?: PopoverAlign | undefined; + /** Boundary used to constrain the popup position. */ + boundary?: PopoverBoundary | undefined; /** * - `false` (default): non-modal; background content remains interactive. * - `true`: modal; sets `aria-modal="true"` on the popup. @@ -35,6 +39,8 @@ export interface PopoverProps { closeDelay?: number | undefined; } +type PopoverCoreProps = Omit; + /** * The raw transition state managed by `createTransition`. Uses `active` (not `open`) to distinguish the generic * transition state machine from the domain-specific `PopoverState.open`. @@ -51,7 +57,7 @@ export interface PopoverState extends TransitionFlags { } export class PopoverCore { - static readonly defaultProps: NonNullableObject = { + static readonly defaultProps: NonNullableObject = { side: 'top', align: 'center', modal: false, @@ -66,11 +72,11 @@ export class PopoverCore { #props = { ...PopoverCore.defaultProps }; - constructor(props?: PopoverProps) { + constructor(props?: PopoverCoreProps) { if (props) this.setProps(props); } - setProps(props: PopoverProps): void { + setProps(props: PopoverCoreProps): void { this.#props = defaults(props, PopoverCore.defaultProps); } @@ -111,7 +117,7 @@ export class PopoverCore { } export namespace PopoverCore { - export type Props = PopoverProps; + export type Props = PopoverCoreProps; export type State = PopoverState; export type Input = PopoverInput; } diff --git a/packages/core/src/core/ui/tooltip/core.ts b/packages/core/src/core/ui/tooltip/core.ts index 49a3c3495a..60f9ee34af 100644 --- a/packages/core/src/core/ui/tooltip/core.ts +++ b/packages/core/src/core/ui/tooltip/core.ts @@ -1,7 +1,7 @@ import { defaults } from '@videojs/utils/object'; import type { NonNullableObject } from '@videojs/utils/types'; -import type { PopoverAlign, PopoverSide } from '../popover/core'; +import type { PopoverAlign, PopoverBoundary, PopoverSide } from '../popover/core'; import type { TransitionFlags, TransitionState, TransitionStatus } from '../transition'; import { getTransitionFlags } from '../transition'; @@ -10,6 +10,8 @@ export interface TooltipProps { side?: PopoverSide | undefined; /** Alignment of the tooltip along the trigger's edge. */ align?: PopoverAlign | undefined; + /** Boundary used to constrain the tooltip position. */ + boundary?: PopoverBoundary | undefined; /** Controlled open state. */ open?: boolean | undefined; /** Initial open state for uncontrolled usage. */ @@ -26,6 +28,8 @@ export interface TooltipProps { sticky?: boolean | undefined; } +type TooltipCoreProps = Omit; + export interface TooltipInput extends TransitionState {} export interface TooltipState extends TransitionFlags { @@ -40,7 +44,7 @@ export interface TooltipState extends TransitionFlags { } export class TooltipCore { - static readonly defaultProps: NonNullableObject = { + static readonly defaultProps: NonNullableObject = { side: 'top', align: 'center', open: false, @@ -54,11 +58,11 @@ export class TooltipCore { #props = { ...TooltipCore.defaultProps }; - constructor(props?: TooltipProps) { + constructor(props?: TooltipCoreProps) { if (props) this.setProps(props); } - setProps(props: TooltipProps): void { + setProps(props: TooltipCoreProps): void { this.#props = defaults(props, TooltipCore.defaultProps); } @@ -89,7 +93,7 @@ export class TooltipCore { } export namespace TooltipCore { - export type Props = TooltipProps; + export type Props = TooltipCoreProps; export type State = TooltipState; export type Input = TooltipInput; } diff --git a/packages/core/src/core/ui/volume-popover/core.ts b/packages/core/src/core/ui/volume-popover/core.ts index f77158d535..3f11c958e0 100644 --- a/packages/core/src/core/ui/volume-popover/core.ts +++ b/packages/core/src/core/ui/volume-popover/core.ts @@ -33,6 +33,6 @@ export class VolumePopoverCore extends PopoverCore { } export namespace VolumePopoverCore { - export type Props = VolumePopoverProps; + export type Props = PopoverCore.Props; export type State = VolumePopoverState; } diff --git a/packages/core/src/dom/ui/menu/menu-popup.ts b/packages/core/src/dom/ui/menu/menu-popup.ts index b058fda3d8..6d3dedda86 100644 --- a/packages/core/src/dom/ui/menu/menu-popup.ts +++ b/packages/core/src/dom/ui/menu/menu-popup.ts @@ -95,6 +95,16 @@ export function createMenuPopup(): MenuPopupApi { ); } + function getVerticalScrollbarWidth(content: HTMLElement): number { + if (content.scrollHeight <= content.clientHeight) return 0; + + const style = getComputedStyle(content); + const inlineBorder = + (Number.parseFloat(style.borderInlineStartWidth) || 0) + (Number.parseFloat(style.borderInlineEndWidth) || 0); + + return Math.max(0, content.offsetWidth - content.clientWidth - inlineBorder); + } + function measureContent(content: HTMLElement, availableWidth: number | null) { const children = getElementChildren( content, @@ -157,8 +167,15 @@ export function createMenuPopup(): MenuPopupApi { const contentAvailableWidth = availableWidth === null ? null : Math.max(0, availableWidth - inlinePadding); const size = measureContent(current.element, contentAvailableWidth); - element.style.setProperty(MenuCSSVars.width, `${Math.ceil(size.width + inlinePadding)}px`); - element.style.setProperty(MenuCSSVars.height, `${Math.ceil(size.height + blockPadding)}px`); + const width = Math.ceil(size.width + inlinePadding); + const height = Math.ceil(size.height + blockPadding); + + element.style.setProperty(MenuCSSVars.width, `${width}px`); + element.style.setProperty(MenuCSSVars.height, `${height}px`); + + const scrollbarWidth = getVerticalScrollbarWidth(current.element); + + if (scrollbarWidth > 0) element.style.setProperty(MenuCSSVars.width, `${width + scrollbarWidth}px`); } function setElement(next: HTMLElement | null): void { diff --git a/packages/core/src/dom/ui/menu/tests/menu-popup.test.ts b/packages/core/src/dom/ui/menu/tests/menu-popup.test.ts new file mode 100644 index 0000000000..b8ef9be1f3 --- /dev/null +++ b/packages/core/src/dom/ui/menu/tests/menu-popup.test.ts @@ -0,0 +1,50 @@ +import { afterEach, describe, expect, it, vi } from 'vite-plus/test'; + +import { MenuCSSVars } from '../../../../core/ui/menu/vars'; +import { createMenuPopup } from '../menu-popup'; +import { createTestMenu } from './create-menu-helpers'; + +afterEach(() => { + document.body.replaceChildren(); + vi.restoreAllMocks(); +}); + +describe('createMenuPopup', () => { + it('includes the vertical scrollbar when sizing the popup', () => { + const popupElement = document.createElement('div'); + const content = document.createElement('div'); + const item = document.createElement('div'); + const { menu } = createTestMenu(); + const popup = createMenuPopup(); + + popupElement.style.paddingInlineStart = '4px'; + popupElement.style.paddingInlineEnd = '4px'; + popupElement.style.paddingBlockStart = '4px'; + popupElement.style.paddingBlockEnd = '4px'; + content.append(item); + popupElement.append(content); + document.body.append(popupElement); + + vi.spyOn(item, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, 60, 266)); + Object.defineProperties(item, { + scrollWidth: { configurable: true, value: 60 }, + scrollHeight: { configurable: true, value: 266 }, + }); + Object.defineProperties(content, { + offsetWidth: { configurable: true, value: 64 }, + clientWidth: { configurable: true, value: 49 }, + scrollHeight: { configurable: true, value: 266 }, + clientHeight: { configurable: true, value: 209 }, + }); + + popup.setElement(popupElement); + popup.registerContent({ menu, parent: null, element: content }); + popup.sync(); + + expect(popupElement.style.getPropertyValue(MenuCSSVars.width)).toBe('83px'); + expect(popupElement.style.getPropertyValue(MenuCSSVars.height)).toBe('274px'); + + popup.destroy(); + menu.destroy(); + }); +}); diff --git a/packages/core/src/dom/ui/popover/popover.ts b/packages/core/src/dom/ui/popover/popover.ts index 483f4c3da1..269cd53652 100644 --- a/packages/core/src/dom/ui/popover/popover.ts +++ b/packages/core/src/dom/ui/popover/popover.ts @@ -181,6 +181,7 @@ export function createPopover(options: PopoverOptions): PopoverApi { const opening = layer.open(() => popupEl); if (!opening) return; + tryShowPopover(popupEl); options.group?.()?.open(groupMember); opening.then(() => { diff --git a/packages/core/src/dom/ui/popover/tests/popover.test.ts b/packages/core/src/dom/ui/popover/tests/popover.test.ts index 3a71cea7af..2fd50e6769 100644 --- a/packages/core/src/dom/ui/popover/tests/popover.test.ts +++ b/packages/core/src/dom/ui/popover/tests/popover.test.ts @@ -38,6 +38,21 @@ describe('createPopover', () => { expect(popover.input.current).toEqual({ active: true, status: 'ending' }); }); + it('shows an already-mounted popup when a deferred open is committed', () => { + const { popover } = createTestPopover({ deferOpenChanges: true }); + const popup = document.createElement('div'); + const showPopover = vi.fn(); + + Object.defineProperty(popup, 'showPopover', { value: showPopover }); + popover.setPopupElement(popup); + + popover.open(); + expect(showPopover).not.toHaveBeenCalled(); + + popover.syncOpen(true); + expect(showPopover).toHaveBeenCalledOnce(); + }); + it('updates input state and calls onOpenChange when opening', () => { const { popover, onOpenChange } = createTestPopover(); diff --git a/packages/react/src/ui/menu/menu-root.tsx b/packages/react/src/ui/menu/menu-root.tsx index 6f3cfd7624..3f37480331 100644 --- a/packages/react/src/ui/menu/menu-root.tsx +++ b/packages/react/src/ui/menu/menu-root.tsx @@ -19,7 +19,7 @@ import { useOptionalControlsContext } from '../controls/context'; import { usePositionedState } from '../hooks/use-positioned-state'; import { MenuContextProvider, useOptionalMenuContext } from './context'; -export interface MenuRootProps extends MenuCore.Props { +export interface MenuRootProps extends Omit { /** Boundary used to constrain the root menu popup size. */ boundary?: PositioningBoundary; /** Called when the menu open state changes (fires immediately, before animations). */ diff --git a/packages/react/src/ui/menu/tests/menu.test.tsx b/packages/react/src/ui/menu/tests/menu.test.tsx index e70e3ca8b0..5c9449c6b3 100644 --- a/packages/react/src/ui/menu/tests/menu.test.tsx +++ b/packages/react/src/ui/menu/tests/menu.test.tsx @@ -717,8 +717,11 @@ describe('MenuContent', () => { expect(submenu.hasAttribute('data-ending-style')).toBe(true); expect(submenu.hasAttribute('data-open')).toBe(true); expect(screen.getByTestId('submenu-trigger').getAttribute('aria-expanded')).toBe('false'); - expect(screen.getByTestId('root-content').hasAttribute('data-child-open')).toBe(false); - expect(submenu.hasAttribute('inert')).toBe(true); + + await waitFor(() => { + expect(screen.getByTestId('root-content').hasAttribute('data-child-open')).toBe(false); + expect(submenu.hasAttribute('inert')).toBe(true); + }); }); it('portals submenu content into the popup', async () => { @@ -1036,7 +1039,6 @@ describe('MenuContent', () => { fireEvent.click(screen.getByTestId('submenu-back')); expect(screen.getByTestId('submenu-content').hasAttribute('data-ending-style')).toBe(true); - expect(screen.getByTestId('root-content').hasAttribute('inert')).toBe(false); await waitFor(() => { expect(screen.queryByTestId('submenu-content')).toBeNull(); diff --git a/packages/react/src/ui/popover/popover-root.tsx b/packages/react/src/ui/popover/popover-root.tsx index 7c8fcbd14a..2347b6742e 100644 --- a/packages/react/src/ui/popover/popover-root.tsx +++ b/packages/react/src/ui/popover/popover-root.tsx @@ -19,7 +19,7 @@ import { useOptionalControlsContext } from '../controls/context'; import { usePositionedState } from '../hooks/use-positioned-state'; import { PopoverContextProvider } from './context'; -export interface PopoverRootProps extends CorePopoverProps { +export interface PopoverRootProps extends Omit { /** Boundary used to constrain the popup size. */ boundary?: PositioningBoundary; /** Called when the popover open state changes (fires immediately, before animations). */ diff --git a/packages/react/src/ui/tooltip/tooltip-root.tsx b/packages/react/src/ui/tooltip/tooltip-root.tsx index 77b162dde9..48bd74049e 100644 --- a/packages/react/src/ui/tooltip/tooltip-root.tsx +++ b/packages/react/src/ui/tooltip/tooltip-root.tsx @@ -20,7 +20,7 @@ import { usePositionedState } from '../hooks/use-positioned-state'; import { type TooltipContent, TooltipContextProvider } from './context'; import { useTooltipGroup } from './group-context'; -export interface TooltipRootProps extends CoreTooltipProps { +export interface TooltipRootProps extends Omit { /** Boundary used to constrain the popup size. */ boundary?: PositioningBoundary; /** Called when the tooltip open state changes (fires immediately, before animations). */ diff --git a/packages/react/src/utils/tests/use-render.test.tsx b/packages/react/src/utils/tests/use-render.test.tsx index 8491619085..a24bfdd681 100644 --- a/packages/react/src/utils/tests/use-render.test.tsx +++ b/packages/react/src/utils/tests/use-render.test.tsx @@ -265,6 +265,17 @@ describe('renderElement', () => { expect(ref.current).toBeInstanceOf(HTMLDivElement); }); + it('keeps a single callback ref attached across renders', () => { + const ref = vi.fn(); + const { rerender } = render(); + const element = ref.mock.calls[0]![0]; + + rerender(); + + expect(ref).toHaveBeenCalledTimes(1); + expect(ref).toHaveBeenCalledWith(element); + }); + it('forwards array of refs', () => { const ref1 = createRef(); const ref2 = createRef(); diff --git a/packages/react/src/utils/use-render.tsx b/packages/react/src/utils/use-render.tsx index 4a60819500..19a1013b7a 100644 --- a/packages/react/src/utils/use-render.tsx +++ b/packages/react/src/utils/use-render.tsx @@ -43,6 +43,15 @@ function getElementRef(element: ReactElement): Ref | undefined { return elementAny.ref ?? elementAny.props?.ref; } +function mergeRefs(...refs: (Ref | Ref[] | undefined)[]): Ref | undefined { + const flatRefs = refs.flat().filter((ref): ref is Ref => ref !== null && ref !== undefined); + if (flatRefs.length === 0) return undefined; + + if (flatRefs.length === 1) return flatRefs[0]; + + return composeRefs(...flatRefs); +} + /** * Render a UI component element. * @@ -95,7 +104,7 @@ export function renderElement< if (isFunction(render)) { // Render function: call with props and state - const mergedRef = composeRefs(ref, mergedProps.ref); + const mergedRef = mergeRefs(ref, mergedProps.ref); return render({ ...mergedProps, ref: mergedRef } as HTMLProps, state); } @@ -103,7 +112,7 @@ export function renderElement< if (isValidElement(render)) { const elementRef = getElementRef(render); - const mergedRef = composeRefs(ref, mergedProps.ref, elementRef); + const mergedRef = mergeRefs(ref, mergedProps.ref, elementRef); const elementProps = mergeProps(mergedProps, render.props as Record); @@ -113,7 +122,7 @@ export function renderElement< } // Default tag - const mergedRef = composeRefs(ref, mergedProps.ref); + const mergedRef = mergeRefs(ref, mergedProps.ref); mergedProps.ref = mergedRef; From 0dfdc2d368ef5b81b0a966dfb98f282134901013 Mon Sep 17 00:00:00 2001 From: Rahim Date: Tue, 1 Sep 2026 16:34:49 -0700 Subject: [PATCH 3/3] fix(skin): improve generated skin parity --- packages/skins/build/packages/react.ts | 6 +-- .../skins/build/packages/tests/react.test.ts | 2 +- packages/skins/build/target/html.tsx | 9 ++-- packages/skins/build/target/react.tsx | 1 + packages/skins/build/tests/vite.test.ts | 17 ++++++- packages/skins/build/transform.ts | 1 + packages/skins/dev/controls.ts | 5 ++ packages/skins/dev/main.tsx | 3 ++ packages/skins/dev/options.ts | 4 ++ packages/skins/dev/styles.css | 47 ++++++++++++------- .../src/components/buttons/airplay-button.tsx | 11 ++--- .../src/components/buttons/button-tooltip.tsx | 4 +- .../components/buttons/captions-button.tsx | 11 ++--- .../src/components/buttons/cast-button.tsx | 11 ++--- .../components/buttons/fullscreen-button.tsx | 13 ++--- .../src/components/buttons/pip-button.tsx | 11 ++--- .../src/components/buttons/play-button.tsx | 16 +++---- .../src/components/buttons/seek-button.tsx | 14 +++--- .../components/controls/volume-popover.tsx | 4 +- .../src/components/menus/captions-menu.tsx | 21 ++------- .../src/components/menus/settings-menu.tsx | 5 +- .../src/skins/audio/error-dialog.styles.ts | 4 +- .../skins/src/skins/audio/play-button.tsx | 5 +- .../skins/src/skins/audio/settings-menu.tsx | 5 +- .../src/skins/audio/time-slider.styles.ts | 7 +-- .../skins/src/skins/audio/time-slider.tsx | 3 +- .../src/skins/default-audio/controls.tsx | 11 +++-- .../src/skins/default-live-audio/controls.tsx | 2 +- .../src/skins/default-live-video/controls.tsx | 21 +++++++-- .../src/skins/default-live-video/skin.tsx | 11 ++--- .../src/skins/default-video/controls.tsx | 25 +++++++--- .../skins/src/skins/default-video/skin.tsx | 11 ++--- .../skins/minimal-audio/controls.styles.ts | 1 + .../src/skins/minimal-audio/controls.tsx | 14 ++++-- .../minimal-live-audio/controls.styles.ts | 1 + .../src/skins/minimal-live-audio/controls.tsx | 5 +- .../src/skins/minimal-live-video/controls.tsx | 21 +++++++-- .../src/skins/minimal-live-video/skin.tsx | 11 ++--- .../src/skins/minimal-video/controls.tsx | 25 +++++++--- .../skins/src/skins/minimal-video/skin.tsx | 11 ++--- .../skins/shared/live-playback-hotkeys.tsx | 4 +- .../src/skins/shared/playback-hotkeys.tsx | 12 ++--- packages/skins/src/skins/video/gestures.tsx | 4 +- packages/skins/src/skins/video/skin.styles.ts | 11 +++++ .../skins/src/styles/buttons/button.styles.ts | 3 ++ .../src/styles/buttons/seek-button.styles.ts | 4 ++ .../feedback/status-indicator.styles.ts | 2 +- .../feedback/volume-indicator.styles.ts | 2 +- .../src/styles/layout/container.styles.ts | 1 - .../src/styles/layout/controls.styles.ts | 2 +- .../skins/src/styles/menus/menu.styles.ts | 11 ++++- .../skins/src/styles/popups/dialog.styles.ts | 4 +- .../skins/src/styles/popups/popup.styles.ts | 13 +++-- .../skins/src/styles/popups/tooltip.styles.ts | 5 +- .../skins/src/styles/sliders/slider.styles.ts | 2 +- .../src/styles/sliders/time-slider.styles.ts | 2 +- .../styles/sliders/volume-slider.styles.ts | 2 +- packages/skins/src/styles/tailwind.shared.css | 4 +- packages/skins/src/styles/themes/audio.css | 2 + packages/skins/src/styles/themes/theme.css | 2 +- packages/skins/src/styles/vars.ts | 4 ++ 61 files changed, 307 insertions(+), 199 deletions(-) create mode 100644 packages/skins/src/skins/video/skin.styles.ts diff --git a/packages/skins/build/packages/react.ts b/packages/skins/build/packages/react.ts index 7dc4b5bfe0..aebe3e903e 100644 --- a/packages/skins/build/packages/react.ts +++ b/packages/skins/build/packages/react.ts @@ -198,8 +198,6 @@ function reactSkinWrapper(options: { }): string { const props = `${options.component}Props`; const base = options.video ? 'BaseVideoSkinProps' : 'BaseSkinProps'; - const parameters = options.video ? `{ renderPoster, ...props }: ${props}` : `props: ${props}`; - const forwarded = options.video ? 'poster={renderPoster} {...props}' : '{...props}'; return `'use client'; @@ -209,8 +207,8 @@ import type { ${base} } from '../types'; export interface ${props} extends ${base} {} -export function ${options.component}(${parameters}) { - return ; +export function ${options.component}(props: ${props}) { + return ; } `; } diff --git a/packages/skins/build/packages/tests/react.test.ts b/packages/skins/build/packages/tests/react.test.ts index 03299196a2..9f55d70b79 100644 --- a/packages/skins/build/packages/tests/react.test.ts +++ b/packages/skins/build/packages/tests/react.test.ts @@ -20,7 +20,7 @@ describe('createReactPackageSkins', () => { ); expect(files.get('packages/react/src/presets/video/skin.tsx')).toContain('export interface VideoSkinProps'); - expect(files.get('packages/react/src/presets/video/skin.tsx')).toContain('poster={renderPoster}'); + expect(files.get('packages/react/src/presets/video/skin.tsx')).toContain(''); expect(files.get('packages/react/src/internal/skins/default-video/skin.tsx')).toContain( "from '../shared/components/button'" ); diff --git a/packages/skins/build/target/html.tsx b/packages/skins/build/target/html.tsx index ba179c469a..b6bb83d143 100644 --- a/packages/skins/build/target/html.tsx +++ b/packages/skins/build/target/html.tsx @@ -190,12 +190,14 @@ export const htmlComponentTarget: ComponentTarget = defineComponentT const controlledId = id(popup ? 'popup' : 'content'); if (popup) { - const playbackRateButtonProps = renderTargetProps(trigger.props, 'PlaybackRateButton'); + const componentProps = + renderTargetProps(trigger.props, 'CaptionsButton') ?? + renderTargetProps(trigger.props, 'PlaybackRateButton'); return [ trigger.replaceWith( - playbackRateButtonProps ? ( - + componentProps ? ( + {trigger.children} ) : ( @@ -309,6 +311,7 @@ export const htmlComponentTarget: ComponentTarget = defineComponentT target: () => htmlComponentTarget, targets: { Button: { element: Button }, + CaptionsButton: { element: htmlElementTarget('CaptionsButton', element), kind: 'component' }, PlaybackRateButton: { element: PlaybackRateButton, kind: 'component' }, SliderBuffer: { element: Div }, SliderFill: { element: Div }, diff --git a/packages/skins/build/target/react.tsx b/packages/skins/build/target/react.tsx index 8dbd2e5ce3..ca91e122fd 100644 --- a/packages/skins/build/target/react.tsx +++ b/packages/skins/build/target/react.tsx @@ -151,6 +151,7 @@ export const reactComponentTarget: ComponentTarget = defineComponent target: () => reactComponentTarget, targets: { Button: { element: Button }, + CaptionsButton: { element: Button, kind: 'component' }, PlaybackRateButton: { element: Button, kind: 'component' }, SliderBuffer: { element: Div }, SliderFill: { element: Div }, diff --git a/packages/skins/build/tests/vite.test.ts b/packages/skins/build/tests/vite.test.ts index 75dc1a8489..e4cc83d686 100644 --- a/packages/skins/build/tests/vite.test.ts +++ b/packages/skins/build/tests/vite.test.ts @@ -12,6 +12,9 @@ const defaultControlsUrl = `/../src/skins/default-video/controls.tsx${reactTarge const htmlContainerUrl = '/../src/components/layout/container.tsx?style=tailwind&target=html&skin=minimal-video'; const playButtonUrl = `/../src/components/buttons/play-button.tsx${reactTarget}`; const settingsMenuUrl = `/../src/components/menus/settings-menu.tsx${reactTarget}`; +const reactCaptionsMenuUrl = + '/../src/components/menus/captions-menu.tsx?style=css&target=react&skin=default-live-video'; +const htmlCaptionsMenuUrl = '/../src/components/menus/captions-menu.tsx?style=css&target=html&skin=default-live-video'; const htmlAudioSettingsMenuUrl = '/../src/skins/audio/settings-menu.tsx?style=css&target=html&skin=default-audio'; const volumePopoverUrl = `/../src/components/controls/volume-popover.tsx${reactTarget}`; const htmlPosterUrl = '/../src/components/layout/poster.tsx?style=tailwind&target=html&skin=default-video'; @@ -132,6 +135,7 @@ describe('Skins Vite workflow', () => { expect(settingsMenu?.code).toContain('children: /* @__PURE__ */ _jsxDEV(Menu.Trigger'); expect(settingsMenu?.code).toContain('render: /* @__PURE__ */ _jsxDEV(Button'); expect(settingsMenu?.code).toContain('resolveClassName(className, state)'); + expect(settingsMenu?.code).toContain('media-menu-resizable-popup'); expect(volumePopover?.code).toContain( 'VolumePopoverPrimitive.Trigger, { render: /* @__PURE__ */ _jsxDEV(MuteButton' ); @@ -143,7 +147,18 @@ describe('Skins Vite workflow', () => { const code = settingsMenu?.code ?? ''; expect(code).toContain('/components/buttons/playback-rate-button.tsx?skin=default-audio&style=css&target=html'); - expect(code).toMatch(/_jsxDEV\(PlaybackRateButton, \{\s+commandfor:[\s\S]*?class: className/); + expect(code).toMatch(/_jsxDEV\(PlaybackRateButton, \{\s+commandfor:[\s\S]*?className/); + expect(code).not.toContain('media-menu-resizable-popup'); + }, 30_000); + + it('uses the captions button itself as the menu trigger', async () => { + const react = await server.transformRequest(reactCaptionsMenuUrl); + const html = await server.transformRequest(htmlCaptionsMenuUrl); + + expect(react?.code).toMatch(/_jsxDEV\(MenuPrimitive\.Trigger, \{\s+render: .*_jsxDEV\(CaptionsButton/); + expect(react?.code).not.toContain('ButtonTooltip'); + expect(html?.code).toMatch(/_jsxDEV\(CaptionsButton, \{\s+commandfor:[\s\S]*?className/); + expect(html?.code).not.toContain('data-vjsc-render-captions-button'); }, 30_000); it('emits base visibility styles for stateful button icons', async () => { diff --git a/packages/skins/build/transform.ts b/packages/skins/build/transform.ts index 7af1034af5..7a30f0bc2f 100644 --- a/packages/skins/build/transform.ts +++ b/packages/skins/build/transform.ts @@ -30,6 +30,7 @@ export function resolveSkinStyles(module: TransformModule): StyleTransformOption export function validateSkinConfig(parameters: URLSearchParams): SkinTransformConfig | null { const target = parameters.get('target'); const skin = parameters.get('skin'); + const style = parameters.get('style'); if ((target !== 'react' && target !== 'html') || (style !== 'tailwind' && style !== 'css')) return null; diff --git a/packages/skins/dev/controls.ts b/packages/skins/dev/controls.ts index 46508cdabe..7103996731 100644 --- a/packages/skins/dev/controls.ts +++ b/packages/skins/dev/controls.ts @@ -40,6 +40,10 @@ function createOptions(preview: PreviewOptions): HTMLFormElement { ['css', 'CSS'], ['tailwind', 'Tailwind'], ]), + createSelect('scheme', 'Color scheme', preview.colorScheme, [ + ['dark', 'Dark'], + ['light', 'Light'], + ]), createSelect( 'media', 'Media', @@ -136,6 +140,7 @@ function createCopyButton(preview: PreviewOptions): HTMLButtonElement { `framework=${preview.framework}`, `skin=${preview.skin}`, `style=${preview.styleMode}`, + `scheme=${preview.colorScheme}`, `media=${preview.mediaId} (${preview.media.label})`, `captions=${preview.captionsMode}`, `width=${width}px (${formatRem(width)})`, diff --git a/packages/skins/dev/main.tsx b/packages/skins/dev/main.tsx index 1e0534b51a..b4ee030ee5 100644 --- a/packages/skins/dev/main.tsx +++ b/packages/skins/dev/main.tsx @@ -14,6 +14,9 @@ import './styles.css'; const captions = new URL('./captions.vtt', import.meta.url).href; const preview = readPreviewOptions(); + +document.documentElement.dataset.colorScheme = preview.colorScheme; + const Skin = await loadSkin(preview); if (preview.styleMode === 'tailwind') await import('../src/styles/tailwind.compiler.css'); diff --git a/packages/skins/dev/options.ts b/packages/skins/dev/options.ts index 8fd38261eb..cb455fce94 100644 --- a/packages/skins/dev/options.ts +++ b/packages/skins/dev/options.ts @@ -27,6 +27,7 @@ export const errorSource = { export const mediaIds = [...SOURCE_IDS, 'error'] as const; export type CaptionsMode = 'multiple' | 'single'; +export type ColorScheme = 'dark' | 'light'; export type Framework = 'html' | 'react'; export type MediaId = (typeof mediaIds)[number]; export type SkinName = @@ -42,6 +43,7 @@ export type StyleMode = 'css' | 'tailwind'; export interface PreviewOptions { readonly captionsMode: CaptionsMode; + readonly colorScheme: ColorScheme; readonly framework: Framework; readonly isAudio: boolean; readonly isLive: boolean; @@ -64,6 +66,7 @@ export function readPreviewOptions(search = location.search): PreviewOptions { const isLive = skin.includes('-live-'); const styleMode = params.get('style') === 'tailwind' ? 'tailwind' : 'css'; const captionsMode = params.get('captions') === 'multiple' ? 'multiple' : 'single'; + const colorScheme = params.get('scheme') === 'light' ? 'light' : 'dark'; const requestedMedia = params.get('media'); const mediaId = isMediaId(requestedMedia) ? requestedMedia : isLive ? 'hls-live' : 'mp4-1'; const requestedWidth = Number.parseInt(params.get('width') ?? '', 10); @@ -75,6 +78,7 @@ export function readPreviewOptions(search = location.search): PreviewOptions { return { captionsMode, + colorScheme, framework, isAudio, isLive, diff --git a/packages/skins/dev/styles.css b/packages/skins/dev/styles.css index 90f7d6f3f9..2f81bccbc1 100644 --- a/packages/skins/dev/styles.css +++ b/packages/skins/dev/styles.css @@ -4,10 +4,18 @@ body, min-height: 100%; } +:root { + color-scheme: dark; +} + +:root[data-color-scheme="light"] { + color-scheme: light; +} + body { margin: 0; - background: #111; - color: white; + background: light-dark(#f5f5f5, #111); + color: light-dark(#111, white); font-family: Inter, ui-sans-serif, system-ui, sans-serif; } @@ -23,6 +31,10 @@ body { aspect-ratio: 16 / 9; } +#root[data-media-kind="audio"] { + margin-top: 10rem; +} + #root[data-media-kind="audio"] .preview-player { aspect-ratio: auto; } @@ -35,15 +47,15 @@ body { align-items: end; width: 100%; padding: 1rem; - border-bottom: 1px solid rgb(255 255 255 / 15%); - background: #181818; + border-bottom: 1px solid light-dark(rgb(0 0 0 / 15%), rgb(255 255 255 / 15%)); + background: light-dark(#fff, #181818); } .preview-control { display: grid; gap: 0.25rem; min-width: 8rem; - color: rgb(255 255 255 / 70%); + color: light-dark(rgb(0 0 0 / 70%), rgb(255 255 255 / 70%)); font-size: 0.75rem; font-weight: 600; } @@ -55,13 +67,12 @@ body { .preview-control select { min-height: 2.25rem; padding: 0.375rem 2rem 0.375rem 0.625rem; - color-scheme: dark; - color: white; + color: light-dark(#111, white); font: inherit; font-size: 0.875rem; - border: 1px solid rgb(255 255 255 / 20%); + border: 1px solid light-dark(rgb(0 0 0 / 20%), rgb(255 255 255 / 20%)); border-radius: 0.375rem; - background: #272727; + background: light-dark(#f5f5f5, #272727); } .preview-control select:focus-visible { @@ -88,7 +99,7 @@ body { } .preview-copy:focus-visible { - outline: 2px solid white; + outline: 2px solid light-dark(#111, white); outline-offset: 2px; } @@ -97,11 +108,11 @@ body { display: grid; gap: 0.625rem; width: min(60rem, calc(100% - 2rem)); - margin: 1rem auto 0; + margin: 1rem auto; padding: 0.75rem 1rem; - border: 1px solid rgb(255 255 255 / 15%); + border: 1px solid light-dark(rgb(0 0 0 / 15%), rgb(255 255 255 / 15%)); border-radius: 0.5rem; - background: #181818; + background: light-dark(#fff, #181818); } .preview-width-header { @@ -114,7 +125,7 @@ body { } .preview-width-header output { - color: rgb(255 255 255 / 70%); + color: light-dark(rgb(0 0 0 / 70%), rgb(255 255 255 / 70%)); font-variant-numeric: tabular-nums; } @@ -137,12 +148,12 @@ body { .preview-width-presets button { padding: 0.25rem 0.5rem; - color: rgb(255 255 255 / 75%); + color: light-dark(rgb(0 0 0 / 75%), rgb(255 255 255 / 75%)); font: inherit; font-size: 0.75rem; - border: 1px solid rgb(255 255 255 / 15%); + border: 1px solid light-dark(rgb(0 0 0 / 15%), rgb(255 255 255 / 15%)); border-radius: 0.375rem; - background: #272727; + background: light-dark(#f5f5f5, #272727); cursor: pointer; } @@ -154,6 +165,6 @@ body { } .preview-width-presets button:focus-visible { - outline: 2px solid white; + outline: 2px solid light-dark(#111, white); outline-offset: 2px; } diff --git a/packages/skins/src/components/buttons/airplay-button.tsx b/packages/skins/src/components/buttons/airplay-button.tsx index 96ca830fc9..cdbb57d561 100644 --- a/packages/skins/src/components/buttons/airplay-button.tsx +++ b/packages/skins/src/components/buttons/airplay-button.tsx @@ -7,16 +7,13 @@ import type { SkinComponentMeta } from '../../meta'; import styles from '../../styles/buttons/airplay-button.styles'; import buttonStyles from '../../styles/buttons/button.styles'; import { Button } from './button'; -import { ButtonTooltip } from './button-tooltip'; export function AirPlayButton({ className, ...props }: Props = {}) { return ( - - <$.AirPlayButton $render={Button} className={[styles.root, className]} {...props}> - - - - + <$.AirPlayButton $render={Button} className={[styles.root, className]} {...props}> + + + ); } diff --git a/packages/skins/src/components/buttons/button-tooltip.tsx b/packages/skins/src/components/buttons/button-tooltip.tsx index 6d9a63428b..c0bc709b1c 100644 --- a/packages/skins/src/components/buttons/button-tooltip.tsx +++ b/packages/skins/src/components/buttons/button-tooltip.tsx @@ -15,7 +15,9 @@ export function ButtonTooltip({ children, label, ...props }: PropsWithChildren <$.Tooltip.Trigger>{children} - <$.Tooltip.Popup className={[popupStyles.popup, popupStyles.transition, popupStyles.surface, styles.popup]}> + <$.Tooltip.Popup + className={[popupStyles.popup, popupStyles.safeArea, popupStyles.transition, popupStyles.surface, styles.popup]} + > {label ?? <$.Tooltip.Label />} {!label && <$.Tooltip.Shortcut className={styles.shortcut} />} diff --git a/packages/skins/src/components/buttons/captions-button.tsx b/packages/skins/src/components/buttons/captions-button.tsx index c858cf5bff..959e2a3a39 100644 --- a/packages/skins/src/components/buttons/captions-button.tsx +++ b/packages/skins/src/components/buttons/captions-button.tsx @@ -7,16 +7,13 @@ import type { SkinComponentMeta } from '../../meta'; import buttonStyles from '../../styles/buttons/button.styles'; import styles from '../../styles/buttons/captions-button.styles'; import { Button } from './button'; -import { ButtonTooltip } from './button-tooltip'; export function CaptionsButton({ className, ...props }: Props = {}) { return ( - - <$.CaptionsButton $render={Button} className={[styles.root, className]} {...props}> - - - - + <$.CaptionsButton $render={Button} className={[styles.root, className]} {...props}> + + + ); } diff --git a/packages/skins/src/components/buttons/cast-button.tsx b/packages/skins/src/components/buttons/cast-button.tsx index cbcc7b4719..112d6bcda8 100644 --- a/packages/skins/src/components/buttons/cast-button.tsx +++ b/packages/skins/src/components/buttons/cast-button.tsx @@ -7,16 +7,13 @@ import type { SkinComponentMeta } from '../../meta'; import buttonStyles from '../../styles/buttons/button.styles'; import styles from '../../styles/buttons/cast-button.styles'; import { Button } from './button'; -import { ButtonTooltip } from './button-tooltip'; export function CastButton({ className, ...props }: Props = {}) { return ( - - <$.CastButton $render={Button} className={[styles.root, className]} {...props}> - - - - + <$.CastButton $render={Button} className={[styles.root, className]} {...props}> + + + ); } diff --git a/packages/skins/src/components/buttons/fullscreen-button.tsx b/packages/skins/src/components/buttons/fullscreen-button.tsx index 8221747353..6ce6b4e7ab 100644 --- a/packages/skins/src/components/buttons/fullscreen-button.tsx +++ b/packages/skins/src/components/buttons/fullscreen-button.tsx @@ -7,16 +7,13 @@ import type { SkinComponentMeta } from '../../meta'; import buttonStyles from '../../styles/buttons/button.styles'; import styles from '../../styles/buttons/fullscreen-button.styles'; import { Button } from './button'; -import { ButtonTooltip } from './button-tooltip'; export function FullscreenButton({ className, ...props }: Props = {}) { return ( - - <$.FullscreenButton $render={Button} className={[styles.root, className]} {...props}> - - - - + <$.FullscreenButton $render={Button} className={[styles.root, className]} {...props}> + + + ); } @@ -24,5 +21,5 @@ export const meta = { name: 'fullscreen-button', type: 'component', title: 'Fullscreen Button', - description: 'A button that enters and exits fullscreen with state-aware icons and an accessible tooltip.', + description: 'A button that enters and exits fullscreen with state-aware icons.', } as const satisfies SkinComponentMeta; diff --git a/packages/skins/src/components/buttons/pip-button.tsx b/packages/skins/src/components/buttons/pip-button.tsx index 29593e5921..1924035624 100644 --- a/packages/skins/src/components/buttons/pip-button.tsx +++ b/packages/skins/src/components/buttons/pip-button.tsx @@ -7,16 +7,13 @@ import type { SkinComponentMeta } from '../../meta'; import buttonStyles from '../../styles/buttons/button.styles'; import styles from '../../styles/buttons/pip-button.styles'; import { Button } from './button'; -import { ButtonTooltip } from './button-tooltip'; export function PiPButton({ className, ...props }: Props = {}) { return ( - - <$.PiPButton $render={Button} className={[styles.root, className]} {...props}> - - - - + <$.PiPButton $render={Button} className={[styles.root, className]} {...props}> + + + ); } diff --git a/packages/skins/src/components/buttons/play-button.tsx b/packages/skins/src/components/buttons/play-button.tsx index a467817760..59be3c4774 100644 --- a/packages/skins/src/components/buttons/play-button.tsx +++ b/packages/skins/src/components/buttons/play-button.tsx @@ -7,17 +7,14 @@ import type { SkinComponentMeta } from '../../meta'; import buttonStyles from '../../styles/buttons/button.styles'; import styles from '../../styles/buttons/play-button.styles'; import { Button } from './button'; -import { ButtonTooltip } from './button-tooltip'; export function PlayButton({ className, ...props }: Props = {}) { return ( - - <$.PlayButton $render={Button} className={[styles.root, className]} {...props}> - - - - - + <$.PlayButton $render={Button} className={[styles.root, className]} {...props}> + + + + ); } @@ -25,6 +22,5 @@ export const meta = { name: 'play-button', type: 'component', title: 'Play Button', - description: - 'A three-state button that plays, pauses, or restarts media with matching icons and an accessible tooltip.', + description: 'A three-state button that plays, pauses, or restarts media with matching icons.', } as const satisfies SkinComponentMeta; diff --git a/packages/skins/src/components/buttons/seek-button.tsx b/packages/skins/src/components/buttons/seek-button.tsx index 983774a615..51f9722b14 100644 --- a/packages/skins/src/components/buttons/seek-button.tsx +++ b/packages/skins/src/components/buttons/seek-button.tsx @@ -1,24 +1,23 @@ import type { SeekButtonProps as CoreProps } from '@videojs/core'; import * as $ from '@videojs/core/vjsc'; import { SeekIcon } from '@videojs/icons/vjsc'; -import { type Props, Text } from 'vjsc/components'; +import { Box, type Props, Text } from 'vjsc/components'; import type { SkinComponentMeta } from '../../meta'; import buttonStyles from '../../styles/buttons/button.styles'; import styles from '../../styles/buttons/seek-button.styles'; import { Button } from './button'; -import { ButtonTooltip } from './button-tooltip'; export function SeekButton({ className, seconds = 10, ...props }: Props = {}) { return ( - - <$.SeekButton $render={Button} className={[styles.root, className]} seconds={seconds} {...props}> + <$.SeekButton $render={Button} className={[styles.root, className]} seconds={seconds} {...props}> + {Math.abs(seconds)} - - + + ); } @@ -26,6 +25,5 @@ export const meta = { name: 'seek-button', type: 'component', title: 'Seek Button', - description: - 'A button that skips playback forward or backward by a configurable number of seconds, with a direction-aware icon and accessible tooltip.', + description: 'A button that skips playback forward or backward with a direction-aware icon and value.', } as const satisfies SkinComponentMeta; diff --git a/packages/skins/src/components/controls/volume-popover.tsx b/packages/skins/src/components/controls/volume-popover.tsx index 7017bfe41c..914052cb00 100644 --- a/packages/skins/src/components/controls/volume-popover.tsx +++ b/packages/skins/src/components/controls/volume-popover.tsx @@ -29,7 +29,9 @@ export function VolumePopover({ - <$.VolumePopover.Popup className={[popupStyles.popup, popupStyles.transition, popupStyles.surface, styles.popup]}> + <$.VolumePopover.Popup + className={[popupStyles.popup, popupStyles.safeArea, popupStyles.transition, popupStyles.surface, styles.popup]} + > diff --git a/packages/skins/src/components/menus/captions-menu.tsx b/packages/skins/src/components/menus/captions-menu.tsx index 520a0eea83..457180b84a 100644 --- a/packages/skins/src/components/menus/captions-menu.tsx +++ b/packages/skins/src/components/menus/captions-menu.tsx @@ -1,35 +1,22 @@ import type { MenuProps } from '@videojs/core'; import * as $ from '@videojs/core/vjsc'; -import { CaptionsOffIcon, CaptionsOnIcon } from '@videojs/icons/vjsc'; -import { type ClassNameValue, type Props, Template } from 'vjsc/components'; +import { type Props, type PropsOf, Template } from 'vjsc/components'; import type { SkinComponentMeta } from '../../meta'; -import buttonStyles from '../../styles/buttons/button.styles'; -import captionsButtonStyles from '../../styles/buttons/captions-button.styles'; import styles from '../../styles/menus/menu.styles'; import popupStyles from '../../styles/popups/popup.styles'; -import { Button } from '../buttons/button'; -import { ButtonTooltip } from '../buttons/button-tooltip'; +import { CaptionsButton } from '../buttons/captions-button'; import { RadioItem } from './radio-item'; export interface CaptionsMenuProps extends MenuProps { - className?: ClassNameValue; + className?: PropsOf['className']; } export function CaptionsMenu({ className, ...props }: Props = {}) { return ( <$.Menu.Root side="top" align="center" boundary="viewport" {...props}> <$.CaptionsRadioGroup.Root> - - <$.Menu.Trigger - $render={Button} - aria-label="Enable captions" - className={[captionsButtonStyles.root, className]} - > - - - - + <$.Menu.Trigger $render={CaptionsButton} className={className} /> <$.Menu.Popup className={[popupStyles.popup, popupStyles.surface, styles.popup]}> <$.Menu.Content className={styles.content}> <$.CaptionsRadioGroup.Options className={styles.radioGroup}> diff --git a/packages/skins/src/components/menus/settings-menu.tsx b/packages/skins/src/components/menus/settings-menu.tsx index 60715adbf4..ebcfa8f31d 100644 --- a/packages/skins/src/components/menus/settings-menu.tsx +++ b/packages/skins/src/components/menus/settings-menu.tsx @@ -26,7 +26,10 @@ export function SettingsMenu({ children, className, ...props }: PropsWithChildre - <$.Menu.Popup keepMounted className={[popupStyles.popup, popupStyles.surface, styles.popup]}> + <$.Menu.Popup + keepMounted + className={[popupStyles.popup, popupStyles.surface, styles.popup, styles.resizablePopup]} + > <$.Menu.Content className={styles.content}>{children} diff --git a/packages/skins/src/skins/audio/error-dialog.styles.ts b/packages/skins/src/skins/audio/error-dialog.styles.ts index a1cdcbcf0c..b19f7c2c0d 100644 --- a/packages/skins/src/skins/audio/error-dialog.styles.ts +++ b/packages/skins/src/skins/audio/error-dialog.styles.ts @@ -34,7 +34,7 @@ export default styles({ }, title: { className: 'audio-dialog-title', - utilities: 'm-0 text-media-lg font-semibold leading-tight', + utilities: 'm-0 text-media font-semibold leading-tight', }, description: { className: 'audio-dialog-description', @@ -46,7 +46,7 @@ export default styles({ }, close: { className: 'audio-dialog-close', - utilities: 'h-media-control w-auto flex-none bg-media-accent px-3 font-medium text-media-accent-text', + utilities: 'h-media-control w-auto flex-none bg-media-primary! px-3 font-medium text-media-primary-foreground!', }, }, }); diff --git a/packages/skins/src/skins/audio/play-button.tsx b/packages/skins/src/skins/audio/play-button.tsx index fc3ab45ed3..4d0c847b1f 100644 --- a/packages/skins/src/skins/audio/play-button.tsx +++ b/packages/skins/src/skins/audio/play-button.tsx @@ -1,5 +1,6 @@ import { Box, type Props } from 'vjsc/components'; +import { ButtonTooltip } from '../../components/buttons/button-tooltip'; import { PlayButton } from '../../components/buttons/play-button'; import { BufferingIndicator } from '../../components/feedback/buffering-indicator'; import styles from './play-button.styles'; @@ -8,7 +9,9 @@ export function AudioPlayButton({ className, ...props }: Props = {}) { return ( - + + + ); } diff --git a/packages/skins/src/skins/audio/settings-menu.tsx b/packages/skins/src/skins/audio/settings-menu.tsx index fda7d6e1e7..404f44d0b9 100644 --- a/packages/skins/src/skins/audio/settings-menu.tsx +++ b/packages/skins/src/skins/audio/settings-menu.tsx @@ -1,7 +1,6 @@ import type { MenuProps } from '@videojs/core'; -import { speedText } from '@videojs/core/i18n/text/menu'; import * as $ from '@videojs/core/vjsc'; -import { type Props, type PropsOf, Template, Text } from 'vjsc/components'; +import { type Props, type PropsOf, Template } from 'vjsc/components'; import { ButtonTooltip } from '../../components/buttons/button-tooltip'; import { PlaybackRateButton } from '../../components/buttons/playback-rate-button'; @@ -17,7 +16,7 @@ export function AudioSettingsMenu({ return ( <$.Menu.Root side="top" align="center" boundary="viewport" {...props}> <$.PlaybackRateRadioGroup.Root> - {speedText.text}} side="top"> + <$.Menu.Trigger $render={PlaybackRateButton} className={className} /> <$.Menu.Popup className={[popupStyles.popup, popupStyles.surface, styles.popup, audioSettingsMenuStyles.popup]}> diff --git a/packages/skins/src/skins/audio/time-slider.styles.ts b/packages/skins/src/skins/audio/time-slider.styles.ts index 046e4e4c07..4704e6a21d 100644 --- a/packages/skins/src/skins/audio/time-slider.styles.ts +++ b/packages/skins/src/skins/audio/time-slider.styles.ts @@ -14,13 +14,10 @@ export default styles({ }, previewContent: { className: 'audio-time-slider-preview-content', - utilities: ['bottom-[calc(100%+--spacing(10))] rounded-media-control px-2.5 py-1 tabular-nums', 'text-media'], + utilities: 'bottom-[calc(100%+--spacing(10))] tabular-nums', variants: { default: 'left-1/2', - minimal: [ - '[left:var(--media-preview-left,var(--media-slider-pointer))]', - 'rounded-[--spacing(2)] px-2 after:hidden', - ], + minimal: ['[left:var(--media-preview-left,var(--media-slider-pointer))]', 'after:hidden'], }, }, value: { diff --git a/packages/skins/src/skins/audio/time-slider.tsx b/packages/skins/src/skins/audio/time-slider.tsx index af2a2a2247..92385e1e75 100644 --- a/packages/skins/src/skins/audio/time-slider.tsx +++ b/packages/skins/src/skins/audio/time-slider.tsx @@ -4,6 +4,7 @@ import { Box, type Props } from 'vjsc/components'; import { SliderBuffer, SliderFill, SliderThumb, SliderTrack } from '../../components/sliders/slider'; import popupStyles from '../../styles/popups/popup.styles'; +import tooltipStyles from '../../styles/popups/tooltip.styles'; import sliderStyles from '../../styles/sliders/slider.styles'; import styles from './time-slider.styles'; @@ -20,7 +21,7 @@ export function AudioTimeSlider({ <$.TimeSlider.Thumb $render={SliderThumb} className={styles.thumb} /> <$.TimeSlider.Preview className={sliderStyles.preview} overflow={previewOverflow}> - + <$.TimeSlider.Value className={styles.value} type="pointer" /> diff --git a/packages/skins/src/skins/default-audio/controls.tsx b/packages/skins/src/skins/default-audio/controls.tsx index db5c4c4e9f..feb43f676d 100644 --- a/packages/skins/src/skins/default-audio/controls.tsx +++ b/packages/skins/src/skins/default-audio/controls.tsx @@ -1,5 +1,6 @@ import * as $ from '@videojs/core/vjsc'; +import { ButtonTooltip } from '../../components/buttons/button-tooltip'; import { SeekButton } from '../../components/buttons/seek-button'; import { VolumePopover } from '../../components/controls/volume-popover'; import audioControlsStyles from '../../styles/layout/audio-controls.styles'; @@ -16,8 +17,12 @@ export function DefaultAudioControls() { <$.Tooltip.Provider> <$.Controls.Group className={styles.start}> - - + + + + + + <$.Controls.Group className={styles.timeSliderGroup}> @@ -28,7 +33,7 @@ export function DefaultAudioControls() { <$.Controls.Group className={styles.end}> - + diff --git a/packages/skins/src/skins/default-live-audio/controls.tsx b/packages/skins/src/skins/default-live-audio/controls.tsx index 420382fa0d..ad48659ec0 100644 --- a/packages/skins/src/skins/default-live-audio/controls.tsx +++ b/packages/skins/src/skins/default-live-audio/controls.tsx @@ -21,7 +21,7 @@ export function DefaultLiveAudioControls() {