Skip to content

Commit

Permalink
fix(menu): render through portal to avoid z-index and overflow issues (
Browse files Browse the repository at this point in the history
…#8829)

* fix(menu): render through portal to avoid z-index and overflow issues

* fix(overflowmenu): create aria relationship between trigger and menu

* fix(overflow-menu): prevent immediate reopening when clicking on trigger

* fix(overflow-menu): return focus to trigger when menu is closed

* docs(overflow-menu): remove test-only story

Co-authored-by: Josh Black <[email protected]>
  • Loading branch information
janhassel and joshblack authored Jul 14, 2021
1 parent c26ff0e commit b3cef52
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,11 @@
}
}

// styles for "next" version
.#{$prefix}--overflow-menu__container {
display: inline-block;
}

// Windows HCM fix
/* stylelint-disable */
.#{$prefix}--overflow-menu:focus,
Expand Down
3 changes: 3 additions & 0 deletions packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7718,6 +7718,9 @@ Map {
"children": Object {
"type": "node",
},
"id": Object {
"type": "string",
},
"level": Object {
"type": "number",
},
Expand Down
19 changes: 8 additions & 11 deletions packages/react/src/components/Menu/Menu-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,25 @@ import Menu, {
} from '../Menu';
import { mount } from 'enzyme';
import { settings } from 'carbon-components';
import { describe, expect } from 'window-or-global';

const { prefix } = settings;

describe('Menu', () => {
describe('renders as expected', () => {
describe('menu', () => {
it('receives the expected classes when closed', () => {
it("isn't rendered when closed", () => {
const wrapper = mount(<Menu />);
const container = wrapper.childAt(0).childAt(0);

expect(container.hasClass(`${prefix}--menu`)).toBe(true);
expect(container.hasClass(`${prefix}--menu--open`)).toBe(false);
expect(wrapper.getDOMNode()).toBe(null);
});

it('receives the expected classes when opened', () => {
const wrapper = mount(<Menu open />);
const container = wrapper.getDOMNode();

const container = wrapper.childAt(0).childAt(0);

expect(container.hasClass(`${prefix}--menu`)).toBe(true);
expect(container.hasClass(`${prefix}--menu--open`)).toBe(true);
expect(container.classList.contains(`${prefix}--menu`)).toBe(true);
expect(container.classList.contains(`${prefix}--menu--open`)).toBe(
true
);
});
});

Expand Down Expand Up @@ -90,7 +87,7 @@ describe('Menu', () => {

it('renders props.children as submenu', () => {
const wrapper = mount(
<Menu>
<Menu open>
<MenuItem label="Format">
<MenuItem label="Bold" />
<MenuItem label="Italic" />
Expand Down
67 changes: 57 additions & 10 deletions packages/react/src/components/Menu/Menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import React, { useEffect, useRef, useState } from 'react';
import ReactDOM from 'react-dom';
import classnames from 'classnames';
import PropTypes from 'prop-types';
import { settings } from 'carbon-components';
Expand Down Expand Up @@ -36,8 +37,9 @@ const margin = 16; // distance to keep to body edges, in px
const Menu = function Menu({
autoclose = true,
children,
open,
id,
level = 1,
open,
x = 0,
y = 0,
onClose = () => {},
Expand All @@ -48,6 +50,25 @@ const Menu = function Menu({
const [position, setPosition] = useState([x, y]);
const [canBeClosed, setCanBeClosed] = useState(false);
const isRootMenu = level === 1;
const focusReturn = useRef(null);

function returnFocus() {
if (focusReturn.current) {
focusReturn.current.focus();
}
}

function close(eventType) {
const isKeyboardEvent = /^key/.test(eventType);

if (isKeyboardEvent) {
window.addEventListener('keyup', returnFocus, { once: true });
} else {
window.addEventListener('mouseup', returnFocus, { once: true });
}

onClose();
}

function getContainerBoundaries() {
const { clientWidth: bodyWidth, clientHeight: bodyHeight } = document.body;
Expand Down Expand Up @@ -107,6 +128,11 @@ const Menu = function Menu({
}

function handleKeyDown(event) {
if (match(event, keys.Tab)) {
event.preventDefault();
close(event.type);
}

if (
event.target.tagName === 'LI' &&
(match(event, keys.Enter) || match(event, keys.Space))
Expand All @@ -120,7 +146,7 @@ const Menu = function Menu({
match(event, keys.Escape) ||
(!isRootMenu && match(event, keys.ArrowLeft))
) {
onClose();
close(event.type);
}

let nodeToFocus;
Expand Down Expand Up @@ -152,17 +178,17 @@ const Menu = function Menu({
}
}

function handleClick(e) {
if (!clickedElementHasSubnodes(e) && e.target.tagName !== 'UL') {
onClose();
function handleClick(event) {
if (!clickedElementHasSubnodes(event) && event.target.tagName !== 'UL') {
close(event.type);
} else {
e.stopPropagation();
event.stopPropagation();
}
}

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

Expand All @@ -187,10 +213,20 @@ const Menu = function Menu({
return correctedPosition;
}

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

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

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

if (isRootMenu) {
Expand Down Expand Up @@ -235,9 +271,11 @@ const Menu = function Menu({
});

const ulAttributes = {
id,
className: classes,
onKeyDown: handleKeyDown,
onClick: handleClick,
onBlur: handleBlur,
role: 'menu',
tabIndex: -1,
'data-direction': direction,
Expand Down Expand Up @@ -274,11 +312,15 @@ const Menu = function Menu({
childrenToRender = React.Children.toArray(options[0].props.children);
}

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

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

Menu.propTypes = {
Expand All @@ -293,6 +335,11 @@ Menu.propTypes = {
*/
children: PropTypes.node,

/**
* Define an ID for this menu
*/
id: PropTypes.string,

/**
* Internal: keeps track of the nesting level of the menu
*/
Expand Down
24 changes: 21 additions & 3 deletions packages/react/src/components/OverflowMenu/next/OverflowMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import PropTypes from 'prop-types';
import classNames from 'classnames';
import { settings } from 'carbon-components';
import { OverflowMenuVertical16 } from '@carbon/icons-react';
import { useId } from '../../../internal/useId';
import Menu from '../../Menu';

const { prefix } = settings;
Expand All @@ -19,6 +20,7 @@ function OverflowMenu({
renderIcon: IconElement = OverflowMenuVertical16,
...rest
}) {
const id = useId('overflowmenu');
const [open, setOpen] = useState(false);
const [position, setPosition] = useState([
[0, 0],
Expand Down Expand Up @@ -55,26 +57,42 @@ function OverflowMenu({
}
}

function handleMousedown(e) {
// prevent default for mousedown on trigger element to avoid
// the "blur" event from firing on the menu as this would close
// it and immediately re-open since "click" event is fired after
// "blur" event.
e.preventDefault();
}

const containerClasses = classNames(`${prefix}--overflow-menu__container`);

const triggerClasses = classNames(`${prefix}--overflow-menu`, {
[`${prefix}--overflow-menu--open`]: open,
});

return (
<>
<div className={containerClasses} aria-owns={id}>
<button
{...rest}
type="button"
aria-haspopup
aria-expanded={open}
className={triggerClasses}
onClick={handleClick}
onMouseDown={handleMousedown}
ref={triggerRef}>
<IconElement />
</button>
<Menu open={open} onClose={closeMenu} x={position[0]} y={position[1]}>
<Menu
id={id}
open={open}
onClose={closeMenu}
x={position[0]}
y={position[1]}>
{children}
</Menu>
</>
</div>
);
}

Expand Down

0 comments on commit b3cef52

Please sign in to comment.