Skip to content

Commit

Permalink
chore(Popup): replace event-stack (#4499)
Browse files Browse the repository at this point in the history
* refactor(event-stack): replace event-stack with window.addEventListener in Popup

* refactor(popup): Move hideOnScroll feature to Portal component to get rid of hacky timout that causes remounting Portal component.

* perf(portal): wrap closePortal in useEventCallback to prevent scroll event resubscribed every render

* fix(portal): Replace AbortController with removeEventListener to support older browsers

---------

Co-authored-by: Dominik Dosoudil <dosoudil@insoft.cz>
  • Loading branch information
dominikdosoudil and Dominik Dosoudil authored Nov 13, 2024
1 parent d70adfc commit 80a60c9
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 42 deletions.
3 changes: 3 additions & 0 deletions src/addons/Portal/Portal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
33 changes: 31 additions & 2 deletions src/addons/Portal/Portal.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
doesNodeContainClick,
makeDebugger,
useAutoControlledValue,
useEventCallback,
} from '../../lib'
import useTrigger from './utils/useTrigger'
import PortalInner from './PortalInner'
Expand Down Expand Up @@ -38,6 +39,7 @@ function Portal(props) {
openOnTriggerClick = true,
openOnTriggerFocus,
openOnTriggerMouseEnter,
hideOnScroll = false,
} = props

const [open, setOpen] = useAutoControlledValue({
Expand Down Expand Up @@ -73,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)
Expand Down Expand Up @@ -146,6 +148,30 @@ function Portal(props) {
// Component Event Handlers
// ----------------------------------------

React.useEffect(() => {
if (!hideOnScroll) {
return
}

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
}

closePortal(e)
}

window.addEventListener('scroll', handleScroll, { passive: true })

return () => {
window.removeEventListener('scroll', handleScroll)
}
}, [closePortal, hideOnScroll])

const handlePortalMouseLeave = (e) => {
if (!closeOnPortalMouseLeave) {
return
Expand Down Expand Up @@ -318,6 +344,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,

Expand Down
47 changes: 7 additions & 40 deletions src/modules/Popup/Popup.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import EventStack from '@semantic-ui-react/event-stack'
import cx from 'clsx'
import _ from 'lodash'
import PropTypes from 'prop-types'
Expand All @@ -14,9 +13,9 @@ import {
getUnhandledProps,
makeDebugger,
SUI,
useIsomorphicLayoutEffect,
useKeyOnly,
useKeyOrValueAndKey,
useIsomorphicLayoutEffect,
useMergedRefs,
usePrevious,
} from '../../lib'
Expand Down Expand Up @@ -72,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 {}
}

Expand Down Expand Up @@ -124,7 +122,7 @@ const Popup = React.forwardRef(function (props, ref) {
eventsEnabled = true,
flowing,
header,
hideOnScroll,
hideOnScroll = false,
inverted,
offset,
pinned = false,
Expand All @@ -139,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)

Expand All @@ -160,12 +151,6 @@ const Popup = React.forwardRef(function (props, ref) {

usePositioningEffect(popperDependencies, positionUpdate)

React.useEffect(() => {
return () => {
clearTimeout(timeoutId.current)
}
}, [])

// ----------------------------------------
// Handlers
// ----------------------------------------
Expand All @@ -180,24 +165,6 @@ 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)) {
return
}

setClosed(true)

timeoutId.current = setTimeout(() => {
setClosed(false)
}, 50)

handleClose(e)
}

const handlePortalMount = (e) => {
debug('handlePortalMount()')
_.invoke(props, 'onMount', e, props)
Expand Down Expand Up @@ -254,7 +221,6 @@ const Popup = React.forwardRef(function (props, ref) {
) : (
children
)}
{hideOnScroll && <EventStack on={handleHideOnScroll} name='scroll' target='window' />}
</ElementType>
)

Expand All @@ -276,7 +242,7 @@ const Popup = React.forwardRef(function (props, ref) {
})
}

if (closed || disabled) {
if (disabled) {
return trigger
}

Expand Down Expand Up @@ -335,6 +301,7 @@ const Popup = React.forwardRef(function (props, ref) {
onUnmount={handlePortalUnmount}
trigger={trigger}
triggerRef={triggerRef}
hideOnScroll={hideOnScroll}
>
<Popper
modifiers={modifiers}
Expand Down

0 comments on commit 80a60c9

Please sign in to comment.