Skip to content

Commit

Permalink
CNV-54208: The dropdown menu of "Actions" is not closed by clicking s…
Browse files Browse the repository at this point in the history
…omewhere else

Signed-off-by: Aviv Turgeman <[email protected]>
  • Loading branch information
avivtur committed Dec 31, 2024
1 parent 8950fbf commit 101f9a0
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 29 deletions.
27 changes: 14 additions & 13 deletions src/utils/components/ActionsDropdown/ActionsDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { FC, memo, ReactNode, useRef, useState } from 'react';

import DropdownToggle from '@kubevirt-utils/components/toggles/DropdownToggle';
import KebabToggle from '@kubevirt-utils/components/toggles/KebabToggle';
import { useClickOutside } from '@kubevirt-utils/hooks/useClickOutside/useClickOutside';
import { useKubevirtTranslation } from '@kubevirt-utils/hooks/useKubevirtTranslation';
import { Menu, MenuContent, MenuList, Popper, Tooltip } from '@patternfly/react-core';

Expand Down Expand Up @@ -34,6 +35,8 @@ const ActionsDropdown: FC<ActionsDropdownProps> = ({
const toggleRef = useRef<HTMLButtonElement>(null);
const containerRef = useRef<HTMLDivElement>(null);

useClickOutside([menuRef, toggleRef], () => setIsOpen(false));

const onToggle = () => {
setIsOpen((prevIsOpen) => {
if (onLazyClick && !prevIsOpen) onLazyClick();
Expand All @@ -52,18 +55,6 @@ const ActionsDropdown: FC<ActionsDropdownProps> = ({
variant,
});

const menu = (
<Menu containsFlyout ref={menuRef}>
<MenuContent>
<MenuList>
{actions?.map((action) => (
<ActionDropdownItem action={action} key={action?.id} setIsOpen={setIsOpen} />
))}
</MenuList>
</MenuContent>
</Menu>
);

if (isDisabled)
return (
<div className="kv-actions-dropdown" ref={containerRef}>
Expand All @@ -77,10 +68,20 @@ const ActionsDropdown: FC<ActionsDropdownProps> = ({
<div className="kv-actions-dropdown" ref={containerRef}>
{Toggle(toggleRef)}
<Popper
popper={
<Menu containsFlyout ref={menuRef}>
<MenuContent>
<MenuList>
{actions?.map((action) => (
<ActionDropdownItem action={action} key={action?.id} setIsOpen={setIsOpen} />
))}
</MenuList>
</MenuContent>
</Menu>
}
appendTo={containerRef.current}
isVisible={isOpen}
placement="bottom-end"
popper={menu}
triggerRef={toggleRef}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ const ComposableDrilldownSelect: FC<ComposableDrilldownMenuProps> = ({
const [menuDrilledIn, setMenuDrilledIn] = useState<string[]>([]);
const [drilldownPath, setDrilldownPath] = useState<string[]>([]);
const [menuHeights, setMenuHeights] = useState<MenuHeightsType>({});
const ref = useRef<HTMLDivElement>(null);
const menuRef = useRef<HTMLDivElement>(null);
const toggleRef = useRef<HTMLDivElement>(null);

const onToggleClick = (ev?: React.MouseEvent) => {
ev?.stopPropagation(); // Stop handleClickOutside from handling
Expand Down Expand Up @@ -73,7 +74,7 @@ const ComposableDrilldownSelect: FC<ComposableDrilldownMenuProps> = ({
}
};

useClickOutside(ref, onToggleClick);
useClickOutside([menuRef], onToggleClick);

return (
<Popper
Expand All @@ -88,15 +89,15 @@ const ComposableDrilldownSelect: FC<ComposableDrilldownMenuProps> = ({
onDrillIn={drillIn}
onDrillOut={drillOut}
onGetMenuHeight={setHeight}
ref={ref}
ref={menuRef}
>
<MenuContent menuHeight={`${menuHeights[activeMenu]}px`}>
<MenuList>{children}</MenuList>
</MenuContent>
</Menu>
}
trigger={
<MenuToggle isExpanded={isOpen} isFullWidth onClick={onToggleClick}>
<MenuToggle isExpanded={isOpen} isFullWidth onClick={onToggleClick} ref={toggleRef}>
{toggleLabel}
</MenuToggle>
}
Expand Down
22 changes: 11 additions & 11 deletions src/utils/hooks/useClickOutside/useClickOutside.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
import { RefObject, useEffect } from 'react';
import { MutableRefObject, useEffect } from 'react';

import { CLICK, ESCAPE, KEYDOWN, TAB } from './constants';

export const useClickOutside = <T extends HTMLElement>(
ref: RefObject<T>,
export const useClickOutside = (
refs: MutableRefObject<HTMLElement | null>[],
onClickOutside: () => void,
) => {
useEffect(() => {
const handleClickOutside = (event: MouseEvent) => {
if (ref?.current && !ref?.current.contains(event.target as Node)) {
if (refs?.every((ref) => ref?.current && !ref?.current.contains(event.target as Node))) {
onClickOutside();
}
};

const handleMenuKeys = (event) => {
if (ref?.current) {
if (event?.key === ESCAPE) {
onClickOutside();
}
if (!ref?.current?.contains(event?.target) && event?.key === TAB) {
const handleMenuKeys = (event: KeyboardEvent) => {
if (event?.key === ESCAPE) {
onClickOutside();
} else if (event.key === TAB) {
// Check if the focus is outside all provided refs
if (refs.every((ref) => ref.current && !ref.current.contains(event.target as Node))) {
onClickOutside();
}
}
Expand All @@ -31,5 +31,5 @@ export const useClickOutside = <T extends HTMLElement>(
window?.removeEventListener(KEYDOWN, handleMenuKeys);
document.removeEventListener(CLICK, handleClickOutside);
};
}, [ref, onClickOutside]);
}, [refs, onClickOutside]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const useInstanceTypeCardMenuSection = (): UseInstanceTypeCardMenuSectionValues
});
};

useClickOutside(menuRef, onMenuToggle);
useClickOutside([menuRef], onMenuToggle);
return { activeMenu, menuRef, onMenuSelect, onMenuToggle };
};

Expand Down

0 comments on commit 101f9a0

Please sign in to comment.