Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions frontend/common/useContainClicks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { RefObject, useEffect } from 'react'

/**
* Keeps clicks inside `ref` from reaching the outside-click watchers, so a
* subtree portalled elsewhere still counts as part of whatever opened it.
*
* Without this, an InlineModal holding a portalled menu closes on the very item
* you picked, because the menu is not a DOM descendant of the modal. Native
* listeners rather than React's onMouseUp, because those sit below document,
* which is where useOutsideClick listens.
*/
const useContainClicks = (
ref: RefObject<HTMLElement | null>,
active = true,
) => {
useEffect(() => {
const node = ref.current
if (!active || !node) return
const stop = (e: Event) => e.stopPropagation()
node.addEventListener('mouseup', stop)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Major · ⚡ Quick win

Add a regression test for the containment contract.

Observed: This hook changes mouseup and touchend propagation, but neither it nor DropdownMenu has automated coverage. Predicted: A later change to the portal, event type, or listener phase could reintroduce the modal-closing bug without CI detecting it. Cover an InlineModal with a portalled DropdownMenu, asserting a menu selection keeps the modal open while a real outside click closes it.

node.addEventListener('touchend', stop)
return () => {
node.removeEventListener('mouseup', stop)
node.removeEventListener('touchend', stop)
}
}, [ref, active])
}

export default useContainClicks
5 changes: 5 additions & 0 deletions frontend/web/components/base/DropdownMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useLayoutEffect, useRef, useState } from 'react'
import Icon, { IconName } from 'components/icons/Icon'
import classNames from 'classnames'
import useOutsideClick from 'common/useOutsideClick'
import useContainClicks from 'common/useContainClicks'
import { createPortal } from 'react-dom'
import { calculateListPosition } from 'common/utils/calculateListPosition'

Expand Down Expand Up @@ -37,6 +38,10 @@ const DropdownMenu: React.FC<DropdownMenuProps> = ({
const dropDownRef = useRef<HTMLDivElement>(null)
useOutsideClick(dropDownRef, () => setIsOpen(false))

// The menu is portalled to the body, so it belongs to its trigger rather
// than to wherever it is drawn.
useContainClicks(dropDownRef, isOpen)

useLayoutEffect(() => {
if (!isOpen || !dropDownRef.current || !btnRef.current) return
const listPosition = calculateListPosition(
Expand Down
Loading