From 4c6e84aa97a1d3c55593ac2ba36fb24781519f5c Mon Sep 17 00:00:00 2001 From: Yousif Yassi Date: Tue, 6 May 2025 13:55:59 -0400 Subject: [PATCH 1/8] fix: OPTIC-2157: Actions Menu Disappears at High Browser Zoom Levels --- web/libs/core/src/lib/utils/dom.ts | 6 ++++-- .../src/components/Common/Dropdown/DropdownComponent.jsx | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/web/libs/core/src/lib/utils/dom.ts b/web/libs/core/src/lib/utils/dom.ts index 6ee9bc1c0390..669dc6cbff92 100644 --- a/web/libs/core/src/lib/utils/dom.ts +++ b/web/libs/core/src/lib/utils/dom.ts @@ -96,6 +96,7 @@ export const alignElements = ( align: Align = "bottom-left", padding = 0, constrainHeight = false, + openUpwardForShortViewport = true, ) => { let offsetLeft = 0; let offsetTop = 0; @@ -139,9 +140,9 @@ export const alignElements = ( break; } - if (offsetTop < window.scrollX) { + if (offsetTop < window.scrollY || !openUpwardForShortViewport) { offsetTop = pos.bottom + padding; - maxHeight = window.scrollX + window.innerHeight - offsetTop; + maxHeight = window.scrollY + window.innerHeight - offsetTop; resultAlign[0] = "bottom"; } // If the dropdown has more space on the top, then we should align it to the top @@ -159,6 +160,7 @@ export const alignElements = ( offsetLeft = pos.horizontalRight; resultAlign[1] = "right"; } + console.log("offsetTop", offsetTop, openUpwardForShortViewport, elem, target, resultAlign); const { scrollY, scrollX } = window; diff --git a/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx b/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx index df644cf33449..45061d496410 100644 --- a/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx +++ b/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx @@ -24,6 +24,7 @@ export const Dropdown = React.forwardRef(({ animated = true, visible = false, .. const [visibility, setVisibility] = React.useState(visible ? "visible" : null); const calculatePosition = React.useCallback(() => { + console.log("calculatePosition", openUpwardForShortViewport); const dropdownEl = dropdown.current; const parent = triggerRef?.current ?? dropdownEl.parentNode; const { left, top } = alignElements( @@ -31,11 +32,12 @@ export const Dropdown = React.forwardRef(({ animated = true, visible = false, .. dropdownEl, align ?? "bottom-left", 0, + false, openUpwardForShortViewport ?? true, ); setOffset({ left, top }); - }, [triggerRef]); + }, [triggerRef, align, openUpwardForShortViewport]); const dropdownIndex = React.useMemo(() => { return lastIndex++; From bb62b379a45ba5c8b93919d768b1fa58f60c1d30 Mon Sep 17 00:00:00 2001 From: Yousif Yassi Date: Tue, 6 May 2025 13:59:39 -0400 Subject: [PATCH 2/8] should be keying off scrollY for height --- web/libs/core/src/lib/utils/dom.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web/libs/core/src/lib/utils/dom.ts b/web/libs/core/src/lib/utils/dom.ts index 669dc6cbff92..46e3ee445b23 100644 --- a/web/libs/core/src/lib/utils/dom.ts +++ b/web/libs/core/src/lib/utils/dom.ts @@ -124,17 +124,17 @@ export const alignElements = ( case "bottom-center": offsetTop = pos.bottom + padding; offsetLeft = pos.horizontalCenter; - maxHeight = window.scrollX + window.innerHeight - offsetTop; + maxHeight = window.scrollY + window.innerHeight - offsetTop; break; case "bottom-left": offsetTop = pos.bottom + padding; offsetLeft = pos.horizontalLeft; - maxHeight = window.scrollX + window.innerHeight - offsetTop; + maxHeight = window.scrollY + window.innerHeight - offsetTop; break; case "bottom-right": offsetTop = pos.bottom + padding; offsetLeft = pos.horizontalRight; - maxHeight = window.scrollX + window.innerHeight - offsetTop; + maxHeight = window.scrollY + window.innerHeight - offsetTop; break; default: break; From 314762cb42d298bf6c91158051ffaa8b55644680 Mon Sep 17 00:00:00 2001 From: Yousif Yassi Date: Tue, 6 May 2025 14:00:57 -0400 Subject: [PATCH 3/8] minor fix for scrollX being used on height stuff --- web/libs/core/src/lib/utils/dom.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/web/libs/core/src/lib/utils/dom.ts b/web/libs/core/src/lib/utils/dom.ts index 46e3ee445b23..6c958dd01f69 100644 --- a/web/libs/core/src/lib/utils/dom.ts +++ b/web/libs/core/src/lib/utils/dom.ts @@ -160,7 +160,6 @@ export const alignElements = ( offsetLeft = pos.horizontalRight; resultAlign[1] = "right"; } - console.log("offsetTop", offsetTop, openUpwardForShortViewport, elem, target, resultAlign); const { scrollY, scrollX } = window; From 161cc23ceb1eaaab121a3356c9a2cc24d973f1fb Mon Sep 17 00:00:00 2001 From: bmartel Date: Tue, 6 May 2025 13:02:33 -0500 Subject: [PATCH 4/8] Update web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx --- .../src/components/Common/Dropdown/DropdownComponent.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx b/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx index 45061d496410..bda4cb44356c 100644 --- a/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx +++ b/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx @@ -24,7 +24,6 @@ export const Dropdown = React.forwardRef(({ animated = true, visible = false, .. const [visibility, setVisibility] = React.useState(visible ? "visible" : null); const calculatePosition = React.useCallback(() => { - console.log("calculatePosition", openUpwardForShortViewport); const dropdownEl = dropdown.current; const parent = triggerRef?.current ?? dropdownEl.parentNode; const { left, top } = alignElements( From efe0cc99746d88dce466eb85a1c0d5bab1554f95 Mon Sep 17 00:00:00 2001 From: Brandon Martel Date: Tue, 6 May 2025 16:04:33 -0500 Subject: [PATCH 5/8] use a prop from the component instead of hardcoding constrainHeight --- .../Common/Dropdown/DropdownComponent.jsx | 308 +++++++++--------- 1 file changed, 155 insertions(+), 153 deletions(-) diff --git a/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx b/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx index bda4cb44356c..4b97409313f6 100644 --- a/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx +++ b/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx @@ -10,162 +10,164 @@ import { DropdownTrigger } from "./DropdownTrigger"; let lastIndex = 1; -export const Dropdown = React.forwardRef(({ animated = true, visible = false, ...props }, ref) => { - const rootName = cn("dropdown-dm"); - - /**@type {import('react').RefObject} */ - const dropdown = React.useRef(); - const { triggerRef } = React.useContext(DropdownContext) ?? {}; - const isInline = triggerRef === undefined; - - const { children, align, openUpwardForShortViewport } = props; - const [currentVisible, setVisible] = React.useState(visible); - const [offset, setOffset] = React.useState({}); - const [visibility, setVisibility] = React.useState(visible ? "visible" : null); - - const calculatePosition = React.useCallback(() => { - const dropdownEl = dropdown.current; - const parent = triggerRef?.current ?? dropdownEl.parentNode; - const { left, top } = alignElements( - parent, - dropdownEl, - align ?? "bottom-left", - 0, - false, - openUpwardForShortViewport ?? true, +export const Dropdown = React.forwardRef( + ({ animated = true, visible = false, constrainHeight = false, ...props }, ref) => { + const rootName = cn("dropdown-dm"); + + /**@type {import('react').RefObject} */ + const dropdown = React.useRef(); + const { triggerRef } = React.useContext(DropdownContext) ?? {}; + const isInline = triggerRef === undefined; + + const { children, align, openUpwardForShortViewport } = props; + const [currentVisible, setVisible] = React.useState(visible); + const [offset, setOffset] = React.useState({}); + const [visibility, setVisibility] = React.useState(visible ? "visible" : null); + + const calculatePosition = React.useCallback(() => { + const dropdownEl = dropdown.current; + const parent = triggerRef?.current ?? dropdownEl.parentNode; + const { left, top } = alignElements( + parent, + dropdownEl, + align ?? "bottom-left", + 0, + constrainHeight, + openUpwardForShortViewport ?? true, + ); + + setOffset({ left, top }); + }, [triggerRef, align, openUpwardForShortViewport]); + + const dropdownIndex = React.useMemo(() => { + return lastIndex++; + }, []); + + const performAnimation = React.useCallback( + async (visible = false) => { + if (props.enabled === false && visible === true) return; + + return new Promise((resolve) => { + const menu = dropdown.current; + + if (animated !== false) { + aroundTransition(menu, { + transition: () => { + setVisibility(visible ? "appear" : "disappear"); + }, + beforeTransition: () => { + setVisibility(visible ? "before-appear" : "before-disappear"); + }, + afterTransition: () => { + setVisibility(visible ? "visible" : null); + resolve(); + }, + }); + } else { + setVisibility(visible ? "visible" : null); + resolve(); + } + }); + }, + [animated], ); - setOffset({ left, top }); - }, [triggerRef, align, openUpwardForShortViewport]); - - const dropdownIndex = React.useMemo(() => { - return lastIndex++; - }, []); - - const performAnimation = React.useCallback( - async (visible = false) => { - if (props.enabled === false && visible === true) return; - - return new Promise((resolve) => { - const menu = dropdown.current; - - if (animated !== false) { - aroundTransition(menu, { - transition: () => { - setVisibility(visible ? "appear" : "disappear"); - }, - beforeTransition: () => { - setVisibility(visible ? "before-appear" : "before-disappear"); - }, - afterTransition: () => { - setVisibility(visible ? "visible" : null); - resolve(); - }, - }); - } else { - setVisibility(visible ? "visible" : null); - resolve(); - } - }); - }, - [animated], - ); - - const close = React.useCallback(async () => { - if (currentVisible === false) return; - - props.onToggle?.(false); - await performAnimation(false); - setVisible(false); - }, [currentVisible, performAnimation, props]); - - const open = React.useCallback(async () => { - if (currentVisible === true) return; - - props.onToggle?.(true); - await performAnimation(true); - setVisible(true); - }, [currentVisible, performAnimation, props]); - - const toggle = React.useCallback(async () => { - const newState = !currentVisible; - - if (newState) { - open(); - } else { - close(); - } - }, [close, currentVisible, open]); - - React.useEffect(() => { - if (!ref) return; - - ref.current = { - dropdown: dropdown.current, - visible: visibility !== null, - toggle, - open, - close, + const close = React.useCallback(async () => { + if (currentVisible === false) return; + + props.onToggle?.(false); + await performAnimation(false); + setVisible(false); + }, [currentVisible, performAnimation, props]); + + const open = React.useCallback(async () => { + if (currentVisible === true) return; + + props.onToggle?.(true); + await performAnimation(true); + setVisible(true); + }, [currentVisible, performAnimation, props]); + + const toggle = React.useCallback(async () => { + const newState = !currentVisible; + + if (newState) { + open(); + } else { + close(); + } + }, [close, currentVisible, open]); + + React.useEffect(() => { + if (!ref) return; + + ref.current = { + dropdown: dropdown.current, + visible: visibility !== null, + toggle, + open, + close, + }; + }, [close, open, ref, toggle, dropdown, visibility]); + + React.useEffect(() => { + setVisible(visible); + }, [visible]); + + React.useEffect(() => { + if (!isInline && visibility === "before-appear") { + calculatePosition(); + } + }, [visibility, calculatePosition, isInline]); + + React.useEffect(() => { + if (props.enabled === false) performAnimation(false); + }, [props.enabled]); + + const content = + children.props && children.props.type === "Menu" + ? React.cloneElement(children, { + ...children.props, + className: rootName.elem("menu").mix(children.props.className), + }) + : children; + + const visibilityClasses = React.useMemo(() => { + switch (visibility) { + case "before-appear": + return "before-appear"; + case "appear": + return "appear before-appear"; + case "before-disappear": + return "before-disappear"; + case "disappear": + return "disappear before-disappear"; + case "visible": + return "visible"; + default: + return visible ? "visible" : null; + } + }, [visibility, visible]); + + const compositeStyles = { + ...(props.style ?? {}), + ...(offset ?? {}), + zIndex: 1000 + dropdownIndex, }; - }, [close, open, ref, toggle, dropdown, visibility]); - - React.useEffect(() => { - setVisible(visible); - }, [visible]); - - React.useEffect(() => { - if (!isInline && visibility === "before-appear") { - calculatePosition(); - } - }, [visibility, calculatePosition, isInline]); - - React.useEffect(() => { - if (props.enabled === false) performAnimation(false); - }, [props.enabled]); - - const content = - children.props && children.props.type === "Menu" - ? React.cloneElement(children, { - ...children.props, - className: rootName.elem("menu").mix(children.props.className), - }) - : children; - - const visibilityClasses = React.useMemo(() => { - switch (visibility) { - case "before-appear": - return "before-appear"; - case "appear": - return "appear before-appear"; - case "before-disappear": - return "before-disappear"; - case "disappear": - return "disappear before-disappear"; - case "visible": - return "visible"; - default: - return visible ? "visible" : null; - } - }, [visibility, visible]); - - const compositeStyles = { - ...(props.style ?? {}), - ...(offset ?? {}), - zIndex: 1000 + dropdownIndex, - }; - const result = ( -
e.stopPropagation()} - > - {content} -
- ); - - return props.inline === true ? result : ReactDOM.createPortal(result, document.body); -}); + const result = ( +
e.stopPropagation()} + > + {content} +
+ ); + + return props.inline === true ? result : ReactDOM.createPortal(result, document.body); + }, +); Dropdown.displayName = "Dropdown"; From 0d61649ff3a3e8d76aebaeee200389eb9567b631 Mon Sep 17 00:00:00 2001 From: Brandon Martel Date: Tue, 6 May 2025 16:06:12 -0500 Subject: [PATCH 6/8] make the diff easier --- .../Common/Dropdown/DropdownComponent.jsx | 308 +++++++++--------- 1 file changed, 153 insertions(+), 155 deletions(-) diff --git a/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx b/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx index 4b97409313f6..b8df20379fcd 100644 --- a/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx +++ b/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx @@ -10,164 +10,162 @@ import { DropdownTrigger } from "./DropdownTrigger"; let lastIndex = 1; -export const Dropdown = React.forwardRef( - ({ animated = true, visible = false, constrainHeight = false, ...props }, ref) => { - const rootName = cn("dropdown-dm"); - - /**@type {import('react').RefObject} */ - const dropdown = React.useRef(); - const { triggerRef } = React.useContext(DropdownContext) ?? {}; - const isInline = triggerRef === undefined; - - const { children, align, openUpwardForShortViewport } = props; - const [currentVisible, setVisible] = React.useState(visible); - const [offset, setOffset] = React.useState({}); - const [visibility, setVisibility] = React.useState(visible ? "visible" : null); - - const calculatePosition = React.useCallback(() => { - const dropdownEl = dropdown.current; - const parent = triggerRef?.current ?? dropdownEl.parentNode; - const { left, top } = alignElements( - parent, - dropdownEl, - align ?? "bottom-left", - 0, - constrainHeight, - openUpwardForShortViewport ?? true, - ); - - setOffset({ left, top }); - }, [triggerRef, align, openUpwardForShortViewport]); - - const dropdownIndex = React.useMemo(() => { - return lastIndex++; - }, []); - - const performAnimation = React.useCallback( - async (visible = false) => { - if (props.enabled === false && visible === true) return; - - return new Promise((resolve) => { - const menu = dropdown.current; - - if (animated !== false) { - aroundTransition(menu, { - transition: () => { - setVisibility(visible ? "appear" : "disappear"); - }, - beforeTransition: () => { - setVisibility(visible ? "before-appear" : "before-disappear"); - }, - afterTransition: () => { - setVisibility(visible ? "visible" : null); - resolve(); - }, - }); - } else { - setVisibility(visible ? "visible" : null); - resolve(); - } - }); - }, - [animated], +export const Dropdown = React.forwardRef(({ animated = true, visible = false, ...props }, ref) => { + const rootName = cn("dropdown-dm"); + + /**@type {import('react').RefObject} */ + const dropdown = React.useRef(); + const { triggerRef } = React.useContext(DropdownContext) ?? {}; + const isInline = triggerRef === undefined; + + const { children, align, openUpwardForShortViewport, constrainHeight = false } = props; + const [currentVisible, setVisible] = React.useState(visible); + const [offset, setOffset] = React.useState({}); + const [visibility, setVisibility] = React.useState(visible ? "visible" : null); + + const calculatePosition = React.useCallback(() => { + const dropdownEl = dropdown.current; + const parent = triggerRef?.current ?? dropdownEl.parentNode; + const { left, top } = alignElements( + parent, + dropdownEl, + align ?? "bottom-left", + 0, + constrainHeight, + openUpwardForShortViewport ?? true, ); - const close = React.useCallback(async () => { - if (currentVisible === false) return; - - props.onToggle?.(false); - await performAnimation(false); - setVisible(false); - }, [currentVisible, performAnimation, props]); - - const open = React.useCallback(async () => { - if (currentVisible === true) return; - - props.onToggle?.(true); - await performAnimation(true); - setVisible(true); - }, [currentVisible, performAnimation, props]); - - const toggle = React.useCallback(async () => { - const newState = !currentVisible; - - if (newState) { - open(); - } else { - close(); - } - }, [close, currentVisible, open]); - - React.useEffect(() => { - if (!ref) return; - - ref.current = { - dropdown: dropdown.current, - visible: visibility !== null, - toggle, - open, - close, - }; - }, [close, open, ref, toggle, dropdown, visibility]); - - React.useEffect(() => { - setVisible(visible); - }, [visible]); - - React.useEffect(() => { - if (!isInline && visibility === "before-appear") { - calculatePosition(); - } - }, [visibility, calculatePosition, isInline]); - - React.useEffect(() => { - if (props.enabled === false) performAnimation(false); - }, [props.enabled]); - - const content = - children.props && children.props.type === "Menu" - ? React.cloneElement(children, { - ...children.props, - className: rootName.elem("menu").mix(children.props.className), - }) - : children; - - const visibilityClasses = React.useMemo(() => { - switch (visibility) { - case "before-appear": - return "before-appear"; - case "appear": - return "appear before-appear"; - case "before-disappear": - return "before-disappear"; - case "disappear": - return "disappear before-disappear"; - case "visible": - return "visible"; - default: - return visible ? "visible" : null; - } - }, [visibility, visible]); - - const compositeStyles = { - ...(props.style ?? {}), - ...(offset ?? {}), - zIndex: 1000 + dropdownIndex, + setOffset({ left, top }); + }, [triggerRef, align, openUpwardForShortViewport]); + + const dropdownIndex = React.useMemo(() => { + return lastIndex++; + }, []); + + const performAnimation = React.useCallback( + async (visible = false) => { + if (props.enabled === false && visible === true) return; + + return new Promise((resolve) => { + const menu = dropdown.current; + + if (animated !== false) { + aroundTransition(menu, { + transition: () => { + setVisibility(visible ? "appear" : "disappear"); + }, + beforeTransition: () => { + setVisibility(visible ? "before-appear" : "before-disappear"); + }, + afterTransition: () => { + setVisibility(visible ? "visible" : null); + resolve(); + }, + }); + } else { + setVisibility(visible ? "visible" : null); + resolve(); + } + }); + }, + [animated], + ); + + const close = React.useCallback(async () => { + if (currentVisible === false) return; + + props.onToggle?.(false); + await performAnimation(false); + setVisible(false); + }, [currentVisible, performAnimation, props]); + + const open = React.useCallback(async () => { + if (currentVisible === true) return; + + props.onToggle?.(true); + await performAnimation(true); + setVisible(true); + }, [currentVisible, performAnimation, props]); + + const toggle = React.useCallback(async () => { + const newState = !currentVisible; + + if (newState) { + open(); + } else { + close(); + } + }, [close, currentVisible, open]); + + React.useEffect(() => { + if (!ref) return; + + ref.current = { + dropdown: dropdown.current, + visible: visibility !== null, + toggle, + open, + close, }; - const result = ( -
e.stopPropagation()} - > - {content} -
- ); - - return props.inline === true ? result : ReactDOM.createPortal(result, document.body); - }, -); + }, [close, open, ref, toggle, dropdown, visibility]); + + React.useEffect(() => { + setVisible(visible); + }, [visible]); + + React.useEffect(() => { + if (!isInline && visibility === "before-appear") { + calculatePosition(); + } + }, [visibility, calculatePosition, isInline]); + + React.useEffect(() => { + if (props.enabled === false) performAnimation(false); + }, [props.enabled]); + + const content = + children.props && children.props.type === "Menu" + ? React.cloneElement(children, { + ...children.props, + className: rootName.elem("menu").mix(children.props.className), + }) + : children; + + const visibilityClasses = React.useMemo(() => { + switch (visibility) { + case "before-appear": + return "before-appear"; + case "appear": + return "appear before-appear"; + case "before-disappear": + return "before-disappear"; + case "disappear": + return "disappear before-disappear"; + case "visible": + return "visible"; + default: + return visible ? "visible" : null; + } + }, [visibility, visible]); + + const compositeStyles = { + ...(props.style ?? {}), + ...(offset ?? {}), + zIndex: 1000 + dropdownIndex, + }; + const result = ( +
e.stopPropagation()} + > + {content} +
+ ); + + return props.inline === true ? result : ReactDOM.createPortal(result, document.body); +}); Dropdown.displayName = "Dropdown"; From 1c62546969c6002402c455c7347d1d0a459dda47 Mon Sep 17 00:00:00 2001 From: Brandon Martel Date: Tue, 6 May 2025 16:07:13 -0500 Subject: [PATCH 7/8] ensure constrainHeight is a dep --- .../src/components/Common/Dropdown/DropdownComponent.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx b/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx index b8df20379fcd..201818cdf2cd 100644 --- a/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx +++ b/web/libs/datamanager/src/components/Common/Dropdown/DropdownComponent.jsx @@ -36,7 +36,7 @@ export const Dropdown = React.forwardRef(({ animated = true, visible = false, .. ); setOffset({ left, top }); - }, [triggerRef, align, openUpwardForShortViewport]); + }, [triggerRef, align, openUpwardForShortViewport, constrainHeight]); const dropdownIndex = React.useMemo(() => { return lastIndex++; From 0ebd85e102669e024ed9d1773c31ab30b88d16c2 Mon Sep 17 00:00:00 2001 From: vladimirheartex Date: Wed, 7 May 2025 07:31:04 +0000 Subject: [PATCH 8/8] Sync Follow Merge dependencies Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/14877667641