From a7020529df3d5d8561fda3525ece2c33656ce6c6 Mon Sep 17 00:00:00 2001 From: kuekhaoyang Date: Wed, 4 Feb 2026 17:22:35 +0800 Subject: [PATCH] Refactor: improve dropdown menu positioning by calculating relative to container and adjust slider track hover area. --- app/styles/video-player.css | 5 +- components/player/desktop/DesktopMoreMenu.tsx | 77 +++++++-------- .../player/desktop/DesktopSpeedMenu.tsx | 98 ++++++++++--------- 3 files changed, 89 insertions(+), 91 deletions(-) diff --git a/app/styles/video-player.css b/app/styles/video-player.css index 353b39a..a5171c9 100644 --- a/app/styles/video-player.css +++ b/app/styles/video-player.css @@ -57,10 +57,11 @@ .slider-track::before { content: ''; position: absolute; - top: -16px; + top: -20px; left: 0; right: 0; - bottom: -16px; + bottom: -5px; + /* Reduced from -16px to avoid covering buttons below */ z-index: 1; } diff --git a/components/player/desktop/DesktopMoreMenu.tsx b/components/player/desktop/DesktopMoreMenu.tsx index 1b45ab5..8e2ba36 100644 --- a/components/player/desktop/DesktopMoreMenu.tsx +++ b/components/player/desktop/DesktopMoreMenu.tsx @@ -84,13 +84,26 @@ export function DesktopMoreMenu({ const calculateMenuPosition = React.useCallback(() => { if (!buttonRef.current || !containerRef.current) return; - const buttonRect = buttonRef.current.getBoundingClientRect(); - const containerRect = containerRef.current.getBoundingClientRect(); - const viewportHeight = window.innerHeight; + if (!buttonRef.current || !containerRef.current) return; - // Space available below and above the button - const spaceBelow = viewportHeight - buttonRect.bottom - 20; // 20px margin - const spaceAbove = buttonRect.top - containerRect.top - 20; + // Calculate position relative to container using offsetParent loop + let top = 0; + let left = 0; + let el: HTMLElement | null = buttonRef.current; + + while (el && el !== containerRef.current) { + top += el.offsetTop; + left += el.offsetLeft; + el = el.offsetParent as HTMLElement; + } + + const buttonHeight = buttonRef.current.offsetHeight; + const buttonWidth = buttonRef.current.offsetWidth; + const containerHeight = containerRef.current.offsetHeight; + + // Use container dimensions for available space + const spaceBelow = containerHeight - (top + buttonHeight) - 20; + const spaceAbove = top - 20; // Estimate menu height (or use actual if already rendered) const estimatedMenuHeight = 450; // approximate height of menu @@ -102,36 +115,26 @@ export function DesktopMoreMenu({ // Calculate max-height based on available space const maxHeight = openUpward ? Math.min(spaceAbove, actualMenuHeight) - : Math.min(spaceBelow, viewportHeight * 0.7); + : Math.min(spaceBelow, containerHeight * 0.7); if (openUpward) { setMenuPosition({ - top: buttonRect.top - containerRect.top - 10, // Position above button - left: buttonRect.left - containerRect.left, + top: top - 10, + left: left, maxHeight: `${maxHeight}px`, openUpward: true }); } else { setMenuPosition({ - top: buttonRect.bottom - containerRect.top + 10, - left: buttonRect.left - containerRect.left, + top: top + buttonHeight + 10, + left: left, maxHeight: `${maxHeight}px`, openUpward: false }); } }, [containerRef]); - // When rotated, use direct viewport positioning - const calculateRotatedPosition = React.useCallback(() => { - if (!buttonRef.current) return; - const buttonRect = buttonRef.current.getBoundingClientRect(); - setMenuPosition({ - top: buttonRect.bottom + 10, - left: buttonRect.left, - maxHeight: `${window.innerHeight * 0.6}px`, - openUpward: false - }); - }, []); + // Auto-close menu on scroll React.useEffect(() => { @@ -147,29 +150,15 @@ export function DesktopMoreMenu({ React.useEffect(() => { if (showMoreMenu) { - if (isRotated) { - calculateRotatedPosition(); - } else { - calculateMenuPosition(); - } - const timer = setTimeout(() => { - if (isRotated) { - calculateRotatedPosition(); - } else { - calculateMenuPosition(); - } - }, 50); + calculateMenuPosition(); + const timer = setTimeout(calculateMenuPosition, 50); return () => clearTimeout(timer); } - }, [showMoreMenu, calculateMenuPosition, calculateRotatedPosition, isRotated]); + }, [showMoreMenu, calculateMenuPosition]); const handleToggle = () => { if (!showMoreMenu) { - if (isRotated) { - calculateRotatedPosition(); - } else { - calculateMenuPosition(); - } + calculateMenuPosition(); } onToggleMoreMenu(); }; @@ -177,10 +166,10 @@ export function DesktopMoreMenu({ const MenuContent = (
{/* More Menu Dropdown (Portal) */} - {showMoreMenu && typeof document !== 'undefined' && createPortal(MenuContent, isRotated ? document.body : (containerRef.current || document.body))} + {showMoreMenu && containerRef.current && createPortal(MenuContent, containerRef.current)}
); } diff --git a/components/player/desktop/DesktopSpeedMenu.tsx b/components/player/desktop/DesktopSpeedMenu.tsx index 1badfaa..dff6357 100644 --- a/components/player/desktop/DesktopSpeedMenu.tsx +++ b/components/player/desktop/DesktopSpeedMenu.tsx @@ -51,13 +51,27 @@ export function DesktopSpeedMenu({ const calculateMenuPosition = React.useCallback(() => { if (!buttonRef.current || !containerRef.current) return; - const buttonRect = buttonRef.current.getBoundingClientRect(); - const containerRect = containerRef.current.getBoundingClientRect(); - const viewportHeight = window.innerHeight; + if (!buttonRef.current || !containerRef.current) return; - // Space available below and above the button - const spaceBelow = viewportHeight - buttonRect.bottom - 20; // 20px margin - const spaceAbove = buttonRect.top - containerRect.top - 20; + // Calculate position relative to container using offsetParent loop + // This works regardless of container rotation because we stay in the local coordinate system + let top = 0; + let left = 0; + let el: HTMLElement | null = buttonRef.current; + + while (el && el !== containerRef.current) { + top += el.offsetTop; + left += el.offsetLeft; + el = el.offsetParent as HTMLElement; + } + + const buttonHeight = buttonRef.current.offsetHeight; + const buttonWidth = buttonRef.current.offsetWidth; + const containerHeight = containerRef.current.offsetHeight; + + // Use container dimensions for available space + const spaceBelow = containerHeight - (top + buttonHeight) - 20; + const spaceAbove = top - 20; // Estimate menu height (or use actual if already rendered) const estimatedMenuHeight = 250; // approximate height of speed menu @@ -69,36 +83,31 @@ export function DesktopSpeedMenu({ // Calculate max-height based on available space const maxHeight = openUpward ? Math.min(spaceAbove, actualMenuHeight) - : Math.min(spaceBelow, viewportHeight * 0.7); + : Math.min(spaceBelow, containerHeight * 0.7); if (openUpward) { setMenuPosition({ - top: buttonRect.top - containerRect.top - 10, // Position above button - left: buttonRect.right - containerRect.left, + top: top - 10, + left: left + buttonWidth, // Right align? No, original was left: buttonRect.right - containerRect.left + // Original logic: left = buttonRect.right - containerRect.left. + // In local coords, buttonRect.right = left + buttonWidth. + // But we want to align the RIGHT edge of menu with RIGHT edge of button? + // CSS uses `transform: translateX(-100%)` and `left: ${menuPos.left}`. + // So left should be the right edge of the button. maxHeight: `${maxHeight}px`, openUpward: true }); } else { setMenuPosition({ - top: buttonRect.bottom - containerRect.top + 10, - left: buttonRect.right - containerRect.left, + top: top + buttonHeight + 10, + left: left + buttonWidth, maxHeight: `${maxHeight}px`, openUpward: false }); } }, [containerRef]); - // When rotated, use direct viewport positioning - const calculateRotatedPosition = React.useCallback(() => { - if (!buttonRef.current) return; - const buttonRect = buttonRef.current.getBoundingClientRect(); - setMenuPosition({ - top: buttonRect.bottom + 10, - left: buttonRect.right, - maxHeight: `${window.innerHeight * 0.6}px`, - openUpward: false - }); - }, []); + // Auto-close menu on scroll React.useEffect(() => { @@ -114,29 +123,15 @@ export function DesktopSpeedMenu({ React.useEffect(() => { if (showSpeedMenu) { - if (isRotated) { - calculateRotatedPosition(); - } else { - calculateMenuPosition(); - } - const timer = setTimeout(() => { - if (isRotated) { - calculateRotatedPosition(); - } else { - calculateMenuPosition(); - } - }, 50); + calculateMenuPosition(); + const timer = setTimeout(calculateMenuPosition, 50); return () => clearTimeout(timer); } - }, [showSpeedMenu, calculateMenuPosition, calculateRotatedPosition, isRotated]); + }, [showSpeedMenu, calculateMenuPosition]); const handleToggle = () => { if (!showSpeedMenu) { - if (isRotated) { - calculateRotatedPosition(); - } else { - calculateMenuPosition(); - } + calculateMenuPosition(); } onToggleSpeedMenu(); }; @@ -144,10 +139,10 @@ export function DesktopSpeedMenu({ const MenuContent = (
- {/* Speed Menu (Portal) */} - {showSpeedMenu && typeof document !== 'undefined' && createPortal(MenuContent, isRotated ? document.body : (containerRef.current || document.body))} + {/* Speed Menu (Portal) - Portal to container to inherit rotation but avoid overflow clipping if container has it? + Actually, the container usually has overflow-hidden. + If we portal to containerRef, it is inside the container div. + If the container div has overflow-hidden, the menu will be clipped. + BUT DesktopVideoPlayer structure: +
(relative, no overflow hidden?) +
(video wrapper) + + So containerRef itself (outer wrapper) seems to NOT have overflow-hidden in my memory? + Checking DesktopVideoPlayer.tsx: + className={`kvideo-container relative aspect-video ...`} + It does NOT have overflow-hidden. The inner div does. + So portaling to containerRef is SAFE and CORRECT. + */} + {showSpeedMenu && containerRef.current && createPortal(MenuContent, containerRef.current)}
); }