Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ContextSubMenuItem): fix popper container #271

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
33 changes: 19 additions & 14 deletions packages/module/src/components/contextmenu/ContextSubMenuItem.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { Dropdown, DropdownItem } from '@patternfly/react-core';
import { Menu, DropdownItem } from '@patternfly/react-core';
import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon';
import { css } from '@patternfly/react-styles';
import topologyStyles from '../../css/topology-components';
Expand All @@ -11,6 +11,11 @@ interface ContextSubMenuItemProps {
children: React.ReactNode[];
}

/** Check if an event target is also a node. Needed to stop runtime errors where the target does not implement Node. */
const isNode = (node: any): boolean => {
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename this to isHTMLNode or something not to be confused with a Topology Node?

return node && typeof node === 'object' && node.nodeType && node.nodeName;
};

const ContextSubMenuItem: React.FunctionComponent<ContextSubMenuItemProps> = ({ label, children, ...other }) => {
const nodeRef = React.useRef<HTMLButtonElement>(null);
const subMenuRef = React.useRef<HTMLDivElement>(null);
Expand All @@ -29,7 +34,10 @@ const ContextSubMenuItem: React.FunctionComponent<ContextSubMenuItemProps> = ({
onMouseEnter={() => setOpen(true)}
onMouseLeave={(e) => {
// if the mouse leaves this item, close the sub menu only if the mouse did not enter the sub menu itself
if (!subMenuRef.current || !subMenuRef.current.contains(e.relatedTarget as Node)) {
if (
!subMenuRef.current ||
(isNode(e.relatedTarget) && !subMenuRef.current.contains(e.relatedTarget as Node))
) {
setOpen(false);
}
}}
Expand All @@ -51,22 +59,18 @@ const ContextSubMenuItem: React.FunctionComponent<ContextSubMenuItemProps> = ({
closeOnOutsideClick
onRequestClose={(e) => {
// only close the sub menu if clicking anywhere outside the menu item that owns the sub menu
if (!e || !nodeRef.current || !nodeRef.current.contains(e.target as Node)) {
if (!e || !nodeRef.current || (isNode(e.target) && !nodeRef.current.contains(e.target as Node))) {
setOpen(false);
}
}}
reference={referenceCb}
// use the parent node to capture the li
container={nodeRef.current ? nodeRef.current.parentElement : nodeRef.current}
container={nodeRef.current?.closest('.pf-v6-c-menu__content')}
returnFocus
>
<div
ref={subMenuRef}
role="presentation"
className="pf-v6-c-dropdown pf-m-expanded"
<Menu
onMouseLeave={(e) => {
// only close the sub menu if the mouse does not enter the item
if (!nodeRef.current || !nodeRef.current.contains(e.relatedTarget as Node)) {
if (!nodeRef.current || (isNode(e.relatedTarget) && !nodeRef.current.contains(e.relatedTarget as Node))) {
setOpen(false);
}
}}
Expand All @@ -77,11 +81,12 @@ const ContextSubMenuItem: React.FunctionComponent<ContextSubMenuItemProps> = ({
e.stopPropagation();
}
}}
ref={subMenuRef}
role="presentation"
isNavFlyout
>
<Dropdown toggle={() => <></>} className={css(topologyStyles.topologyContextMenuCDropdownMenu)}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this class still needed?

Copy link
Author

Choose a reason for hiding this comment

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

It's also used here:

className={css(topologyStyles.topologyContextMenuCDropdownMenu)}

{children}
</Dropdown>
</div>
{children}
</Menu>
</Popper>
</>
);
Expand Down
Loading