diff --git a/packages/core/src/core/ui/menu/menu-core.ts b/packages/core/src/core/ui/menu/menu-core.ts index ebc6e572..5da6c57a 100644 --- a/packages/core/src/core/ui/menu/menu-core.ts +++ b/packages/core/src/core/ui/menu/menu-core.ts @@ -7,7 +7,7 @@ import { getTransitionFlags } from '../transition'; export type { PopoverAlign, PopoverSide }; export interface MenuProps { - /** Which side of the trigger the menu appears on. Root menus only. */ + /** Preferred side of the trigger for the menu. Root menus only. */ side?: PopoverSide | undefined; /** Alignment along the trigger's edge. Root menus only. */ align?: PopoverAlign | undefined; @@ -29,6 +29,7 @@ export interface MenuInput extends TransitionState {} export interface MenuState extends TransitionFlags { open: boolean; status: TransitionStatus; + /** Preferred side of the trigger for the menu. Root menus only. */ side: PopoverSide | undefined; align: PopoverAlign | undefined; /** Whether this menu is nested inside another menu's content. */ diff --git a/packages/core/src/core/ui/menu/menu-data-attrs.ts b/packages/core/src/core/ui/menu/menu-data-attrs.ts index dc65a4f3..b49824fd 100644 --- a/packages/core/src/core/ui/menu/menu-data-attrs.ts +++ b/packages/core/src/core/ui/menu/menu-data-attrs.ts @@ -6,7 +6,7 @@ import type { MenuState } from './menu-core'; export const MenuDataAttrs = { /** Present when the menu is open. */ open: 'data-open', - /** Popover positioning side. Absent on submenus. */ + /** Rendered positioning side after collision handling. Absent on submenus. */ side: 'data-side', /** Popover positioning alignment. Absent on submenus. */ align: 'data-align', diff --git a/packages/core/src/core/ui/popover/popover-core.ts b/packages/core/src/core/ui/popover/popover-core.ts index 203de30f..176b9bfe 100644 --- a/packages/core/src/core/ui/popover/popover-core.ts +++ b/packages/core/src/core/ui/popover/popover-core.ts @@ -9,7 +9,7 @@ export type PopoverSide = 'top' | 'bottom' | 'left' | 'right'; export type PopoverAlign = 'start' | 'center' | 'end'; export interface PopoverProps { - /** Which side of the trigger the popup appears on. */ + /** Preferred side of the trigger for the popup. */ side?: PopoverSide | undefined; /** Alignment of the popup along the trigger's edge. */ align?: PopoverAlign | undefined; @@ -45,6 +45,7 @@ export interface PopoverInput extends TransitionState {} export interface PopoverState extends TransitionFlags { open: boolean; status: TransitionStatus; + /** Preferred side of the trigger for the popup. */ side: PopoverSide; align: PopoverAlign; modal: boolean | 'trap-focus'; diff --git a/packages/core/src/core/ui/popover/popover-data-attrs.ts b/packages/core/src/core/ui/popover/popover-data-attrs.ts index 0d53f26c..73d74c98 100644 --- a/packages/core/src/core/ui/popover/popover-data-attrs.ts +++ b/packages/core/src/core/ui/popover/popover-data-attrs.ts @@ -5,7 +5,7 @@ import type { PopoverState } from './popover-core'; export const PopoverDataAttrs = { /** Present when the popover is open. */ open: 'data-open', - /** Indicates which side the popover is positioned relative to the trigger. */ + /** Indicates the rendered side of the popover after collision handling. */ side: 'data-side', /** Indicates how the popover is aligned relative to the specified side. */ align: 'data-align', diff --git a/packages/core/src/core/ui/tooltip/tooltip-core.ts b/packages/core/src/core/ui/tooltip/tooltip-core.ts index ad0d1eaa..1ae51b7f 100644 --- a/packages/core/src/core/ui/tooltip/tooltip-core.ts +++ b/packages/core/src/core/ui/tooltip/tooltip-core.ts @@ -6,7 +6,7 @@ import type { TransitionFlags, TransitionState, TransitionStatus } from '../tran import { getTransitionFlags } from '../transition'; export interface TooltipProps { - /** Which side of the trigger the tooltip appears on. */ + /** Preferred side of the trigger for the tooltip. */ side?: PopoverSide | undefined; /** Alignment of the tooltip along the trigger's edge. */ align?: PopoverAlign | undefined; @@ -31,7 +31,7 @@ export interface TooltipState extends TransitionFlags { open: boolean; /** Current phase of the transition lifecycle. */ status: TransitionStatus; - /** Which side of the trigger the tooltip is positioned on. */ + /** Preferred side of the trigger for the tooltip. */ side: PopoverSide; /** How the tooltip is aligned relative to the specified side. */ align: PopoverAlign; diff --git a/packages/core/src/core/ui/tooltip/tooltip-data-attrs.ts b/packages/core/src/core/ui/tooltip/tooltip-data-attrs.ts index 90886a06..f298ac57 100644 --- a/packages/core/src/core/ui/tooltip/tooltip-data-attrs.ts +++ b/packages/core/src/core/ui/tooltip/tooltip-data-attrs.ts @@ -5,7 +5,7 @@ import type { TooltipState } from './tooltip-core'; export const TooltipDataAttrs = { /** Present when the tooltip is open. */ open: 'data-open', - /** Indicates which side the tooltip is positioned relative to the trigger. */ + /** Indicates the rendered side of the tooltip after collision handling. */ side: 'data-side', /** Indicates how the tooltip is aligned relative to the specified side. */ align: 'data-align', diff --git a/packages/core/src/dom/ui/popover/popover-positioning.ts b/packages/core/src/dom/ui/popover/popover-positioning.ts index 063fcfa5..0576f214 100644 --- a/packages/core/src/dom/ui/popover/popover-positioning.ts +++ b/packages/core/src/dom/ui/popover/popover-positioning.ts @@ -4,6 +4,8 @@ import type { PopoverAlign, PopoverSide } from '../../../core/ui/popover/popover import { type PopoverCSSVarKey, PopoverCSSVars } from '../../../core/ui/popover/popover-css-vars'; import { createDOMRect } from '../../utils/layout'; +export { getPositionedSide } from '@videojs/utils/dom'; + export interface PositioningOptions { side: PopoverSide; align: PopoverAlign; @@ -56,22 +58,6 @@ function formatPixels(value: number): string { return `${clamp(value, 0, Infinity)}px`; } -function getCrossAxisAvailable( - start: number, - end: number, - size: number, - boundaryStart: number, - boundaryEnd: number, - align: PopoverAlign, - alignOffset: number -): number { - if (align === 'start') return boundaryEnd - (start + alignOffset); - if (align === 'end') return end + alignOffset - boundaryStart; - - const center = start + size / 2 + alignOffset; - return Math.min(center - boundaryStart, boundaryEnd - center) * 2; -} - function shiftCrossAxis(value: number, boundaryStart: number, boundaryEnd: number, size: number): number { const max = boundaryEnd - size; return max < boundaryStart ? boundaryStart : clamp(value, boundaryStart, max); @@ -134,13 +120,10 @@ export function getAnchorPositionStyle( if (triggerRect && popupRect) { const resolved: ManualOffsets = offsets ?? ZERO_OFFSETS; return { + position: 'fixed', + margin: '0', ...getManualPositionStyle(triggerRect, popupRect, opts, resolved, boundaryRect), ...(boundaryRect ? getPositioningCSSVars(triggerRect, boundaryRect, opts, resolved, cssVars) : {}), - position: 'fixed', - // Reset UA [popover] defaults (inset: 0; margin: auto) which would - // otherwise conflict with computed positioning. - inset: 'auto', - margin: '0', }; } @@ -266,7 +249,7 @@ export function getPositioningCSSVars( cssVars: PositioningCSSVars = PopoverCSSVars ): Record { const vars: Record = {}; - const { side, align } = opts; + const { side } = opts; const boundaryOffset = offsets.boundaryOffset ?? 0; const boundaryStartX = boundaryRect.left + boundaryOffset; const boundaryEndX = boundaryRect.right - boundaryOffset; @@ -280,32 +263,12 @@ export function getPositioningCSSVars( const sideSpace = side === 'top' ? triggerRect.top - boundaryStartY : boundaryEndY - triggerRect.bottom; vars[cssVars.availableHeight] = formatPixels(sideSpace - offsets.sideOffset); - vars[cssVars.availableWidth] = formatPixels( - getCrossAxisAvailable( - triggerRect.left, - triggerRect.right, - triggerRect.width, - boundaryStartX, - boundaryEndX, - align, - offsets.alignOffset - ) - ); + vars[cssVars.availableWidth] = formatPixels(boundaryEndX - boundaryStartX); } else { const sideSpace = side === 'left' ? triggerRect.left - boundaryStartX : boundaryEndX - triggerRect.right; vars[cssVars.availableWidth] = formatPixels(sideSpace - offsets.sideOffset); - vars[cssVars.availableHeight] = formatPixels( - getCrossAxisAvailable( - triggerRect.top, - triggerRect.bottom, - triggerRect.height, - boundaryStartY, - boundaryEndY, - align, - offsets.alignOffset - ) - ); + vars[cssVars.availableHeight] = formatPixels(boundaryEndY - boundaryStartY); } return vars; @@ -355,16 +318,18 @@ export function getManualPositionStyle( const { side, align } = opts; const { sideOffset, alignOffset } = offsets; let top = 0; + let bottom: string | undefined; let left = 0; + let right: string | undefined; // Side positioning in viewport coordinates. // Positive sideOffset always increases distance from the trigger. if (side === 'top') { - top = triggerRect.top - popupRect.height - sideOffset; + bottom = `calc(100% - ${triggerRect.top}px + ${sideOffset}px)`; } else if (side === 'bottom') { top = triggerRect.bottom + sideOffset; } else if (side === 'left') { - left = triggerRect.left - popupRect.width - sideOffset; + right = `calc(100% - ${triggerRect.left}px + ${sideOffset}px)`; } else { left = triggerRect.right + sideOffset; } @@ -409,8 +374,10 @@ export function getManualPositionStyle( } return { - top: `${top}px`, - left: `${left}px`, + top: side === 'top' ? 'auto' : `${top}px`, + bottom: bottom ?? 'auto', + left: side === 'left' ? 'auto' : `${left}px`, + right: right ?? 'auto', }; } @@ -432,12 +399,18 @@ export function resolveOffsets(el: Element, cssVars: PositioningCSSVars = Popove * * `getBoundingClientRect()` includes active transforms, which causes the * fallback position to drift while opening/closing animations scale the popup. - * Using `offsetWidth`/`offsetHeight` preserves the untransformed size. + * Using layout dimensions preserves the untransformed size, while the + * side-axis scroll dimension includes content clipped by size constraints. */ -export function getPopupPositionRect(el: HTMLElement): DOMRect { +export function getPopupPositionRect(el: HTMLElement, side: PopoverSide): DOMRect { const rect = el.getBoundingClientRect(); const width = el.offsetWidth || rect.width; const height = el.offsetHeight || rect.height; - return createDOMRect(rect.left, rect.top, width, height); + return createDOMRect( + rect.left, + rect.top, + side === 'left' || side === 'right' ? Math.max(width, el.scrollWidth) : width, + side === 'top' || side === 'bottom' ? Math.max(height, el.scrollHeight) : height + ); } diff --git a/packages/core/src/dom/ui/popover/tests/popover-positioning.test.ts b/packages/core/src/dom/ui/popover/tests/popover-positioning.test.ts index 37509d7b..f03ebaa4 100644 --- a/packages/core/src/dom/ui/popover/tests/popover-positioning.test.ts +++ b/packages/core/src/dom/ui/popover/tests/popover-positioning.test.ts @@ -41,8 +41,8 @@ describe('getManualPositionStyle', () => { it('positions above trigger for side=top', () => { const style = getManualPositionStyle(trigger, popup, { side: 'top', align: 'center' }); - // top = trigger.top - popup.height = 200 - 80 = 120 - expect(style.top).toBe('120px'); + expect(style.bottom).toBe('calc(100% - 200px + 0px)'); + expect(style.top).toBe('auto'); // left = trigger.left + (trigger.width - popup.width)/2 = 100 + (120-200)/2 = 60 expect(style.left).toBe('60px'); }); @@ -57,8 +57,8 @@ describe('getManualPositionStyle', () => { it('positions to the left of trigger for side=left', () => { const style = getManualPositionStyle(trigger, popup, { side: 'left', align: 'center' }); - // left = trigger.left - popup.width = 100 - 200 = -100 - expect(style.left).toBe('-100px'); + expect(style.right).toBe('calc(100% - 100px + 0px)'); + expect(style.left).toBe('auto'); }); it('positions to the right of trigger for side=right', () => { @@ -68,12 +68,23 @@ describe('getManualPositionStyle', () => { expect(style.left).toBe('220px'); }); + it('positions top and left popups independently of their side-axis size', () => { + const shortPopup = makeDOMRect(0, 0, popup.width, 20); + const narrowPopup = makeDOMRect(0, 0, 20, popup.height); + + expect(getManualPositionStyle(trigger, shortPopup, { side: 'top', align: 'center' }).bottom).toBe( + 'calc(100% - 200px + 0px)' + ); + expect(getManualPositionStyle(trigger, narrowPopup, { side: 'left', align: 'center' }).right).toBe( + 'calc(100% - 100px + 0px)' + ); + }); + it('applies sideOffset from resolved CSS vars', () => { const offsets: ManualOffsets = { sideOffset: 8, alignOffset: 0 }; const style = getManualPositionStyle(trigger, popup, { side: 'top', align: 'center' }, offsets); - // top = 200 - 80 - 8 = 112 - expect(style.top).toBe('112px'); + expect(style.bottom).toBe('calc(100% - 200px + 8px)'); }); it('applies sideOffset for bottom side', () => { @@ -134,7 +145,7 @@ describe('getManualPositionStyle', () => { boundary ); - expect(topStyle.top).toBe('50px'); + expect(topStyle.bottom).toBe('calc(100% - 100px + 0px)'); expect(topStyle.left).toBe('200px'); expect(bottomStyle.top).toBe('120px'); expect(bottomStyle.left).toBe('0px'); @@ -164,7 +175,7 @@ describe('getManualPositionStyle', () => { expect(rightStyle.top).toBe('120px'); expect(rightStyle.left).toBe('140px'); expect(leftStyle.top).toBe('0px'); - expect(leftStyle.left).toBe('20px'); + expect(leftStyle.right).toBe('calc(100% - 100px + 0px)'); }); it('respects boundary offset when shifting cross-axis overflow', () => { @@ -181,6 +192,7 @@ describe('getManualPositionStyle', () => { boundary ); + expect(style.bottom).toBe('auto'); expect(style.top).toBe('120px'); expect(style.left).toBe('188px'); }); @@ -248,7 +260,7 @@ describe('getPopoverCSSVars', () => { describe('getPositioningCSSVars', () => { const boundary = makeDOMRect(0, 0, 300, 200); - it('computes available size for center-aligned top and bottom popups', () => { + it('uses the boundary width for top and bottom popups', () => { const trigger = makeDOMRect(250, 150, 40, 20); const vars = getPositioningCSSVars( trigger, @@ -258,22 +270,22 @@ describe('getPositioningCSSVars', () => { ); expect(vars[PopoverCSSVars.availableHeight]).toBe('22px'); - expect(vars[PopoverCSSVars.availableWidth]).toBe('60px'); + expect(vars[PopoverCSSVars.availableWidth]).toBe('300px'); }); - it('applies align offset to start-aligned cross-axis size', () => { + it.each(['start', 'center', 'end'] as const)('does not reduce cross-axis size for %s alignment', (align) => { const trigger = makeDOMRect(250, 150, 40, 20); const vars = getPositioningCSSVars( trigger, boundary, - { side: 'bottom', align: 'start' }, + { side: 'bottom', align }, { sideOffset: 0, alignOffset: 10 } ); - expect(vars[PopoverCSSVars.availableWidth]).toBe('40px'); + expect(vars[PopoverCSSVars.availableWidth]).toBe('300px'); }); - it('computes available size for center-aligned left and right popups', () => { + it('uses the boundary height for left and right popups', () => { const trigger = makeDOMRect(120, 160, 40, 20); const vars = getPositioningCSSVars( trigger, @@ -283,7 +295,7 @@ describe('getPositioningCSSVars', () => { ); expect(vars[PopoverCSSVars.availableWidth]).toBe('128px'); - expect(vars[PopoverCSSVars.availableHeight]).toBe('60px'); + expect(vars[PopoverCSSVars.availableHeight]).toBe('200px'); }); it('subtracts boundary offset from side-axis and cross-axis sizes', () => { @@ -296,7 +308,7 @@ describe('getPositioningCSSVars', () => { ); expect(vars[PopoverCSSVars.availableHeight]).toBe('12px'); - expect(vars[PopoverCSSVars.availableWidth]).toBe('40px'); + expect(vars[PopoverCSSVars.availableWidth]).toBe('280px'); }); }); @@ -321,7 +333,8 @@ describe('getAnchorPositionStyle', () => { const style = getAnchorPositionStyle('my-anchor', { side: 'top', align: 'center' }, trigger, positioner, boundary); - expect(style.top).toBe('120px'); + expect(style.bottom).toBe('calc(100% - 200px + 0px)'); + expect(style.top).toBe('auto'); expect(style.left).toBe('60px'); expect(style.position).toBe('fixed'); // Also includes sizing CSS vars @@ -359,7 +372,7 @@ describe('getPopupPositionRect', () => { Object.defineProperty(el, 'offsetHeight', { configurable: true, value: 80 }); vi.spyOn(el, 'getBoundingClientRect').mockImplementation(() => makeDOMRect(20, 40, 100, 40)); - const rect = getPopupPositionRect(el); + const rect = getPopupPositionRect(el, 'top'); expect(rect.left).toBe(20); expect(rect.top).toBe(40); @@ -376,7 +389,7 @@ describe('getPopupPositionRect', () => { Object.defineProperty(el, 'offsetHeight', { configurable: true, value: 80 }); vi.spyOn(el, 'getBoundingClientRect').mockImplementation(() => makeDOMRect(20, 40, 100, 40)); - const rect = getPopupPositionRect(el); + const rect = getPopupPositionRect(el, 'top'); expect(rect.toJSON()).toEqual( expect.objectContaining({ @@ -389,6 +402,37 @@ describe('getPopupPositionRect', () => { }) ); }); + + it.each([ + ['top', 100, 80], + ['left', 120, 60], + ] as const)('includes overflow on the %s side axis', (side, expectedWidth, expectedHeight) => { + const el = document.createElement('div'); + vi.spyOn(el, 'getBoundingClientRect').mockImplementation(() => makeDOMRect(20, 40, 100, 60)); + Object.defineProperty(el, 'offsetWidth', { configurable: true, value: 100 }); + Object.defineProperty(el, 'offsetHeight', { configurable: true, value: 60 }); + Object.defineProperty(el, 'scrollWidth', { configurable: true, value: 120 }); + Object.defineProperty(el, 'scrollHeight', { configurable: true, value: 80 }); + + const rect = getPopupPositionRect(el, side); + expect(rect.width).toBe(expectedWidth); + expect(rect.height).toBe(expectedHeight); + }); + + it('does not change available-size styles while measuring', () => { + const el = document.createElement('div'); + el.style.setProperty(PopoverCSSVars.availableHeight, '20px'); + vi.spyOn(el, 'getBoundingClientRect').mockImplementation(() => { + expect(el.style.getPropertyValue(PopoverCSSVars.availableHeight)).toBe('20px'); + return makeDOMRect(20, 40, 100, 60); + }); + Object.defineProperty(el, 'offsetWidth', { configurable: true, value: 100 }); + Object.defineProperty(el, 'offsetHeight', { configurable: true, value: 20 }); + Object.defineProperty(el, 'scrollHeight', { configurable: true, value: 60 }); + + expect(getPopupPositionRect(el, 'top').height).toBe(60); + expect(el.style.getPropertyValue(PopoverCSSVars.availableHeight)).toBe('20px'); + }); }); // Tests the CSS anchor positioning path via getAnchorPositionStyle with diff --git a/packages/html/src/ui/menu/menu-element.ts b/packages/html/src/ui/menu/menu-element.ts index 7d0a1e26..2651b45d 100644 --- a/packages/html/src/ui/menu/menu-element.ts +++ b/packages/html/src/ui/menu/menu-element.ts @@ -10,6 +10,7 @@ import { getMenuViewportAttrs, getMenuViewTransitionAttrs, getPopupPositionRect, + getPositionedSide, getPositioningBoundaryRect, getRootPositionOptions, isMenuNavigationKey, @@ -263,16 +264,25 @@ export class MenuElement extends MediaElement { const boundaryRect = getPositioningBoundaryRect(boundaryElement); const offsets = resolveOffsets(this); const anchorSupported = supportsAnchorPositioning(); - const getNextStyle = () => - getAnchorPositionStyle( + if (!triggerRect) return; + + const getNextPosition = () => { + const popupRect = getPopupPositionRect(this, positionOptions.side); + const side = getPositionedSide(triggerRect, popupRect, boundaryRect, positionOptions, offsets); + const style = getAnchorPositionStyle( this.id, - positionOptions, + { ...positionOptions, side }, triggerRect, - anchorSupported ? undefined : getPopupPositionRect(this), + anchorSupported ? undefined : popupRect, boundaryRect, offsets ); - let nextStyle = getNextStyle(); + + return { side, style }; + }; + let nextPosition = getNextPosition(); + let nextStyle = nextPosition.style; + this.setAttribute(MenuDataAttrs.side, nextPosition.side); if (anchorSupported) { applyStyles(this, nextStyle); @@ -282,7 +292,9 @@ export class MenuElement extends MediaElement { syncMenuViewRoot(this, this.#navState.stack.length > 0, availableWidth ? { availableWidth } : undefined); if (!anchorSupported) { - nextStyle = getNextStyle(); + nextPosition = getNextPosition(); + nextStyle = nextPosition.style; + this.setAttribute(MenuDataAttrs.side, nextPosition.side); applyStyles(this, nextStyle); } 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 ebf3b648..96958302 100644 --- a/packages/html/src/ui/menu/tests/menu-element.test.ts +++ b/packages/html/src/ui/menu/tests/menu-element.test.ts @@ -107,10 +107,40 @@ function expectNoMenuStateAttrs(element: HTMLElement): void { } afterEach(() => { + vi.restoreAllMocks(); document.body.innerHTML = ''; }); describe('MenuElement', () => { + it('exposes the positioned side on root content', async () => { + const trigger = document.createElement('button'); + const root = createElement(MenuElement); + const view = createElement(MenuViewElement); + const item = createElement(MenuItemElement); + + root.id = 'menu'; + root.open = true; + root.side = 'top'; + root.boundary = 'viewport'; + trigger.setAttribute('commandfor', root.id); + item.textContent = 'Auto'; + view.append(item); + root.append(view); + + vi.spyOn(trigger, 'getBoundingClientRect').mockReturnValue(new DOMRect(100, 10, 40, 20)); + vi.spyOn(root, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, 100, 60)); + vi.spyOn(document.documentElement, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, 300, 200)); + Object.defineProperty(root, 'offsetWidth', { configurable: true, value: 100 }); + Object.defineProperty(root, 'offsetHeight', { configurable: true, value: 5 }); + Object.defineProperty(root, 'scrollHeight', { configurable: true, value: 60 }); + root.style.setProperty('--media-popover-available-height', '5px'); + + document.body.append(trigger, root); + await root.updateComplete; + + expect(root.getAttribute('data-side')).toBe('bottom'); + }); + it('scopes menu state data attributes to menu elements', async () => { const root = createElement(MenuElement); const label = createElement(MenuGroupLabelElement); diff --git a/packages/html/src/ui/popover/popover-element.ts b/packages/html/src/ui/popover/popover-element.ts index 2bc82d0d..fceb7f20 100644 --- a/packages/html/src/ui/popover/popover-element.ts +++ b/packages/html/src/ui/popover/popover-element.ts @@ -7,6 +7,7 @@ import { getAnchorNameStyle, getAnchorPositionStyle, getPopupPositionRect, + getPositionedSide, getPositioningBoundaryRect, type PopoverApi, type PopoverChangeDetails, @@ -181,18 +182,24 @@ export class PopoverElement extends MediaElement { } // Apply positioning styles to self. - const posOpts = { side: state.side, align: state.align }; + const preferredOpts = { side: state.side, align: state.align }; const boundaryElement = this.#getBoundaryElement(); const triggerRect = this.#currentTrigger?.getBoundingClientRect(); const boundaryRect = getPositioningBoundaryRect(boundaryElement); const offsets = resolveOffsets(this); + const popupRect = getPopupPositionRect(this, preferredOpts.side); + + if (!triggerRect) return; + + const side = getPositionedSide(triggerRect, popupRect, boundaryRect, preferredOpts, offsets); + const posOpts = { ...preferredOpts, side }; + this.setAttribute(PopoverDataAttrs.side, side); if (supportsAnchorPositioning()) { applyStyles(this, getAnchorPositionStyle(this.id, posOpts, triggerRect, undefined, boundaryRect, offsets)); } else { // JS fallback: measure rects and resolve CSS var offsets. - const selfRect = getPopupPositionRect(this); - applyStyles(this, getAnchorPositionStyle(this.id, posOpts, triggerRect, selfRect, boundaryRect, offsets)); + applyStyles(this, getAnchorPositionStyle(this.id, posOpts, triggerRect, popupRect, boundaryRect, offsets)); } this.#position.sync(this.#currentTrigger, boundaryElement); diff --git a/packages/html/src/ui/popover/tests/popover-element.test.ts b/packages/html/src/ui/popover/tests/popover-element.test.ts new file mode 100644 index 00000000..3f68fbb9 --- /dev/null +++ b/packages/html/src/ui/popover/tests/popover-element.test.ts @@ -0,0 +1,44 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { PopoverElement } from '../popover-element'; + +let tagCounter = 0; + +function createPopover(): PopoverElement { + const tag = `test-popover-${tagCounter++}`; + customElements.define(tag, class extends PopoverElement {}); + return document.createElement(tag) as PopoverElement; +} + +function makeDOMRect(x: number, y: number, width: number, height: number): DOMRect { + return new DOMRect(x, y, width, height); +} + +afterEach(() => { + vi.restoreAllMocks(); + document.body.innerHTML = ''; +}); + +describe('PopoverElement', () => { + it('exposes the positioned side on the popup', async () => { + const trigger = document.createElement('button'); + const popover = createPopover(); + + popover.id = 'popover'; + popover.open = true; + popover.side = 'top'; + popover.boundary = 'viewport'; + trigger.setAttribute('commandfor', popover.id); + + vi.spyOn(trigger, 'getBoundingClientRect').mockReturnValue(makeDOMRect(100, 10, 40, 20)); + vi.spyOn(popover, 'getBoundingClientRect').mockReturnValue(makeDOMRect(0, 0, 100, 60)); + vi.spyOn(document.documentElement, 'getBoundingClientRect').mockReturnValue(makeDOMRect(0, 0, 300, 200)); + Object.defineProperty(popover, 'offsetWidth', { configurable: true, value: 100 }); + Object.defineProperty(popover, 'offsetHeight', { configurable: true, value: 60 }); + + document.body.append(trigger, popover); + await popover.updateComplete; + + expect(popover.getAttribute('data-side')).toBe('bottom'); + }); +}); diff --git a/packages/html/src/ui/tooltip/tests/tooltip-element.test.ts b/packages/html/src/ui/tooltip/tests/tooltip-element.test.ts index ebc3b9f9..4884c55a 100644 --- a/packages/html/src/ui/tooltip/tests/tooltip-element.test.ts +++ b/packages/html/src/ui/tooltip/tests/tooltip-element.test.ts @@ -4,7 +4,7 @@ import { HOTKEY_SHORTCUT_CHANGE_EVENT, playbackFeature } from '@videojs/core/dom import { registerI18n, resetI18nRegistry } from '@videojs/core/i18n'; import { ContextProvider } from '@videojs/element/context'; import { createState, createStore } from '@videojs/store'; -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { MediaI18nProviderElement } from '../../../i18n'; import { playerContext } from '../../../player/context'; @@ -97,11 +97,30 @@ function setup() { defineElement(TestPlayerProviderElement.tagName, TestPlayerProviderElement); afterEach(() => { + vi.restoreAllMocks(); resetI18nRegistry(); document.body.innerHTML = ''; }); describe('TooltipElement', () => { + it('exposes the positioned side on the popup', async () => { + const { tooltip, trigger } = setup(); + + tooltip.open = true; + tooltip.side = 'top'; + tooltip.boundary = 'viewport'; + + vi.spyOn(trigger, 'getBoundingClientRect').mockReturnValue(new DOMRect(100, 10, 40, 20)); + vi.spyOn(tooltip, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, 100, 60)); + vi.spyOn(document.documentElement, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, 300, 200)); + Object.defineProperty(tooltip, 'offsetWidth', { configurable: true, value: 100 }); + Object.defineProperty(tooltip, 'offsetHeight', { configurable: true, value: 60 }); + + await tooltip.updateComplete; + + expect(tooltip.getAttribute('data-side')).toBe('bottom'); + }); + it('creates default label and shortcut elements for empty tooltips', async () => { const { tooltip } = setup(); diff --git a/packages/html/src/ui/tooltip/tooltip-element.ts b/packages/html/src/ui/tooltip/tooltip-element.ts index bd0ae923..dccb377a 100644 --- a/packages/html/src/ui/tooltip/tooltip-element.ts +++ b/packages/html/src/ui/tooltip/tooltip-element.ts @@ -14,6 +14,7 @@ import { getAnchorNameStyle, getAnchorPositionStyle, getPopupPositionRect, + getPositionedSide, getPositioningBoundaryRect, HOTKEY_SHORTCUT_CHANGE_EVENT, type PositioningBoundary, @@ -205,11 +206,18 @@ export class TooltipElement extends MediaElement { } // Apply positioning styles to self. - const posOpts = { side: state.side, align: state.align }; + const preferredOpts = { side: state.side, align: state.align }; const boundaryElement = this.#getBoundaryElement(); const triggerRect = this.#currentTrigger?.getBoundingClientRect(); const boundaryRect = getPositioningBoundaryRect(boundaryElement); const offsets = resolveOffsets(this, TooltipCSSVars); + const popupRect = getPopupPositionRect(this, preferredOpts.side); + + if (!triggerRect) return; + + const side = getPositionedSide(triggerRect, popupRect, boundaryRect, preferredOpts, offsets); + const posOpts = { ...preferredOpts, side }; + this.setAttribute(TooltipDataAttrs.side, side); if (supportsAnchorPositioning()) { applyStyles( @@ -218,10 +226,9 @@ export class TooltipElement extends MediaElement { ); } else { // JS fallback: measure rects and resolve CSS var offsets. - const selfRect = getPopupPositionRect(this); applyStyles( this, - getAnchorPositionStyle(this.id, posOpts, triggerRect, selfRect, boundaryRect, offsets, TooltipCSSVars) + getAnchorPositionStyle(this.id, posOpts, triggerRect, popupRect, boundaryRect, offsets, TooltipCSSVars) ); } diff --git a/packages/react/src/ui/hooks/tests/use-positioned-state.test.tsx b/packages/react/src/ui/hooks/tests/use-positioned-state.test.tsx new file mode 100644 index 00000000..02081c1a --- /dev/null +++ b/packages/react/src/ui/hooks/tests/use-positioned-state.test.tsx @@ -0,0 +1,27 @@ +import { act, renderHook } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import { usePositionedState } from '../use-positioned-state'; + +describe('usePositionedState', () => { + it('uses the preferred side on the first closed render', () => { + const sides: string[] = []; + const { result, rerender } = renderHook( + ({ open }: { open: boolean }) => { + const positioned = usePositionedState<{ open: boolean; side: 'top' | 'bottom' }>({ open, side: 'top' }); + sides.push(positioned.state.side); + return positioned; + }, + { initialProps: { open: true } } + ); + + act(() => result.current.setPositionedSide('bottom')); + expect(result.current.state.side).toBe('bottom'); + + sides.length = 0; + rerender({ open: false }); + + expect(sides[0]).toBe('top'); + expect(result.current.state.side).toBe('top'); + }); +}); diff --git a/packages/react/src/ui/hooks/use-positioned-state.ts b/packages/react/src/ui/hooks/use-positioned-state.ts new file mode 100644 index 00000000..785f6cc9 --- /dev/null +++ b/packages/react/src/ui/hooks/use-positioned-state.ts @@ -0,0 +1,43 @@ +'use client'; + +import { useCallback, useEffect, useMemo, useState } from 'react'; + +interface PopupState { + open: boolean; + side: string | undefined; +} + +interface PositionedState { + state: State; + preferredSide: State['side']; + setPositionedSide: (side: State['side']) => void; +} + +export function usePositionedState(preferredState: State): PositionedState { + const preferredSide = preferredState.side; + const [position, setPosition] = useState<{ preferred: State['side']; side: State['side'] }>({ + preferred: preferredSide, + side: preferredSide, + }); + + const side = preferredState.open && position.preferred === preferredSide ? position.side : preferredSide; + + const state = useMemo( + () => (side === preferredSide ? preferredState : { ...preferredState, side }), + [side, preferredState, preferredSide] + ); + + const setPositionedSide = useCallback( + (nextSide: State['side']) => + setPosition((prev) => + prev.preferred === preferredSide && prev.side === nextSide ? prev : { preferred: preferredSide, side: nextSide } + ), + [preferredSide] + ); + + useEffect(() => { + if (!preferredState.open) setPositionedSide(preferredSide); + }, [preferredState.open, preferredSide, setPositionedSide]); + + return { state, preferredSide, setPositionedSide }; +} diff --git a/packages/react/src/ui/menu/context.tsx b/packages/react/src/ui/menu/context.tsx index 74d43875..8c224aac 100644 --- a/packages/react/src/ui/menu/context.tsx +++ b/packages/react/src/ui/menu/context.tsx @@ -10,6 +10,8 @@ export interface MenuContextValue { core: MenuCore; menu: MenuApi; state: MenuState; + preferredSide: MenuState['side']; + setPositionedSide: (side: MenuState['side']) => void; stateAttrMap: StateAttrMap; contentId: string; anchorName: string; diff --git a/packages/react/src/ui/menu/menu-content.tsx b/packages/react/src/ui/menu/menu-content.tsx index 7008fcee..ccaff6e2 100644 --- a/packages/react/src/ui/menu/menu-content.tsx +++ b/packages/react/src/ui/menu/menu-content.tsx @@ -8,6 +8,7 @@ import { getMenuViewportElement, getMenuViewTransitionAttrs, getPopupPositionRect, + getPositionedSide, getPositioningBoundaryRect, getRootPositionOptions, isEventWithinElement, @@ -109,8 +110,19 @@ export const MenuContent = forwardRef(function { render, className, style, onKeyDown, onBlur, ...elementProps }, forwardedRef ) { - const { core, menu, state, stateAttrMap, anchorName, contentId, boundary, container, activeSubMenuId } = - useMenuContext(); + const { + core, + menu, + state, + preferredSide, + setPositionedSide, + stateAttrMap, + anchorName, + contentId, + boundary, + container, + activeSubMenuId, + } = useMenuContext(); const subMenuCtx = useSubMenuContext(); const isSubmenu = state.isSubmenu; @@ -229,7 +241,10 @@ export const MenuContent = forwardRef(function const rootComposedRef = useComposedRefs(forwardedRef, contentRef, internalRef); const menuViewComposedRef = useComposedRefs(forwardedRef, setMenuViewElement); - const positionOptions = useMemo(() => getRootPositionOptions(state.side, state.align), [state.side, state.align]); + const positionOptions = useMemo( + () => getRootPositionOptions(preferredSide, state.align), + [preferredSide, state.align] + ); const anchorStyle = useMemo(() => { if (isSubmenu || !positionOptions || !supportsAnchorPositioning()) return null; @@ -237,7 +252,7 @@ export const MenuContent = forwardRef(function return rest as CSSProperties; }, [isSubmenu, anchorName, positionOptions]); - const [manualStyle, setManualStyle] = useState(null); + const [position, setPosition] = useState(null); useLayoutEffect(() => { if (isSubmenu) return; @@ -264,7 +279,7 @@ export const MenuContent = forwardRef(function useLayoutEffect(() => { if (isSubmenu) return; if (!state.open) { - setManualStyle(null); + setPosition(null); return; } @@ -284,15 +299,16 @@ export const MenuContent = forwardRef(function const root = contentElement.getRootNode() as Document | ShadowRoot; const boundaryElement = resolvePositioningBoundary(boundary, { container, root }); const anchorSupported = supportsAnchorPositioning(); - const contentRect = anchorSupported ? undefined : getPopupPositionRect(contentElement); + let contentRect = getPopupPositionRect(contentElement, rootPositionOptions.side); const boundaryRect = getPositioningBoundaryRect(boundaryElement); const offsets = resolveOffsets(contentElement); + let side = getPositionedSide(triggerRect, contentRect, boundaryRect, rootPositionOptions, offsets); let nextStyle = getAnchorPositionStyle( anchorName, - rootPositionOptions, + { ...rootPositionOptions, side }, triggerRect, - contentRect, + anchorSupported ? undefined : contentRect, boundaryRect, offsets ); @@ -305,11 +321,13 @@ export const MenuContent = forwardRef(function ); if (!anchorSupported) { + contentRect = getPopupPositionRect(contentElement, rootPositionOptions.side); + side = getPositionedSide(triggerRect, contentRect, boundaryRect, rootPositionOptions, offsets); nextStyle = getAnchorPositionStyle( anchorName, - rootPositionOptions, + { ...rootPositionOptions, side }, triggerRect, - getPopupPositionRect(contentElement), + contentRect, boundaryRect, offsets ); @@ -317,7 +335,8 @@ export const MenuContent = forwardRef(function const { positionAnchor: _, ...rootStyle } = nextStyle; - setManualStyle(rootStyle as CSSProperties); + setPosition(rootStyle as CSSProperties); + setPositionedSide(side); } measure(); @@ -356,7 +375,7 @@ export const MenuContent = forwardRef(function window.removeEventListener('scroll', reposition, true); window.removeEventListener('resize', reposition); }; - }, [isSubmenu, state.open, anchorName, positionOptions, menu, boundary, container]); + }, [isSubmenu, state.open, anchorName, positionOptions, menu, boundary, container, setPositionedSide]); // ─── Render ─────────────────────────────────────────────────────────────── @@ -393,7 +412,7 @@ export const MenuContent = forwardRef(function if (!state.open) return null; - const positioningStyle = manualStyle ?? anchorStyle ?? POPOVER_RESET; + const positioningStyle = position ?? anchorStyle ?? POPOVER_RESET; return renderElement( 'div', diff --git a/packages/react/src/ui/menu/menu-root.tsx b/packages/react/src/ui/menu/menu-root.tsx index 45ae240f..90c9e4a6 100644 --- a/packages/react/src/ui/menu/menu-root.tsx +++ b/packages/react/src/ui/menu/menu-root.tsx @@ -10,6 +10,7 @@ import { useDestroy } from '../../utils/use-destroy'; import { useLatestRef } from '../../utils/use-latest-ref'; import { useSafeId } from '../../utils/use-safe-id'; import { useOptionalControlsContext } from '../controls/context'; +import { usePositionedState } from '../hooks/use-positioned-state'; import { MenuContextProvider, SubMenuContextProvider, useOptionalMenuContext } from './context'; export interface MenuRootProps extends MenuCore.Props { @@ -98,11 +99,12 @@ export function MenuRoot({ useDestroy(menu); const input = useSnapshot(menu.input); - const state = useMemo(() => { + const preferredState = useMemo(() => { core.setProps({ side, align, closeOnEscape, closeOnOutsideClick, isSubmenu }); core.setInput(input); return core.getState(); }, [core, input, side, align, closeOnEscape, closeOnOutsideClick, isSubmenu]); + const { state, preferredSide, setPositionedSide } = usePositionedState(preferredState); // Subscribe to navigation state — used by Content/Trigger when this is a root menu. const navigationInput = useSnapshot(menu.navigationInput); @@ -116,6 +118,8 @@ export function MenuRoot({ core, menu, state, + preferredSide, + setPositionedSide, stateAttrMap: MenuDataAttrs, contentId, anchorName, @@ -131,6 +135,8 @@ export function MenuRoot({ core, menu, state, + preferredSide, + setPositionedSide, contentId, anchorName, boundary, diff --git a/packages/react/src/ui/menu/tests/menu.test.tsx b/packages/react/src/ui/menu/tests/menu.test.tsx index bb9de2d7..6859620e 100644 --- a/packages/react/src/ui/menu/tests/menu.test.tsx +++ b/packages/react/src/ui/menu/tests/menu.test.tsx @@ -19,7 +19,14 @@ import { MenuSeparator } from '../menu-separator'; import { MenuTrigger } from '../menu-trigger'; import { MenuView } from '../menu-view'; -afterEach(cleanup); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +function makeDOMRect(x: number, y: number, width: number, height: number): DOMRect { + return new DOMRect(x, y, width, height); +} function SubmenuFixture() { return ( @@ -374,6 +381,40 @@ function DynamicMenuFixture({ showCaptions }: { showCaptions: boolean }) { } describe('MenuContent', () => { + it('exposes the positioned side on root content', async () => { + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function (this: HTMLElement) { + if (this.dataset.testid === 'trigger') return makeDOMRect(100, 10, 40, 20); + if (this.dataset.testid === 'content') return makeDOMRect(0, 0, 100, 60); + return makeDOMRect(0, 0, 300, 200); + }); + vi.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation(function (this: HTMLElement) { + return this.dataset.testid === 'content' ? 100 : 0; + }); + vi.spyOn(HTMLElement.prototype, 'offsetHeight', 'get').mockImplementation(function (this: HTMLElement) { + return this.dataset.testid === 'content' ? 60 : 0; + }); + + render( + +