fix: click on trigger component should close the popper if currently …

…open refs: CDS-56
zextras · Aug 11, 2022 · 7fc387e · 7fc387e
1 parent 58d7eac
commit 7fc387e
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 13 deletions.
diff --git a/src/components/display/Dropdown.tsx b/src/components/display/Dropdown.tsx
@@ -325,6 +325,8 @@ interface DropdownProps extends Omit<HTMLAttributes<HTMLDivElement>, 'contextMen
 	onClose?: () => void;
 	/** Only one component can be passed as children */
 	children: React.ReactElement;
+	/** trigger ref that can be used instead of lost children ref caused by cloneElement */
+	triggerRef?: React.Ref<HTMLElement>;
 	/** Placement of the dropdown */
 	placement?:
 		| 'auto'
@@ -378,6 +380,7 @@ const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(function Dropdo
 		onOpen,
 		onClose,
 		children,
+		triggerRef = createRef<HTMLElement>(),
 		disablePortal = false,
 		preventDefault = true,
 		selectedBackgroundColor,
@@ -393,7 +396,7 @@ const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(function Dropdo
 	const [open, setOpen] = useState<boolean>(forceOpen);
 	const openRef = useRef<boolean>(open);
 	const dropdownRef = useCombinedRefs<HTMLDivElement>(dropdownListRef);
-	const triggerRef = useRef<HTMLElement | null>(null);
+	const innerTriggerRef = useCombinedRefs(triggerRef);
 	const popperItemsRef = useRef<HTMLDivElement | null>(null);
 	const startSentinelRef = useRef<HTMLDivElement | null>(null);
 	const endSentinelRef = useRef<HTMLDivElement | null>(null);
@@ -413,20 +416,23 @@ const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(function Dropdo
 		(e?: React.SyntheticEvent | KeyboardEvent) => {
 			e && e.stopPropagation();
 			setOpen(forceOpen);
-			!disableRestoreFocus && triggerRef.current && triggerRef.current.focus();
+			!disableRestoreFocus && innerTriggerRef.current && innerTriggerRef.current.focus();
 			onClose && onClose();
 		},
-		[disableRestoreFocus, forceOpen, onClose]
+		[disableRestoreFocus, forceOpen, innerTriggerRef, onClose]
 	);
 
 	const handleClick = useCallback<(e: React.SyntheticEvent | KeyboardEvent) => void>(
 		(e) => {
-			if (!disabled && !openRef.current) {
+			if (openRef.current) {
+				e.preventDefault();
+				closePopper();
+			} else if (!disabled) {
 				e.preventDefault();
 				openPopper();
 			}
 		},
-		[disabled, openPopper]
+		[closePopper, disabled, openPopper]
 	);
 
 	// TODO: it probably makes sense to merge this callback and the handleClick
@@ -470,6 +476,9 @@ const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(function Dropdo
 				dropdownRef.current &&
 				e.target !== dropdownRef.current &&
 				!dropdownRef.current.contains(e.target as Node | null) &&
+				innerTriggerRef.current &&
+				e.target !== innerTriggerRef.current &&
+				!innerTriggerRef.current?.contains(e.target as Node | null) &&
 				// check if the attribute is in the event path
 				!find(
 					e.composedPath?.() ?? [],
@@ -479,7 +488,7 @@ const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(function Dropdo
 				closePopper();
 			}
 		},
-		[closePopper, dropdownRef]
+		[closePopper, dropdownRef, innerTriggerRef]
 	);
 
 	const onStartSentinelFocus = useCallback(() => {
@@ -499,7 +508,7 @@ const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(function Dropdo
 		() => (handleTriggerEvents ? getKeyboardPreset('button', handleClick) : []),
 		[handleClick, handleTriggerEvents]
 	);
-	useKeyboard(triggerRef, triggerEvents);
+	useKeyboard(innerTriggerRef, triggerEvents);
 
 	// We need to add 'open' as dependency because we want to reattach these events each time we open the dropdown
 	const listEvents = useMemo(
@@ -533,7 +542,7 @@ const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(function Dropdo
 				strategy: 'fixed'
 			};
 
-			const popperReference = contextMenu ? position : triggerRef.current;
+			const popperReference = contextMenu ? position : innerTriggerRef.current;
 			if (popperReference && dropdownRef.current) {
 				popperInstance = createPopper<StrictModifiers>(
 					popperReference,
@@ -543,7 +552,7 @@ const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(function Dropdo
 			}
 		}
 		return (): void => popperInstance && popperInstance.destroy();
-	}, [open, placement, contextMenu, position, dropdownRef]);
+	}, [open, placement, contextMenu, position, dropdownRef, innerTriggerRef]);
 
 	useEffect(() => {
 		if (!disableAutoFocus) {
@@ -707,8 +716,8 @@ const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(function Dropdo
 
 	const triggerComponent = useMemo(() => {
 		const props = contextMenu ? { onContextMenu: handleRightClick } : { onClick: handleLeftClick };
-		return React.cloneElement(children, { ref: triggerRef, ...props });
-	}, [children, contextMenu, handleLeftClick, handleRightClick]);
+		return React.cloneElement(children, { ref: innerTriggerRef, ...props });
+	}, [children, innerTriggerRef, contextMenu, handleLeftClick, handleRightClick]);
 
 	const popperListProps = useMemo(
 		() =>
@@ -728,7 +737,7 @@ const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(function Dropdo
 					width={width}
 					maxWidth={maxWidth}
 					maxHeight={maxHeight}
-					triggerRef={triggerRef}
+					triggerRef={innerTriggerRef}
 					data-testid="dropdown-popper-list"
 					{...popperListProps}
 				>

diff --git a/src/components/inputs/MultiButton.tsx b/src/components/inputs/MultiButton.tsx
@@ -89,6 +89,7 @@ const MultiButton = React.forwardRef<HTMLButtonElement, MultiButtonProps>(functi
 			disableRestoreFocus
 			{...dropdownProps}
 			$containerWidth={(width === 'fill' && '100%') || 'auto'}
+			triggerRef={ref}
 		>
 			<Button
 				backgroundColor={background}
@@ -98,7 +99,6 @@ const MultiButton = React.forwardRef<HTMLButtonElement, MultiButtonProps>(functi
 				disabled={disabledPrimary}
 				icon={primaryIcon}
 				secondaryAction={secondaryAction}
-				ref={ref}
 				width={width}
 				{...rest}
 			/>