Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions packages/core/src/core/ui/menu/core.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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). */
Expand All @@ -22,6 +24,8 @@ export interface MenuProps {
closeOnOutsideClick?: boolean | undefined;
}

type MenuCoreProps = Omit<MenuProps, 'boundary'>;

export interface MenuTriggerProps {
disabled?: boolean | undefined;
}
Expand Down Expand Up @@ -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<MenuProps> = {
static readonly defaultProps: NonNullableObject<MenuCoreProps> = {
side: 'bottom',
align: 'start',
open: false,
Expand All @@ -103,15 +107,15 @@ export class MenuCore {
#props = { ...MenuCore.defaultProps };
#input: MenuInput | null = null;

get props(): Readonly<NonNullableObject<MenuProps>> {
get props(): Readonly<NonNullableObject<MenuCoreProps>> {
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);
}

Expand Down Expand Up @@ -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;
}
7 changes: 1 addition & 6 deletions packages/core/src/core/ui/menu/menu-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<MenuRootProps>(),
Root: defineComponent<MenuProps>(),
Trigger: defineComponent<MenuTriggerProps>(),
Popup: defineComponent<MenuPopupProps>(),
Content: defineComponent(),
Expand Down
14 changes: 10 additions & 4 deletions packages/core/src/core/ui/popover/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -35,6 +39,8 @@ export interface PopoverProps {
closeDelay?: number | undefined;
}

type PopoverCoreProps = Omit<PopoverProps, 'boundary'>;

/**
* The raw transition state managed by `createTransition`. Uses `active` (not `open`) to distinguish the generic
* transition state machine from the domain-specific `PopoverState.open`.
Expand All @@ -51,7 +57,7 @@ export interface PopoverState extends TransitionFlags {
}

export class PopoverCore {
static readonly defaultProps: NonNullableObject<PopoverProps> = {
static readonly defaultProps: NonNullableObject<PopoverCoreProps> = {
side: 'top',
align: 'center',
modal: false,
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
}
14 changes: 9 additions & 5 deletions packages/core/src/core/ui/tooltip/core.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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. */
Expand All @@ -26,6 +28,8 @@ export interface TooltipProps {
sticky?: boolean | undefined;
}

type TooltipCoreProps = Omit<TooltipProps, 'boundary'>;

export interface TooltipInput extends TransitionState {}

export interface TooltipState extends TransitionFlags {
Expand All @@ -40,7 +44,7 @@ export interface TooltipState extends TransitionFlags {
}

export class TooltipCore {
static readonly defaultProps: NonNullableObject<TooltipProps> = {
static readonly defaultProps: NonNullableObject<TooltipCoreProps> = {
side: 'top',
align: 'center',
open: false,
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
}
2 changes: 1 addition & 1 deletion packages/core/src/core/ui/volume-popover/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
21 changes: 19 additions & 2 deletions packages/core/src/dom/ui/menu/menu-popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
50 changes: 50 additions & 0 deletions packages/core/src/dom/ui/menu/tests/menu-popup.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
1 change: 1 addition & 0 deletions packages/core/src/dom/ui/popover/popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/dom/ui/popover/tests/popover.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/ui/menu/menu-root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<MenuCore.Props, 'boundary'> {
/** Boundary used to constrain the root menu popup size. */
boundary?: PositioningBoundary;
/** Called when the menu open state changes (fires immediately, before animations). */
Expand Down
8 changes: 5 additions & 3 deletions packages/react/src/ui/menu/tests/menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/ui/popover/popover-root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<CorePopoverProps, 'boundary'> {
/** Boundary used to constrain the popup size. */
boundary?: PositioningBoundary;
/** Called when the popover open state changes (fires immediately, before animations). */
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/ui/tooltip/tooltip-root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<CoreTooltipProps, 'boundary'> {
/** Boundary used to constrain the popup size. */
boundary?: PositioningBoundary;
/** Called when the tooltip open state changes (fires immediately, before animations). */
Expand Down
11 changes: 11 additions & 0 deletions packages/react/src/utils/tests/use-render.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<TestComponent ref={ref} />);
const element = ref.mock.calls[0]![0];

rerender(<TestComponent ref={ref} active />);

expect(ref).toHaveBeenCalledTimes(1);
expect(ref).toHaveBeenCalledWith(element);
});

it('forwards array of refs', () => {
const ref1 = createRef<HTMLDivElement>();
const ref2 = createRef<HTMLDivElement>();
Expand Down
Loading
Loading