Skip to content

Commit

Permalink
fix(menu): remove unnecessary outside click handler (#9528)
Browse files Browse the repository at this point in the history
* fix(menu): remove unnecessary outside click handler

* fix(use-context-menu): remove unnecessary can-be-closed logic

Co-authored-by: Andrea N. Cardona <andreancardona@gmail.com>
  • Loading branch information
janhassel and andreancardona authored Aug 26, 2021
1 parent b5628f4 commit f42860f
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 64 deletions.
3 changes: 0 additions & 3 deletions packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8234,9 +8234,6 @@ Map {
},
},
"propTypes": Object {
"autoclose": Object {
"type": "bool",
},
"children": Object {
"type": "node",
},
Expand Down
28 changes: 0 additions & 28 deletions packages/react/src/components/ContextMenu/useContextMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { useEffect, useState } from 'react';
*/
function useContextMenu(trigger = document) {
const [open, setOpen] = useState(false);
const [canBeClosed, setCanBeClosed] = useState(false);
const [position, setPosition] = useState([0, 0]);

function openContextMenu(e) {
Expand All @@ -16,32 +15,6 @@ function useContextMenu(trigger = document) {

setPosition([x, y]);
setOpen(true);

// Safari emits the click event when preventDefault was called on
// the contextmenu event. This is registered by the ClickListener
// component and would lead to immediate closing when a user is
// triggering the menu with ctrl+click. To prevent this, we only
// allow the menu to be closed after the click event was received.
// Since other browsers don't emit this event, it's also reset with
// a 50ms delay after mouseup event was called.

document.addEventListener(
'mouseup',
() => {
setTimeout(() => {
setCanBeClosed(true);
}, 50);
},
{ once: true }
);

document.addEventListener(
'click',
() => {
setCanBeClosed(true);
},
{ once: true }
);
}

function onClose() {
Expand All @@ -66,7 +39,6 @@ function useContextMenu(trigger = document) {
open,
x: position[0],
y: position[1],
autoclose: canBeClosed,
onClose,
};
}
Expand Down
41 changes: 8 additions & 33 deletions packages/react/src/components/Menu/Menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import classnames from 'classnames';
import PropTypes from 'prop-types';
import { settings } from 'carbon-components';
import { keys, match } from '../../internal/keyboard';
import ClickListener from '../../internal/ClickListener';

import {
capWithinRange,
Expand All @@ -36,7 +35,6 @@ const margin = 16; // distance to keep to body edges, in px
const defaultSize = 'sm';

const Menu = function Menu({
autoclose = true,
children,
id,
level = 1,
Expand All @@ -50,7 +48,6 @@ const Menu = function Menu({
const rootRef = useRef(null);
const [direction, setDirection] = useState(1); // 1 = to right, -1 = to left
const [position, setPosition] = useState([x, y]);
const [canBeClosed, setCanBeClosed] = useState(false);
const isRootMenu = level === 1;
const focusReturn = useRef(null);

Expand Down Expand Up @@ -90,7 +87,7 @@ const Menu = function Menu({

if (!isRootMenu) {
const { width: parentWidth } = getParentMenu(
rootRef?.current?.element
rootRef.current
)?.getBoundingClientRect();

targetBoundaries[2] -= parentWidth;
Expand Down Expand Up @@ -124,7 +121,7 @@ const Menu = function Menu({

function focusNode(node) {
if (node) {
resetFocus(rootRef?.current?.element);
resetFocus(rootRef.current);
focusNodeUtil(node);
}
}
Expand Down Expand Up @@ -188,14 +185,8 @@ const Menu = function Menu({
}
}

function handleClickOutside(event) {
if (!clickedElementHasSubnodes(event) && open && canBeClosed && autoclose) {
close(event.type);
}
}

function getCorrectedPosition(preferredDirection) {
const elementRect = rootRef?.current?.element?.getBoundingClientRect();
const elementRect = rootRef.current?.getBoundingClientRect();
const elementDimensions = [elementRect.width, elementRect.height];
const targetBoundaries = getTargetBoundaries();
const containerBoundaries = getContainerBoundaries();
Expand All @@ -216,25 +207,20 @@ const Menu = function Menu({
}

function handleBlur(event) {
if (
isRootMenu &&
!rootRef?.current?.element.contains(event.relatedTarget)
) {
if (isRootMenu && !rootRef.current?.contains(event.relatedTarget)) {
close(event.type);
}
}

useEffect(() => {
setCanBeClosed(false);

if (open) {
focusReturn.current = document.activeElement;
let localDirection = 1;

if (isRootMenu) {
rootRef?.current?.element?.focus();
rootRef.current?.focus();
} else {
const parentMenu = getParentMenu(rootRef?.current?.element);
const parentMenu = getParentMenu(rootRef.current);

if (parentMenu) {
localDirection = Number(parentMenu.dataset.direction);
Expand All @@ -243,8 +229,6 @@ const Menu = function Menu({

const correctedPosition = getCorrectedPosition(localDirection);
setPosition(correctedPosition);

setCanBeClosed(true);
} else {
setPosition([0, 0]);
}
Expand Down Expand Up @@ -278,6 +262,7 @@ const Menu = function Menu({

const ulAttributes = {
id,
ref: rootRef,
className: classes,
onKeyDown: handleKeyDown,
onClick: handleClick,
Expand Down Expand Up @@ -318,24 +303,14 @@ const Menu = function Menu({
childrenToRender = React.Children.toArray(options[0].props.children);
}

const menu = (
<ClickListener onClickOutside={handleClickOutside} ref={rootRef}>
<ul {...ulAttributes}>{childrenToRender}</ul>
</ClickListener>
);
const menu = <ul {...ulAttributes}>{childrenToRender}</ul>;

return isRootMenu
? (open && ReactDOM.createPortal(menu, document.body)) || null
: menu;
};

Menu.propTypes = {
/**
* Whether or not the menu should automatically close when
* an outside click is registered
*/
autoclose: PropTypes.bool,

/**
* Specify the children of the Menu
*/
Expand Down

0 comments on commit f42860f

Please sign in to comment.