From 53e0d7ec25980eb4985fbc3c742b68f0b96d703d Mon Sep 17 00:00:00 2001 From: Dominik Dosoudil Date: Wed, 13 Nov 2024 09:24:40 +0100 Subject: [PATCH 1/4] refactor(event-stack): replace event-stack with window.addEventListener in Popup --- src/modules/Popup/Popup.js | 43 +++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/src/modules/Popup/Popup.js b/src/modules/Popup/Popup.js index ccd61053d0..b7cb6fc512 100644 --- a/src/modules/Popup/Popup.js +++ b/src/modules/Popup/Popup.js @@ -1,4 +1,3 @@ -import EventStack from '@semantic-ui-react/event-stack' import cx from 'clsx' import _ from 'lodash' import PropTypes from 'prop-types' @@ -14,9 +13,9 @@ import { getUnhandledProps, makeDebugger, SUI, + useIsomorphicLayoutEffect, useKeyOnly, useKeyOrValueAndKey, - useIsomorphicLayoutEffect, useMergedRefs, usePrevious, } from '../../lib' @@ -180,23 +179,38 @@ const Popup = React.forwardRef(function (props, ref) { _.invoke(props, 'onOpen', e, { ...props, open: true }) } - const handleHideOnScroll = (e) => { - debug('handleHideOnScroll()') - - // Do not hide the popup when scroll comes from inside the popup - // https://github.com/Semantic-Org/Semantic-UI-React/issues/4305 - if (_.isElement(e.target) && elementRef.current.contains(e.target)) { + React.useEffect(() => { + if (!hideOnScroll || closed || disabled) { return } + const abortController = new AbortController() - setClosed(true) + window.addEventListener( + 'scroll', + (e) => { + debug('handleHideOnScroll()') - timeoutId.current = setTimeout(() => { - setClosed(false) - }, 50) + // Do not hide the popup when scroll comes from inside the popup + // https://github.com/Semantic-Org/Semantic-UI-React/issues/4305 + if (_.isElement(e.target) && elementRef.current.contains(e.target)) { + return + } - handleClose(e) - } + setClosed(true) + + timeoutId.current = setTimeout(() => { + setClosed(false) + }, 50) + + handleClose(e) + }, + { signal: abortController.signal }, + ) + + return () => { + abortController.abort() + } + }, [closed, disabled, elementRef, handleClose, hideOnScroll]) const handlePortalMount = (e) => { debug('handlePortalMount()') @@ -254,7 +268,6 @@ const Popup = React.forwardRef(function (props, ref) { ) : ( children )} - {hideOnScroll && } ) From 12bcbbda6ede6b6aee34499911e675fe77a1320b Mon Sep 17 00:00:00 2001 From: Dominik Dosoudil Date: Wed, 13 Nov 2024 13:42:43 +0100 Subject: [PATCH 2/4] refactor(popup): Move hideOnScroll feature to Portal component to get rid of hacky timout that causes remounting Portal component. --- src/addons/Portal/Portal.d.ts | 3 ++ src/addons/Portal/Portal.js | 31 +++++++++++++++++++ src/modules/Popup/Popup.js | 58 ++++------------------------------- 3 files changed, 40 insertions(+), 52 deletions(-) diff --git a/src/addons/Portal/Portal.d.ts b/src/addons/Portal/Portal.d.ts index 09d5b75bc4..c45f14a724 100644 --- a/src/addons/Portal/Portal.d.ts +++ b/src/addons/Portal/Portal.d.ts @@ -37,6 +37,9 @@ export interface StrictPortalProps { /** Event pool namespace that is used to handle component events. */ eventPool?: string + /** Hide the Popup when scrolling the window. */ + hideOnScroll?: boolean + /** The node where the portal should mount. */ mountNode?: any diff --git a/src/addons/Portal/Portal.js b/src/addons/Portal/Portal.js index 4ad57da0cd..46388aae01 100644 --- a/src/addons/Portal/Portal.js +++ b/src/addons/Portal/Portal.js @@ -38,6 +38,7 @@ function Portal(props) { openOnTriggerClick = true, openOnTriggerFocus, openOnTriggerMouseEnter, + hideOnScroll = false, } = props const [open, setOpen] = useAutoControlledValue({ @@ -146,6 +147,33 @@ function Portal(props) { // Component Event Handlers // ---------------------------------------- + React.useEffect(() => { + if (!hideOnScroll) { + return + } + const abortController = new AbortController() + + window.addEventListener( + 'scroll', + (e) => { + debug('handleHideOnScroll()') + + // Do not hide the popup when scroll comes from inside the popup + // https://github.com/Semantic-Org/Semantic-UI-React/issues/4305 + if (_.isElement(e.target) && contentRef.current.contains(e.target)) { + return + } + + closePortal(e) + }, + { signal: abortController.signal, passive: true }, + ) + + return () => { + abortController.abort() + } + }, [closePortal, hideOnScroll]) + const handlePortalMouseLeave = (e) => { if (!closeOnPortalMouseLeave) { return @@ -318,6 +346,9 @@ Portal.propTypes = { /** Event pool namespace that is used to handle component events */ eventPool: PropTypes.string, + /** Hide the Popup when scrolling the window. */ + hideOnScroll: PropTypes.bool, + /** The node where the portal should mount. */ mountNode: PropTypes.any, diff --git a/src/modules/Popup/Popup.js b/src/modules/Popup/Popup.js index b7cb6fc512..591748acdd 100644 --- a/src/modules/Popup/Popup.js +++ b/src/modules/Popup/Popup.js @@ -71,11 +71,10 @@ function getPortalProps(props) { * Splits props for Portal & Popup. * * @param {Object} unhandledProps - * @param {Boolean} closed * @param {Boolean} disabled */ -function partitionPortalProps(unhandledProps, closed, disabled) { - if (closed || disabled) { +function partitionPortalProps(unhandledProps, disabled) { + if (disabled) { return {} } @@ -123,7 +122,7 @@ const Popup = React.forwardRef(function (props, ref) { eventsEnabled = true, flowing, header, - hideOnScroll, + hideOnScroll = false, inverted, offset, pinned = false, @@ -138,18 +137,11 @@ const Popup = React.forwardRef(function (props, ref) { wide, } = props - const [closed, setClosed] = React.useState(false) - const unhandledProps = getUnhandledProps(Popup, props) - const { contentRestProps, portalRestProps } = partitionPortalProps( - unhandledProps, - closed, - disabled, - ) + const { contentRestProps, portalRestProps } = partitionPortalProps(unhandledProps, disabled) const elementRef = useMergedRefs(ref) const positionUpdate = React.useRef() - const timeoutId = React.useRef() const triggerRef = React.useRef() const zIndexWasSynced = React.useRef(false) @@ -159,12 +151,6 @@ const Popup = React.forwardRef(function (props, ref) { usePositioningEffect(popperDependencies, positionUpdate) - React.useEffect(() => { - return () => { - clearTimeout(timeoutId.current) - } - }, []) - // ---------------------------------------- // Handlers // ---------------------------------------- @@ -179,39 +165,6 @@ const Popup = React.forwardRef(function (props, ref) { _.invoke(props, 'onOpen', e, { ...props, open: true }) } - React.useEffect(() => { - if (!hideOnScroll || closed || disabled) { - return - } - const abortController = new AbortController() - - window.addEventListener( - 'scroll', - (e) => { - debug('handleHideOnScroll()') - - // Do not hide the popup when scroll comes from inside the popup - // https://github.com/Semantic-Org/Semantic-UI-React/issues/4305 - if (_.isElement(e.target) && elementRef.current.contains(e.target)) { - return - } - - setClosed(true) - - timeoutId.current = setTimeout(() => { - setClosed(false) - }, 50) - - handleClose(e) - }, - { signal: abortController.signal }, - ) - - return () => { - abortController.abort() - } - }, [closed, disabled, elementRef, handleClose, hideOnScroll]) - const handlePortalMount = (e) => { debug('handlePortalMount()') _.invoke(props, 'onMount', e, props) @@ -289,7 +242,7 @@ const Popup = React.forwardRef(function (props, ref) { }) } - if (closed || disabled) { + if (disabled) { return trigger } @@ -348,6 +301,7 @@ const Popup = React.forwardRef(function (props, ref) { onUnmount={handlePortalUnmount} trigger={trigger} triggerRef={triggerRef} + hideOnScroll={hideOnScroll} > Date: Wed, 13 Nov 2024 16:03:55 +0100 Subject: [PATCH 3/4] perf(portal): wrap closePortal in useEventCallback to prevent scroll event resubscribed every render --- src/addons/Portal/Portal.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/addons/Portal/Portal.js b/src/addons/Portal/Portal.js index 46388aae01..324d9706af 100644 --- a/src/addons/Portal/Portal.js +++ b/src/addons/Portal/Portal.js @@ -9,6 +9,7 @@ import { doesNodeContainClick, makeDebugger, useAutoControlledValue, + useEventCallback, } from '../../lib' import useTrigger from './utils/useTrigger' import PortalInner from './PortalInner' @@ -74,12 +75,12 @@ function Portal(props) { return setTimeout(() => openPortal(eventClone), delay || 0) } - const closePortal = (e) => { + const closePortal = useEventCallback((e) => { debug('close()') setOpen(false) _.invoke(props, 'onClose', e, { ...props, open: false }) - } + }) const closePortalWithTimeout = (e, delay) => { debug('closeWithTimeout()', delay) From d912d6a4646f8973645c615e576d71f8a703f885 Mon Sep 17 00:00:00 2001 From: Dominik Dosoudil Date: Wed, 13 Nov 2024 16:05:04 +0100 Subject: [PATCH 4/4] fix(portal): Replace AbortController with removeEventListener to support older browsers --- src/addons/Portal/Portal.js | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/addons/Portal/Portal.js b/src/addons/Portal/Portal.js index 324d9706af..01cbb2808d 100644 --- a/src/addons/Portal/Portal.js +++ b/src/addons/Portal/Portal.js @@ -152,26 +152,23 @@ function Portal(props) { if (!hideOnScroll) { return } - const abortController = new AbortController() - window.addEventListener( - 'scroll', - (e) => { - debug('handleHideOnScroll()') + const handleScroll = (e) => { + debug('handleHideOnScroll()') - // Do not hide the popup when scroll comes from inside the popup - // https://github.com/Semantic-Org/Semantic-UI-React/issues/4305 - if (_.isElement(e.target) && contentRef.current.contains(e.target)) { - return - } + // Do not hide the popup when scroll comes from inside the popup + // https://github.com/Semantic-Org/Semantic-UI-React/issues/4305 + if (_.isElement(e.target) && contentRef.current.contains(e.target)) { + return + } - closePortal(e) - }, - { signal: abortController.signal, passive: true }, - ) + closePortal(e) + } + + window.addEventListener('scroll', handleScroll, { passive: true }) return () => { - abortController.abort() + window.removeEventListener('scroll', handleScroll) } }, [closePortal, hideOnScroll])