From c003bdea205d4cb721e9767aa921621e27d99f96 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 27 Nov 2020 13:57:11 +0100 Subject: [PATCH 01/54] chore(ContextMenu): scaffold new component --- .../ContextMenu/ContextMenu-story.js | 21 +++++++++++++++++++ .../src/components/ContextMenu/ContextMenu.js | 14 +++++++++++++ .../react/src/components/ContextMenu/index.js | 8 +++++++ packages/react/src/index.js | 1 + 4 files changed, 44 insertions(+) create mode 100644 packages/react/src/components/ContextMenu/ContextMenu-story.js create mode 100644 packages/react/src/components/ContextMenu/ContextMenu.js create mode 100644 packages/react/src/components/ContextMenu/index.js diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js new file mode 100644 index 000000000000..0631e92e760a --- /dev/null +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -0,0 +1,21 @@ +/** + * Copyright IBM Corp. 2020 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +import React from 'react'; + +import ContextMenu from '../ContextMenu'; + +export default { + title: 'ContextMenu', + parameters: { + ContextMenu, + }, +}; + +export const _ContextMenu = () => ; + +_ContextMenu.storyName = 'ContextMenu'; diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js new file mode 100644 index 000000000000..d2871cd9e568 --- /dev/null +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -0,0 +1,14 @@ +/** + * Copyright IBM Corp. 2020 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +import React from 'react'; + +const ContextMenu = React.forwardRef(function ContextMenu() { + return
; +}); + +export default ContextMenu; diff --git a/packages/react/src/components/ContextMenu/index.js b/packages/react/src/components/ContextMenu/index.js new file mode 100644 index 000000000000..3c21a5430eba --- /dev/null +++ b/packages/react/src/components/ContextMenu/index.js @@ -0,0 +1,8 @@ +/** + * Copyright IBM Corp. 2020 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +export default from './ContextMenu'; diff --git a/packages/react/src/index.js b/packages/react/src/index.js index 4ba2c266aa23..135b571de0e5 100644 --- a/packages/react/src/index.js +++ b/packages/react/src/index.js @@ -20,6 +20,7 @@ export ComposedModal, { ModalFooter, } from './components/ComposedModal'; export ContentSwitcher from './components/ContentSwitcher'; +export ContextMenu from './components/ContextMenu'; export Copy from './components/Copy'; export CopyButton from './components/CopyButton'; export DangerButton from './components/DangerButton'; From 21e01a1b49d79059a443a348b400868876c8851f Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 27 Nov 2020 14:52:06 +0100 Subject: [PATCH 02/54] feat(context-menu): build basic structure and styling --- .../context-menu/_context-menu.scss | 39 +++++++++++++++++++ .../components/src/globals/scss/styles.scss | 1 + packages/react/.storybook/styles.scss | 1 + .../ContextMenu/ContextMenu-story.js | 17 ++++++-- .../src/components/ContextMenu/ContextMenu.js | 25 +++++++++++- .../ContextMenu/ContextMenuOption.js | 28 +++++++++++++ .../react/src/components/ContextMenu/index.js | 8 +++- packages/react/src/index.js | 2 +- 8 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 packages/components/src/components/context-menu/_context-menu.scss create mode 100644 packages/react/src/components/ContextMenu/ContextMenuOption.js diff --git a/packages/components/src/components/context-menu/_context-menu.scss b/packages/components/src/components/context-menu/_context-menu.scss new file mode 100644 index 000000000000..f7e17fe3aa21 --- /dev/null +++ b/packages/components/src/components/context-menu/_context-menu.scss @@ -0,0 +1,39 @@ +// +// Copyright IBM Corp. 2020 +// +// This source code is licensed under the Apache-2.0 license found in the +// LICENSE file in the root directory of this source tree. +// + +@import '../../globals/scss/vars'; +@import '../../globals/scss/vendor/@carbon/elements/scss/import-once/import-once'; +@import '../../globals/scss/helper-mixins'; + +/// Context Menu styles +/// @access private +/// @group context-menu +@mixin context-menu { + .#{$prefix}--context-menu { + @include box-shadow; + + width: 13rem; + padding: $spacing-02 0; + background-color: $ui-01; + } + + .#{$prefix}--context-menu-option { + display: flex; + align-items: center; + height: $spacing-07; + padding: 0 $spacing-05; + background-color: $ui-01; + + &:hover { + background-color: $hover-ui; + } + } +} + +@include exports('context-menu') { + @include context-menu; +} diff --git a/packages/components/src/globals/scss/styles.scss b/packages/components/src/globals/scss/styles.scss index bc0c2ddc2eca..a2a03f0dc5b9 100644 --- a/packages/components/src/globals/scss/styles.scss +++ b/packages/components/src/globals/scss/styles.scss @@ -130,6 +130,7 @@ $deprecations--message: 'Deprecated code was found, this code will be removed be @import '../../components/code-snippet/code-snippet'; @import '../../components/overflow-menu/overflow-menu'; @import '../../components/content-switcher/content-switcher'; +@import '../../components/context-menu/context-menu'; @import '../../components/date-picker/date-picker'; @import '../../components/dropdown/dropdown'; @import '../../components/loading/loading'; diff --git a/packages/react/.storybook/styles.scss b/packages/react/.storybook/styles.scss index a671ae619860..bd62c9d6113c 100644 --- a/packages/react/.storybook/styles.scss +++ b/packages/react/.storybook/styles.scss @@ -50,6 +50,7 @@ $prefix: 'bx'; @import '~carbon-components/src/components/code-snippet/code-snippet'; @import '~carbon-components/src/components/overflow-menu/overflow-menu'; @import '~carbon-components/src/components/content-switcher/content-switcher'; +@import '~carbon-components/src/components/context-menu/context-menu'; @import '~carbon-components/src/components/date-picker/date-picker'; @import '~carbon-components/src/components/dropdown/dropdown'; @import '~carbon-components/src/components/loading/loading'; diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 0631e92e760a..a7900a39f84a 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -7,15 +7,26 @@ import React from 'react'; -import ContextMenu from '../ContextMenu'; +import ContextMenu, { ContextMenuOption } from '../ContextMenu'; export default { title: 'ContextMenu', parameters: { - ContextMenu, + component: ContextMenu, }, }; -export const _ContextMenu = () => ; +export const _ContextMenu = () => ( + + + + + + + + + + +); _ContextMenu.storyName = 'ContextMenu'; diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index d2871cd9e568..162fbee93cbe 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -6,9 +6,30 @@ */ import React from 'react'; +import PropTypes from 'prop-types'; +import { settings } from 'carbon-components'; -const ContextMenu = React.forwardRef(function ContextMenu() { - return
; +const { prefix } = settings; + +const ContextMenu = React.forwardRef(function ContextMenu({ children }, ref) { + const options = React.Children.map(children, (node) => { + if (React.isValidElement(node)) { + return React.cloneElement(node); + } + }); + + return ( +
    + {options} +
+ ); }); +ContextMenu.propTypes = { + /** + * Specify the children of the ContextMenu + */ + children: PropTypes.node, +}; + export default ContextMenu; diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js new file mode 100644 index 000000000000..f4c44fae9f1c --- /dev/null +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -0,0 +1,28 @@ +/** + * Copyright IBM Corp. 2020 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +import React from 'react'; +import PropTypes from 'prop-types'; +import classnames from 'classnames'; +import { settings } from 'carbon-components'; + +const { prefix } = settings; + +function ContextMenuOption({ label }) { + const classes = classnames(`${prefix}--context-menu-option`); + + return
  • {label}
    • ; +} + +ContextMenuOption.propTypes = { + /** + * Rendered label for the TreeNode + */ + label: PropTypes.node.isRequired, +}; + +export default ContextMenuOption; diff --git a/packages/react/src/components/ContextMenu/index.js b/packages/react/src/components/ContextMenu/index.js index 3c21a5430eba..586feee34f4d 100644 --- a/packages/react/src/components/ContextMenu/index.js +++ b/packages/react/src/components/ContextMenu/index.js @@ -5,4 +5,10 @@ * LICENSE file in the root directory of this source tree. */ -export default from './ContextMenu'; +import ContextMenu from './ContextMenu'; +import ContextMenuOption from './ContextMenuOption'; + +ContextMenu.ContextMenuOption = ContextMenuOption; + +export { ContextMenuOption }; +export default ContextMenu; diff --git a/packages/react/src/index.js b/packages/react/src/index.js index 135b571de0e5..d2fe6c95399b 100644 --- a/packages/react/src/index.js +++ b/packages/react/src/index.js @@ -20,7 +20,7 @@ export ComposedModal, { ModalFooter, } from './components/ComposedModal'; export ContentSwitcher from './components/ContentSwitcher'; -export ContextMenu from './components/ContextMenu'; +export ContextMenu, { ContextMenuOption } from './components/ContextMenu'; export Copy from './components/Copy'; export CopyButton from './components/CopyButton'; export DangerButton from './components/DangerButton'; From e435d19417a8e4b3a76535b4422361653c010b62 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 27 Nov 2020 15:22:49 +0100 Subject: [PATCH 03/54] feat(context-menu): build basic nesting support --- .../context-menu/_context-menu.scss | 27 ++++++++++++++-- .../ContextMenu/ContextMenu-story.js | 8 +++-- .../src/components/ContextMenu/ContextMenu.js | 17 ++++++++-- .../ContextMenu/ContextMenuOption.js | 31 +++++++++++++++++-- 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/packages/components/src/components/context-menu/_context-menu.scss b/packages/components/src/components/context-menu/_context-menu.scss index f7e17fe3aa21..4552df80b10e 100644 --- a/packages/components/src/components/context-menu/_context-menu.scss +++ b/packages/components/src/components/context-menu/_context-menu.scss @@ -16,22 +16,43 @@ .#{$prefix}--context-menu { @include box-shadow; + display: none; width: 13rem; padding: $spacing-02 0; background-color: $ui-01; } + .#{$prefix}--context-menu--open { + display: block; + } + .#{$prefix}--context-menu-option { - display: flex; - align-items: center; + position: relative; height: $spacing-07; - padding: 0 $spacing-05; background-color: $ui-01; &:hover { background-color: $hover-ui; } } + + .#{$prefix}--context-menu-option > .#{$prefix}--context-menu { + position: absolute; + top: -$spacing-02; + left: 100%; + z-index: 1; + } + + .#{$prefix}--context-menu-option:hover > .#{$prefix}--context-menu { + display: block; + } + + .#{$prefix}--context-menu-option__content { + display: flex; + align-items: center; + height: 100%; + padding: 0 $spacing-05; + } } @include exports('context-menu') { diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index a7900a39f84a..17a495f4e99a 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -17,8 +17,12 @@ export default { }; export const _ContextMenu = () => ( - - + + + + + + diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index 162fbee93cbe..25eedb30a000 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -6,20 +6,28 @@ */ import React from 'react'; +import classnames from 'classnames'; import PropTypes from 'prop-types'; import { settings } from 'carbon-components'; const { prefix } = settings; -const ContextMenu = React.forwardRef(function ContextMenu({ children }, ref) { +const ContextMenu = React.forwardRef(function ContextMenu( + { children, open }, + ref +) { const options = React.Children.map(children, (node) => { if (React.isValidElement(node)) { return React.cloneElement(node); } }); + const classes = classnames(`${prefix}--context-menu`, { + [`${prefix}--context-menu--open`]: open, + }); + return ( -
      +
        {options}
      ); @@ -30,6 +38,11 @@ ContextMenu.propTypes = { * Specify the children of the ContextMenu */ children: PropTypes.node, + + /** + * Specify whether the ContextMenu is currently open + */ + open: PropTypes.bool, }; export default ContextMenu; diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index f4c44fae9f1c..50571192d4ab 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -9,18 +9,43 @@ import React from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import { settings } from 'carbon-components'; +import ContextMenu from './ContextMenu'; const { prefix } = settings; -function ContextMenuOption({ label }) { +function ContextMenuOption({ label, children }) { + const subOptions = React.Children.map(children, (node) => { + if (React.isValidElement(node)) { + return React.cloneElement(node); + } + }); + const classes = classnames(`${prefix}--context-menu-option`); - return
    • {label}
      • ; + return ( +
    • + {subOptions ? ( + <> +
      + {label} +
      + {subOptions} + + ) : ( +
      {label}
      + )} +
      • + ); } ContextMenuOption.propTypes = { /** - * Rendered label for the TreeNode + * Specify the children of the ContextMenuOption + */ + children: PropTypes.node, + + /** + * Rendered label for the ContextMenuOption */ label: PropTypes.node.isRequired, }; From 37a38ce9f7d547a56741607e5671eab3899e7443 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 27 Nov 2020 15:44:23 +0100 Subject: [PATCH 04/54] feat(context-menu): use button markup --- .../context-menu/_context-menu.scss | 24 +++++++++++++++ .../ContextMenu/ContextMenu-story.js | 2 +- .../ContextMenu/ContextMenuOption.js | 30 ++++++++++++++++--- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/packages/components/src/components/context-menu/_context-menu.scss b/packages/components/src/components/context-menu/_context-menu.scss index 4552df80b10e..3c1e39f4215a 100644 --- a/packages/components/src/components/context-menu/_context-menu.scss +++ b/packages/components/src/components/context-menu/_context-menu.scss @@ -8,6 +8,7 @@ @import '../../globals/scss/vars'; @import '../../globals/scss/vendor/@carbon/elements/scss/import-once/import-once'; @import '../../globals/scss/helper-mixins'; +@import '../../globals/scss/css--reset'; /// Context Menu styles /// @access private @@ -30,6 +31,7 @@ position: relative; height: $spacing-07; background-color: $ui-01; + transition: background-color $duration--fast-01 motion(standard, productive); &:hover { background-color: $hover-ui; @@ -48,10 +50,32 @@ } .#{$prefix}--context-menu-option__content { + @include button-reset; + display: flex; align-items: center; + justify-content: space-between; height: 100%; padding: 0 $spacing-05; + + &:focus { + @include focus-outline('outline'); + } + } + + .#{$prefix}--context-menu-option__label { + @include type-style('body-short-01'); + + // add top/bottom padding to make sure letters are not cut off by hidden overflow + padding: $spacing-02 0; + overflow: hidden; + color: $text-01; + white-space: nowrap; + text-overflow: ellipsis; + } + + .#{$prefix}--context-menu-option__info { + margin-left: $spacing-05; } } diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 17a495f4e99a..9cd6b31c8cb0 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -18,7 +18,7 @@ export default { export const _ContextMenu = () => ( - + diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index 50571192d4ab..6f1028acd81c 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -9,10 +9,22 @@ import React from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import { settings } from 'carbon-components'; +import { CaretRight16 } from '@carbon/icons-react'; import ContextMenu from './ContextMenu'; const { prefix } = settings; +function ContextMenuOptionContent({ label, info }) { + return ( + + ); +} + function ContextMenuOption({ label, children }) { const subOptions = React.Children.map(children, (node) => { if (React.isValidElement(node)) { @@ -26,18 +38,28 @@ function ContextMenuOption({ label, children }) {
    • {subOptions ? ( <> -
      - {label} -
      + } /> {subOptions} ) : ( -
      {label}
      + )}
      • ); } +ContextMenuOptionContent.propTypes = { + /** + * Additional information such as shortcut or caret + */ + info: PropTypes.node, + + /** + * Rendered label for the ContextMenuOptionContent + */ + label: PropTypes.node.isRequired, +}; + ContextMenuOption.propTypes = { /** * Specify the children of the ContextMenuOption From cc571c247558b4027a8f5cdda290b5bb2f1c554f Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 27 Nov 2020 15:55:16 +0100 Subject: [PATCH 05/54] feat(context-menu): add support for disabled options --- .../context-menu/_context-menu.scss | 9 ++++++++ .../ContextMenu/ContextMenu-story.js | 2 +- .../ContextMenu/ContextMenuOption.js | 22 +++++++++++++++---- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/packages/components/src/components/context-menu/_context-menu.scss b/packages/components/src/components/context-menu/_context-menu.scss index 3c1e39f4215a..434713c44c70 100644 --- a/packages/components/src/components/context-menu/_context-menu.scss +++ b/packages/components/src/components/context-menu/_context-menu.scss @@ -63,6 +63,15 @@ } } + .#{$prefix}--context-menu-option__content--disabled { + cursor: not-allowed; + } + + .#{$prefix}--context-menu-option__content--disabled + .#{$prefix}--context-menu-option__label { + color: $disabled-02; + } + .#{$prefix}--context-menu-option__label { @include type-style('body-short-01'); diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 9cd6b31c8cb0..b41b213dd0b3 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -26,7 +26,7 @@ export const _ContextMenu = () => ( - + diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index 6f1028acd81c..be74bf0f2108 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -14,9 +14,13 @@ import ContextMenu from './ContextMenu'; const { prefix } = settings; -function ContextMenuOptionContent({ label, info }) { +function ContextMenuOptionContent({ label, info, disabled }) { + const classes = classnames(`${prefix}--context-menu-option__content`, { + [`${prefix}--context-menu-option__content--disabled`]: disabled, + }); + return ( -
    From 26babe87f148a09eaa50574682a04ed0b3516238 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 11 Dec 2020 11:00:09 +0100 Subject: [PATCH 29/54] fix(context-menu): only open submenu on long hover or right arrow --- .../context-menu/_context-menu.scss | 25 +++--- .../src/components/ContextMenu/ContextMenu.js | 84 +++++-------------- .../ContextMenu/ContextMenuOption.js | 64 +++++++++++++- .../src/components/ContextMenu/_utils.js | 67 +++++++++++++++ 4 files changed, 162 insertions(+), 78 deletions(-) create mode 100644 packages/react/src/components/ContextMenu/_utils.js diff --git a/packages/components/src/components/context-menu/_context-menu.scss b/packages/components/src/components/context-menu/_context-menu.scss index 7661a2371f8e..51a606cd42a4 100644 --- a/packages/components/src/components/context-menu/_context-menu.scss +++ b/packages/components/src/components/context-menu/_context-menu.scss @@ -18,15 +18,20 @@ @include box-shadow; z-index: z('modal'); - display: none; + // display: block; width: 13rem; padding: $spacing-02 0; background-color: $ui-01; + visibility: hidden; } - .#{$prefix}--context-menu--open { + .#{$prefix}--context-menu--root { position: fixed; - display: block; + } + + .#{$prefix}--context-menu--open { + // display: block; + visibility: visible; &:focus { @include focus-outline('border'); @@ -38,11 +43,12 @@ height: $spacing-07; background-color: $ui-01; transition: background-color $duration--fast-01 motion(standard, productive); + } - &:hover, - &:focus-within { - background-color: $hover-ui; - } + .#{$prefix}--context-menu-option--active, + .#{$prefix}--context-menu-option:hover, + .#{$prefix}--context-menu-option:focus-within { + background-color: $hover-ui; } .#{$prefix}--context-menu-option > .#{$prefix}--context-menu { @@ -56,11 +62,6 @@ left: -100%; } - .#{$prefix}--context-menu-option:hover > .#{$prefix}--context-menu, - .#{$prefix}--context-menu-option:focus-within > .#{$prefix}--context-menu { - display: block; - } - .#{$prefix}--context-menu-option__content { @include button-reset; diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index cc6b9b4a7085..bfa3f031a95f 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -12,6 +12,15 @@ import { settings } from 'carbon-components'; import { keys, match } from '../../internal/keyboard'; import ClickListener from '../../internal/ClickListener'; +import { + getValidNodes, + resetFocus, + focusNode as focusNodeUtil, + getNextNode, + getFirstSubNode, + getParentNode, +} from './_utils'; + import ContextMenuSelectableOption from './ContextMenuSelectableOption'; import ContextMenuRadioGroup from './ContextMenuRadioGroup'; @@ -30,77 +39,22 @@ const ContextMenu = function ContextMenu({ }) { const rootRef = useRef(null); const [shouldReverse, setShouldReverse] = useState(false); - const isRootMenu = open !== undefined; - - function resetFocus() { - Array.from( - rootRef?.current?.element.querySelectorAll('[tabindex="0"]') ?? [] - ).forEach((node) => { - node.tabIndex = -1; - }); - } + const isRootMenu = level === 1; function focusNode(node) { if (node) { - resetFocus(); - node.tabIndex = 0; - node.focus(); + resetFocus(rootRef?.current?.element); + focusNodeUtil(node); } } - function getValidNodes(list) { - const nodes = Array.from(list?.childNodes ?? []).reduce((acc, child) => { - if (child.tagName === 'LI') { - return [...acc, child]; - } - - if (child.classList.contains(`${prefix}--context-menu-radio-group`)) { - return [...acc, ...child.childNodes]; - } - - return acc; - }, []); - - return nodes.filter((node) => - node.matches( - `li.${prefix}--context-menu-option:not(.${prefix}--context-menu-option--disabled)` - ) - ); - } - - function getNextNode(current, direction) { - const nodes = getValidNodes(current.parentNode); - const currentIndex = nodes.indexOf(current); - - const nextNode = nodes[currentIndex + direction]; - - return nextNode?.firstChild || null; - } - - function getFirstSubNode(node) { - const submenu = node.querySelector(`ul.${prefix}--context-menu`); - - if (submenu) { - const subnodes = getValidNodes(submenu); - - return subnodes[0]?.firstChild || null; - } - - return null; - } - - function getParentNode(node) { - const parentNode = node.parentNode.closest( - `li.${prefix}--context-menu-option` - ); - - return parentNode?.firstChild || null; - } - function handleKeyDown(event) { event.stopPropagation(); - if (match(event, keys.Escape)) { + if ( + match(event, keys.Escape) || + (!isRootMenu && match(event, keys.ArrowLeft)) + ) { onClose(); } @@ -115,6 +69,7 @@ const ContextMenu = function ContextMenu({ nodeToFocus = getNextNode(currentNode, 1); } else if (match(event, keys.ArrowRight)) { nodeToFocus = getFirstSubNode(currentNode); + console.log(nodeToFocus); } else if (match(event, keys.ArrowLeft)) { nodeToFocus = getParentNode(currentNode); } @@ -175,7 +130,9 @@ const ContextMenu = function ContextMenu({ useEffect(() => { if (open) { - rootRef?.current?.element?.focus(); + if (isRootMenu) { + rootRef?.current?.element?.focus(); + } } setShouldReverse(!willFit()); @@ -201,6 +158,7 @@ const ContextMenu = function ContextMenu({ const classes = classnames(`${prefix}--context-menu`, { [`${prefix}--context-menu--open`]: open, [`${prefix}--context-menu--reverse`]: shouldReverse, + [`${prefix}--context-menu--root`]: isRootMenu, }); return ( diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index 64309ee3a34a..21912c28fcf0 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -5,15 +5,21 @@ * LICENSE file in the root directory of this source tree. */ -import React from 'react'; +import React, { useState, useRef, useEffect } from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import { settings } from 'carbon-components'; import { CaretRight16 } from '@carbon/icons-react'; +import { keys, match } from '../../internal/keyboard'; + +import { getFirstSubNode, focusNode } from './_utils'; + import ContextMenu from './ContextMenu'; const { prefix } = settings; +const hoverIntentDelay = 150; // in ms + function ContextMenuOptionContent({ label, info, @@ -58,14 +64,54 @@ function ContextMenuOption({ menuX, ...rest }) { + const [submenuOpen, setSubmenuOpen] = useState(false); + const [submenuOpenedByKeyboard, setSubmenuOpenedByKeyboard] = useState(false); + const rootRef = useRef(null); + const hoverIntentTimeout = useRef(null); + const subOptions = React.Children.map(children, (node) => { if (React.isValidElement(node)) { return React.cloneElement(node); } }); + function openSubmenu(openedByKeyboard = false) { + setSubmenuOpenedByKeyboard(openedByKeyboard); + setSubmenuOpen(true); + } + + function handleKeyDown(event) { + if (match(event, keys.ArrowRight)) { + openSubmenu(true); + } + } + + function handleMouseEnter() { + hoverIntentTimeout.current = setTimeout(openSubmenu, hoverIntentDelay); + } + + function handleMouseLeave(event) { + clearTimeout(hoverIntentTimeout?.current); + + if ( + submenuOpen && + rootRef?.current?.parentNode.contains(event.relatedTarget) + ) { + setSubmenuOpen(false); + } + } + + useEffect(() => { + if (subOptions && submenuOpenedByKeyboard) { + const firstSubnode = getFirstSubNode(rootRef?.current); + focusNode(firstSubnode); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [submenuOpen]); + const classes = classnames(`${prefix}--context-menu-option`, { [`${prefix}--context-menu-option--disabled`]: disabled, + [`${prefix}--context-menu-option--active`]: subOptions && submenuOpen, }); const allowedRoles = ['radio', 'checkbox']; @@ -76,12 +122,18 @@ function ContextMenuOption({ shortcut && shortcutText ? `${label}, ${shortcutText}` : label; return ( + // role is either radio, checkbox, or menuitem which are all interactive + // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
  • + aria-disabled={!subOptions && disabled} + onKeyDown={subOptions ? handleKeyDown : null} + onMouseEnter={subOptions ? handleMouseEnter : null} + onMouseLeave={subOptions ? handleMouseLeave : null}> {subOptions ? ( <> - + { + setSubmenuOpen(false); + }}> {subOptions} diff --git a/packages/react/src/components/ContextMenu/_utils.js b/packages/react/src/components/ContextMenu/_utils.js new file mode 100644 index 000000000000..62bf5c08ffa4 --- /dev/null +++ b/packages/react/src/components/ContextMenu/_utils.js @@ -0,0 +1,67 @@ +import { settings } from 'carbon-components'; + +const { prefix } = settings; + +export function resetFocus(el) { + if (el) { + Array.from(el.querySelectorAll('[tabindex="0"]') ?? []).forEach((node) => { + node.tabIndex = -1; + }); + } +} + +export function focusNode(node) { + if (node) { + node.tabIndex = 0; + node.focus(); + } +} + +export function getValidNodes(list) { + const nodes = Array.from(list?.childNodes ?? []).reduce((acc, child) => { + if (child.tagName === 'LI') { + return [...acc, child]; + } + + if (child.classList.contains(`${prefix}--context-menu-radio-group`)) { + return [...acc, ...child.childNodes]; + } + + return acc; + }, []); + + return nodes.filter((node) => + node.matches( + `li.${prefix}--context-menu-option:not(.${prefix}--context-menu-option--disabled)` + ) + ); +} + +export function getNextNode(current, direction) { + const nodes = getValidNodes(current.parentNode); + const currentIndex = nodes.indexOf(current); + + const nextNode = nodes[currentIndex + direction]; + + return nextNode?.firstChild || null; +} + +export function getFirstSubNode(node) { + const submenu = node.querySelector(`ul.${prefix}--context-menu`); + + if (submenu) { + const subnodes = getValidNodes(submenu); + + return subnodes[0]?.firstChild || null; + } + + return null; +} + +export function getParentNode(node) { + const parentNode = node.parentNode.closest( + `li.${prefix}--context-menu-option` + ); + + return parentNode?.firstChild || null; +} From 1d13ee068fc0580fb2ca4e496a08a6e72f5ae33a Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Mon, 14 Dec 2020 16:12:16 +0100 Subject: [PATCH 30/54] feat(context-menu): always keep menu within body boundaries --- .../context-menu/_context-menu.scss | 26 ++-- .../__snapshots__/PublicAPI-test.js.snap | 6 - .../ContextMenu/ContextMenu-story.js | 4 +- .../src/components/ContextMenu/ContextMenu.js | 113 ++++++++++++------ .../ContextMenu/ContextMenuOption.js | 41 ++++--- .../src/components/ContextMenu/_utils.js | 22 +++- 6 files changed, 132 insertions(+), 80 deletions(-) diff --git a/packages/components/src/components/context-menu/_context-menu.scss b/packages/components/src/components/context-menu/_context-menu.scss index 51a606cd42a4..0c6029584c7b 100644 --- a/packages/components/src/components/context-menu/_context-menu.scss +++ b/packages/components/src/components/context-menu/_context-menu.scss @@ -17,20 +17,16 @@ .#{$prefix}--context-menu { @include box-shadow; + position: fixed; z-index: z('modal'); - // display: block; - width: 13rem; + min-width: 13rem; + max-width: 20rem; padding: $spacing-02 0; background-color: $ui-01; visibility: hidden; } - .#{$prefix}--context-menu--root { - position: fixed; - } - .#{$prefix}--context-menu--open { - // display: block; visibility: visible; &:focus { @@ -38,6 +34,10 @@ } } + .#{$prefix}--context-menu--invisible { + opacity: 0; + } + .#{$prefix}--context-menu-option { position: relative; height: $spacing-07; @@ -52,14 +52,7 @@ } .#{$prefix}--context-menu-option > .#{$prefix}--context-menu { - position: absolute; - top: calc(#{$spacing-02} * -1); - left: 100%; - z-index: 1; - } - - .#{$prefix}--context-menu-option > .#{$prefix}--context-menu--reverse { - left: -100%; + margin-top: calc(#{$spacing-02} * -1); } .#{$prefix}--context-menu-option__content { @@ -114,8 +107,9 @@ .#{$prefix}--context-menu-option__icon { display: flex; - flex: 0 0 1rem; align-items: center; + width: 1rem; + height: 1rem; margin-right: $spacing-03; color: $icon-01; } diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 28676f41cc2e..0eb6b2471319 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -867,9 +867,6 @@ Map { "level": Object { "type": "number", }, - "menuX": Object { - "type": "number", - }, "renderIcon": Object { "args": Array [ Array [ @@ -967,9 +964,6 @@ Map { "level": Object { "type": "number", }, - "menuX": Object { - "type": "number", - }, "renderIcon": Object { "args": Array [ Array [ diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index bef2336afb20..89d85c49f993 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -46,7 +46,7 @@ export const _ContextMenu = () => { }); return ( - <> +
    { onClick={action('onClick')} /> - +
    ); }; diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index bfa3f031a95f..dd411edbacbb 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -19,6 +19,7 @@ import { getNextNode, getFirstSubNode, getParentNode, + getParentMenu, } from './_utils'; import ContextMenuSelectableOption from './ContextMenuSelectableOption'; @@ -26,7 +27,7 @@ import ContextMenuRadioGroup from './ContextMenuRadioGroup'; const { prefix } = settings; -const contextMenuWidth = 208; // in px +const margin = 16; // distance to keep to body edges, in px const ContextMenu = function ContextMenu({ children, @@ -38,7 +39,8 @@ const ContextMenu = function ContextMenu({ ...rest }) { const rootRef = useRef(null); - const [shouldReverse, setShouldReverse] = useState(false); + const [direction, setDirection] = useState(1); // 1 = to right, -1 = to left + const [position, setPosition] = useState([x, y]); const isRootMenu = level === 1; function focusNode(node) { @@ -69,7 +71,6 @@ const ContextMenu = function ContextMenu({ nodeToFocus = getNextNode(currentNode, 1); } else if (match(event, keys.ArrowRight)) { nodeToFocus = getFirstSubNode(currentNode); - console.log(nodeToFocus); } else if (match(event, keys.ArrowLeft)) { nodeToFocus = getParentNode(currentNode); } @@ -98,45 +99,80 @@ const ContextMenu = function ContextMenu({ onClose(); } - function willFit() { - if (rootRef?.current?.element) { - const bodyWidth = document.body.clientWidth; - - const reverseMap = [...Array(level)].reduce( - (acc) => { - const endX = acc.lastX + contextMenuWidth * acc.direction; - const fits = - acc.direction === 1 ? endX < bodyWidth : endX > contextMenuWidth; - - const newDirection = fits ? acc.direction : acc.direction * -1; - const newLastX = fits - ? endX - : acc.lastX + contextMenuWidth * newDirection; - - return { - direction: newDirection, - lastX: newLastX, - map: [...acc.map, newDirection === 1], - }; - }, - { direction: 1, lastX: x, map: [] } - ); - - return reverseMap.map[level - 1]; + function getCorrectedPosition(assumedDirection) { + const pos = [x, y]; + + const { + width, + height, + } = rootRef?.current?.element?.getBoundingClientRect(); + const { clientWidth: bodyWidth, clientHeight: bodyHeight } = document.body; + const parentWidth = isRootMenu + ? 0 + : getParentMenu(rootRef?.current?.element)?.getBoundingClientRect() + ?.width; + let localDirection = assumedDirection; + + const min = [margin, margin]; + const max = [bodyWidth - margin - width, bodyHeight - margin - height]; + + // in case it is root menu previously had direction -1, check + // if direction 1 would be possible + if (isRootMenu && localDirection === -1 && pos[0] < max[0]) { + localDirection = 1; } - return true; + // make sure menu is visible in y bounds + if (pos[1] > max[1]) pos[1] = max[1]; + if (pos[1] < min[1]) pos[1] = min[1]; + + if (localDirection === 1) { + // if it won't fit anymore + if (pos[0] > max[0]) { + pos[0] = x - width - parentWidth; + if (pos[0] + width > bodyWidth - margin) pos[0] = max[0]; + localDirection = -1; + } else if (pos[0] < min[0]) { + // keep distance to left screen edge + pos[0] = min[0]; + } + } else if (localDirection === -1) { + pos[0] = x - width - parentWidth; + + // if it should re-reverse + if (pos[0] < min[0]) { + pos[0] = x; + localDirection = 1; + } + } + + setDirection(localDirection); + + return [Math.round(pos[0]), Math.round(pos[1])]; } useEffect(() => { if (open) { + let localDirection = 1; + if (isRootMenu) { rootRef?.current?.element?.focus(); + } else { + const parentMenu = getParentMenu(rootRef?.current?.element); + + if (parentMenu) { + localDirection = Number(parentMenu.dataset.direction); + } } + + const correctedPosition = getCorrectedPosition(localDirection); + setPosition(correctedPosition); + } else { + setPosition([0, 0]); } - setShouldReverse(!willFit()); - }, [open, x, y]); // eslint-disable-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [open, x, y]); const someNodesHaveIcons = React.Children.toArray(children).some( (node) => @@ -150,27 +186,32 @@ const ContextMenu = function ContextMenu({ return React.cloneElement(node, { indented: someNodesHaveIcons, level: level, - menuX: x, }); } }); const classes = classnames(`${prefix}--context-menu`, { [`${prefix}--context-menu--open`]: open, - [`${prefix}--context-menu--reverse`]: shouldReverse, + [`${prefix}--context-menu--invisible`]: + open && position[0] === 0 && position[1] === 0, [`${prefix}--context-menu--root`]: isRootMenu, }); return (
      + tabIndex={-1} + data-level={level} + data-direction={direction} + style={{ + left: `${position[0]}px`, + top: `${position[1]}px`, + }}> {options}
    diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index 21912c28fcf0..3afd03023201 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -12,7 +12,7 @@ import { settings } from 'carbon-components'; import { CaretRight16 } from '@carbon/icons-react'; import { keys, match } from '../../internal/keyboard'; -import { getFirstSubNode, focusNode } from './_utils'; +import { getFirstSubNode, focusNode, getParentMenu } from './_utils'; import ContextMenu from './ContextMenu'; @@ -61,7 +61,6 @@ function ContextMenuOption({ renderIcon, indented, level, - menuX, ...rest }) { const [submenuOpen, setSubmenuOpen] = useState(false); @@ -90,15 +89,28 @@ function ContextMenuOption({ hoverIntentTimeout.current = setTimeout(openSubmenu, hoverIntentDelay); } - function handleMouseLeave(event) { + function handleMouseLeave() { clearTimeout(hoverIntentTimeout?.current); - if ( - submenuOpen && - rootRef?.current?.parentNode.contains(event.relatedTarget) - ) { - setSubmenuOpen(false); + setSubmenuOpen(false); + } + + function getSubmenuPosition() { + const pos = [0, 0]; + + if (subOptions) { + const parentMenu = getParentMenu(rootRef?.current); + + if (parentMenu) { + const { x, width } = parentMenu.getBoundingClientRect(); + const { y } = rootRef.current.getBoundingClientRect(); + + pos[0] = x + width; + pos[1] = y; + } } + + return pos; } useEffect(() => { @@ -121,6 +133,8 @@ function ContextMenuOption({ const ariaLabel = shortcut && shortcutText ? `${label}, ${shortcutText}` : label; + const submenuPosition = getSubmenuPosition(); + return ( // role is either radio, checkbox, or menuitem which are all interactive // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions @@ -145,11 +159,12 @@ function ContextMenuOption({ /> { setSubmenuOpen(false); - }}> + }} + x={submenuPosition[0]} + y={submenuPosition[1]}> {subOptions} @@ -226,12 +241,6 @@ ContextMenuOption.propTypes = { */ level: PropTypes.number, - /** - * The x position of the root menu. - * Is automatically set by ContextMenu - */ - menuX: PropTypes.number, - /** * Rendered icon for the ContextMenuOption. * Can be a React component class diff --git a/packages/react/src/components/ContextMenu/_utils.js b/packages/react/src/components/ContextMenu/_utils.js index 62bf5c08ffa4..4beb2532df5b 100644 --- a/packages/react/src/components/ContextMenu/_utils.js +++ b/packages/react/src/components/ContextMenu/_utils.js @@ -59,9 +59,23 @@ export function getFirstSubNode(node) { } export function getParentNode(node) { - const parentNode = node.parentNode.closest( - `li.${prefix}--context-menu-option` - ); + if (node) { + const parentNode = node.parentNode.closest( + `li.${prefix}--context-menu-option` + ); + + return parentNode?.firstChild || null; + } + + return null; +} + +export function getParentMenu(el) { + if (el) { + const parentMenu = el.parentNode.closest(`ul.${prefix}--context-menu`); - return parentNode?.firstChild || null; + return parentMenu || null; + } + + return null; } From 852668224a22ef7d3b7f300fe23c15bae6d34c83 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Mon, 18 Jan 2021 16:22:08 +0100 Subject: [PATCH 31/54] fix(context-menu): implement review feedback --- .../context-menu/_context-menu.scss | 7 +- .../ContextMenu/ContextMenu-story.js | 103 +++++++++++++++--- 2 files changed, 92 insertions(+), 18 deletions(-) diff --git a/packages/components/src/components/context-menu/_context-menu.scss b/packages/components/src/components/context-menu/_context-menu.scss index 0c6029584c7b..4a4d48003dd1 100644 --- a/packages/components/src/components/context-menu/_context-menu.scss +++ b/packages/components/src/components/context-menu/_context-menu.scss @@ -20,7 +20,7 @@ position: fixed; z-index: z('modal'); min-width: 13rem; - max-width: 20rem; + max-width: 18rem; padding: $spacing-02 0; background-color: $ui-01; visibility: hidden; @@ -46,8 +46,7 @@ } .#{$prefix}--context-menu-option--active, - .#{$prefix}--context-menu-option:hover, - .#{$prefix}--context-menu-option:focus-within { + .#{$prefix}--context-menu-option:hover { background-color: $hover-ui; } @@ -70,6 +69,7 @@ } .#{$prefix}--context-menu-option__content--disabled { + background-color: $ui-01; cursor: not-allowed; } @@ -101,6 +101,7 @@ } .#{$prefix}--context-menu-option__info { + display: inline-flex; margin-left: $spacing-05; color: $icon-01; } diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 89d85c49f993..98863724ade6 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -24,7 +24,7 @@ export default { }, }; -export const _ContextMenu = () => { +function useContextMenu() { const [open, setOpen] = useState(false); const [position, setPosition] = useState([0, 0]); @@ -37,6 +37,10 @@ export const _ContextMenu = () => { setOpen(true); } + function onClose() { + setOpen(false); + } + useEffect(() => { document.addEventListener('contextmenu', openContextMenu); @@ -45,22 +49,91 @@ export const _ContextMenu = () => { }; }); + return { + open, + x: position[0], + y: position[1], + onClose, + }; +} + +const InfoBanner = () => ( + +); + +export const _ContextMenu = () => { + const contextMenuProps = useContextMenu(); + + return ( +
    + + + + + + + + + + + + + + + +
    + ); +}; + +export const WithIcons = () => { + const contextMenuProps = useContextMenu(); + return (
    - - { - setOpen(false); - }}> + + Date: Mon, 18 Jan 2021 16:39:37 +0100 Subject: [PATCH 32/54] fix(context-menu): don't close when disabled or divider was clicked --- .../react/src/components/ContextMenu/ContextMenu.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index dd411edbacbb..21df26f57334 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -91,12 +91,16 @@ const ContextMenu = function ContextMenu({ } } - function handleClick() { - onClose(); + function handleClick(e) { + if (e.target.tagName !== 'UL') { + onClose(); + } } function handleClickOutside() { - onClose(); + if (open) { + onClose(); + } } function getCorrectedPosition(assumedDirection) { From b1e8a2a121a5430e6e6650e197a1bfe29ecf0679 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Tue, 19 Jan 2021 09:59:41 +0100 Subject: [PATCH 33/54] fix(context-menu): prevent menu from closing immediately in Safari --- .../src/components/ContextMenu/ContextMenu.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index 21df26f57334..cfe4f4ada10c 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -41,6 +41,7 @@ const ContextMenu = function ContextMenu({ 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; function focusNode(node) { @@ -98,7 +99,7 @@ const ContextMenu = function ContextMenu({ } function handleClickOutside() { - if (open) { + if (open && canBeClosed) { onClose(); } } @@ -156,6 +157,8 @@ const ContextMenu = function ContextMenu({ } useEffect(() => { + setCanBeClosed(false); + if (open) { let localDirection = 1; @@ -171,6 +174,18 @@ const ContextMenu = function ContextMenu({ const correctedPosition = getCorrectedPosition(localDirection); setPosition(correctedPosition); + + document.addEventListener( + 'mouseup', + () => { + // wait until mouse button is released before allowing ClickListener + // to close context menu as Safari emits 'click' event after 'contextmenu' + // event when 'e.preventDefault()' is used on 'contextmenu' and would + // otherwise close the menu immediately + setCanBeClosed(true); + }, + { once: true } + ); } else { setPosition([0, 0]); } From 0cabaa9145a4993a0ca57d2cde5c8bc3d66153f3 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 5 Feb 2021 13:44:40 +0100 Subject: [PATCH 34/54] refactor(context-menu): separate between external and internal component --- .../ContextMenu/ContextMenu-story.js | 83 +++---------------- .../src/components/ContextMenu/ContextMenu.js | 1 - .../components/ContextMenu/ContextMenuItem.js | 60 ++++++++++++++ .../react/src/components/ContextMenu/index.js | 6 +- packages/react/src/index.js | 2 +- 5 files changed, 74 insertions(+), 78 deletions(-) create mode 100644 packages/react/src/components/ContextMenu/ContextMenuItem.js diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 98863724ade6..44002687d119 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -7,11 +7,10 @@ import React, { useEffect, useState } from 'react'; import { action } from '@storybook/addon-actions'; -import { FolderShared16, Edit16, TrashCan16 } from '@carbon/icons-react'; import { InlineNotification } from '../Notification'; import ContextMenu, { - ContextMenuOption, + ContextMenuItem, ContextMenuDivider, ContextMenuSelectableOption, ContextMenuRadioGroup, @@ -74,101 +73,41 @@ export const _ContextMenu = () => {
    - + - + - - - - - - - - - -
    - ); -}; - -export const WithIcons = () => { - const contextMenuProps = useContextMenu(); - - return ( -
    - - - - - - - - - - - + { onChange={action('onChange')} /> - - diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index cfe4f4ada10c..5cf67affa7b1 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -195,7 +195,6 @@ const ContextMenu = function ContextMenu({ const someNodesHaveIcons = React.Children.toArray(children).some( (node) => - node.props.renderIcon || node.type === ContextMenuSelectableOption || node.type === ContextMenuRadioGroup ); diff --git a/packages/react/src/components/ContextMenu/ContextMenuItem.js b/packages/react/src/components/ContextMenu/ContextMenuItem.js new file mode 100644 index 000000000000..4186633b3903 --- /dev/null +++ b/packages/react/src/components/ContextMenu/ContextMenuItem.js @@ -0,0 +1,60 @@ +/** + * Copyright IBM Corp. 2020 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +import React from 'react'; +import PropTypes from 'prop-types'; + +import ContextMenuOption from './ContextMenuOption'; + +function ContextMenuItem({ + label, + children, + disabled, + shortcut, + shortcutText, + ...rest +}) { + return ( + + {children} + + ); +} + +ContextMenuItem.propTypes = { + /** + * Specify the children of the ContextMenuItem + */ + children: PropTypes.node, + + /** + * Specify whether this ContextMenuItem is disabled + */ + disabled: PropTypes.bool, + + /** + * Rendered label for the ContextMenuItem + */ + label: PropTypes.node.isRequired, + + /** + * Rendered shortcut for the ContextMenuItem + */ + shortcut: PropTypes.node, + + /** + * The text-only representation of the shortcut read to screen readers + */ + shortcutText: PropTypes.string, +}; + +export default ContextMenuItem; diff --git a/packages/react/src/components/ContextMenu/index.js b/packages/react/src/components/ContextMenu/index.js index 5d34e5247c05..887888fbc00d 100644 --- a/packages/react/src/components/ContextMenu/index.js +++ b/packages/react/src/components/ContextMenu/index.js @@ -6,18 +6,18 @@ */ import ContextMenu from './ContextMenu'; -import ContextMenuOption from './ContextMenuOption'; +import ContextMenuItem from './ContextMenuItem'; import ContextMenuDivider from './ContextMenuDivider'; import ContextMenuSelectableOption from './ContextMenuSelectableOption'; import ContextMenuRadioGroup from './ContextMenuRadioGroup'; -ContextMenu.ContextMenuOption = ContextMenuOption; +ContextMenu.ContextMenuItem = ContextMenuItem; ContextMenu.ContextMenuDivider = ContextMenuDivider; ContextMenu.ContextMenuSelectableOption = ContextMenuSelectableOption; ContextMenu.ContextMenuRadioGroup = ContextMenuRadioGroup; export { - ContextMenuOption, + ContextMenuItem, ContextMenuDivider, ContextMenuSelectableOption, ContextMenuRadioGroup, diff --git a/packages/react/src/index.js b/packages/react/src/index.js index 9fea81c458f5..0646101f6e20 100644 --- a/packages/react/src/index.js +++ b/packages/react/src/index.js @@ -21,7 +21,7 @@ export ComposedModal, { } from './components/ComposedModal'; export ContentSwitcher from './components/ContentSwitcher'; export ContextMenu, { - ContextMenuOption, + ContextMenuItem, ContextMenuDivider, ContextMenuSelectableOption, ContextMenuRadioGroup, From ba3f46ff32d8d1cab7e83ef7f4d542ec2f2b0efa Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 5 Feb 2021 13:58:38 +0100 Subject: [PATCH 35/54] refactor(context-menu): mark as unstable --- .../__snapshots__/PublicAPI-test.js.snap | 314 ++++++++---------- .../ContextMenu/ContextMenu-story.js | 29 +- packages/react/src/index.js | 12 +- 3 files changed, 163 insertions(+), 192 deletions(-) diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 0eb6b2471319..9f2bf8d6b234 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -847,182 +847,6 @@ Map { }, }, }, - "ContextMenu" => Object { - "ContextMenuDivider": Object {}, - "ContextMenuOption": Object { - "propTypes": Object { - "children": Object { - "type": "node", - }, - "disabled": Object { - "type": "bool", - }, - "indented": Object { - "type": "bool", - }, - "label": Object { - "isRequired": true, - "type": "node", - }, - "level": Object { - "type": "number", - }, - "renderIcon": Object { - "args": Array [ - Array [ - Object { - "type": "func", - }, - Object { - "type": "object", - }, - ], - ], - "type": "oneOfType", - }, - "shortcut": Object { - "type": "node", - }, - "shortcutText": Object { - "type": "string", - }, - }, - }, - "ContextMenuRadioGroup": Object { - "propTypes": Object { - "initialSelectedItem": Object { - "type": "string", - }, - "items": Object { - "args": Array [ - Object { - "type": "string", - }, - ], - "isRequired": true, - "type": "arrayOf", - }, - "label": Object { - "isRequired": true, - "type": "string", - }, - "onChange": Object { - "type": "func", - }, - }, - }, - "ContextMenuSelectableOption": Object { - "propTypes": Object { - "initialChecked": Object { - "type": "bool", - }, - "label": Object { - "isRequired": true, - "type": "node", - }, - "onChange": Object { - "type": "func", - }, - }, - }, - "propTypes": Object { - "children": Object { - "type": "node", - }, - "level": Object { - "type": "number", - }, - "onClose": Object { - "type": "func", - }, - "open": Object { - "type": "bool", - }, - "x": Object { - "type": "number", - }, - "y": Object { - "type": "number", - }, - }, - }, - "ContextMenuOption" => Object { - "propTypes": Object { - "children": Object { - "type": "node", - }, - "disabled": Object { - "type": "bool", - }, - "indented": Object { - "type": "bool", - }, - "label": Object { - "isRequired": true, - "type": "node", - }, - "level": Object { - "type": "number", - }, - "renderIcon": Object { - "args": Array [ - Array [ - Object { - "type": "func", - }, - Object { - "type": "object", - }, - ], - ], - "type": "oneOfType", - }, - "shortcut": Object { - "type": "node", - }, - "shortcutText": Object { - "type": "string", - }, - }, - }, - "ContextMenuDivider" => Object {}, - "ContextMenuSelectableOption" => Object { - "propTypes": Object { - "initialChecked": Object { - "type": "bool", - }, - "label": Object { - "isRequired": true, - "type": "node", - }, - "onChange": Object { - "type": "func", - }, - }, - }, - "ContextMenuRadioGroup" => Object { - "propTypes": Object { - "initialSelectedItem": Object { - "type": "string", - }, - "items": Object { - "args": Array [ - Object { - "type": "string", - }, - ], - "isRequired": true, - "type": "arrayOf", - }, - "label": Object { - "isRequired": true, - "type": "string", - }, - "onChange": Object { - "type": "func", - }, - }, - }, "Copy" => Object { "defaultProps": Object { "feedback": "Copied!", @@ -7665,5 +7489,143 @@ Map { }, }, }, + "unstable_ContextMenu" => Object { + "ContextMenuDivider": Object {}, + "ContextMenuItem": Object { + "propTypes": Object { + "children": Object { + "type": "node", + }, + "disabled": Object { + "type": "bool", + }, + "label": Object { + "isRequired": true, + "type": "node", + }, + "shortcut": Object { + "type": "node", + }, + "shortcutText": Object { + "type": "string", + }, + }, + }, + "ContextMenuRadioGroup": Object { + "propTypes": Object { + "initialSelectedItem": Object { + "type": "string", + }, + "items": Object { + "args": Array [ + Object { + "type": "string", + }, + ], + "isRequired": true, + "type": "arrayOf", + }, + "label": Object { + "isRequired": true, + "type": "string", + }, + "onChange": Object { + "type": "func", + }, + }, + }, + "ContextMenuSelectableOption": Object { + "propTypes": Object { + "initialChecked": Object { + "type": "bool", + }, + "label": Object { + "isRequired": true, + "type": "node", + }, + "onChange": Object { + "type": "func", + }, + }, + }, + "propTypes": Object { + "children": Object { + "type": "node", + }, + "level": Object { + "type": "number", + }, + "onClose": Object { + "type": "func", + }, + "open": Object { + "type": "bool", + }, + "x": Object { + "type": "number", + }, + "y": Object { + "type": "number", + }, + }, + }, + "unstable_ContextMenuItem" => Object { + "propTypes": Object { + "children": Object { + "type": "node", + }, + "disabled": Object { + "type": "bool", + }, + "label": Object { + "isRequired": true, + "type": "node", + }, + "shortcut": Object { + "type": "node", + }, + "shortcutText": Object { + "type": "string", + }, + }, + }, + "unstable_ContextMenuDivider" => Object {}, + "unstable_ContextMenuSelectableOption" => Object { + "propTypes": Object { + "initialChecked": Object { + "type": "bool", + }, + "label": Object { + "isRequired": true, + "type": "node", + }, + "onChange": Object { + "type": "func", + }, + }, + }, + "unstable_ContextMenuRadioGroup" => Object { + "propTypes": Object { + "initialSelectedItem": Object { + "type": "string", + }, + "items": Object { + "args": Array [ + Object { + "type": "string", + }, + ], + "isRequired": true, + "type": "arrayOf", + }, + "label": Object { + "isRequired": true, + "type": "string", + }, + "onChange": Object { + "type": "func", + }, + }, + }, } `; diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 44002687d119..f722923b5a39 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -17,7 +17,7 @@ import ContextMenu, { } from '../ContextMenu'; export default { - title: 'ContextMenu', + title: 'unstable_ContextMenu', parameters: { component: ContextMenu, }, @@ -56,14 +56,23 @@ function useContextMenu() { }; } -const InfoBanner = () => ( - +const InfoBanners = () => ( + <> + + + ); export const _ContextMenu = () => { @@ -71,7 +80,7 @@ export const _ContextMenu = () => { return (
    - + Date: Fri, 5 Feb 2021 14:15:16 +0100 Subject: [PATCH 36/54] feat(context-menu): export useContextMenu hook --- .../__snapshots__/PublicAPI-test.js.snap | 1 + .../ContextMenu/ContextMenu-story.js | 36 +-------------- .../react/src/components/ContextMenu/index.js | 3 ++ .../components/ContextMenu/useContextMenu.js | 46 +++++++++++++++++++ packages/react/src/index.js | 1 + 5 files changed, 53 insertions(+), 34 deletions(-) create mode 100644 packages/react/src/components/ContextMenu/useContextMenu.js diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 9f2bf8d6b234..792b4d73525d 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -7627,5 +7627,6 @@ Map { }, }, }, + "unstable_useContextMenu" => Object {}, } `; diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index f722923b5a39..e4bcfcda34f5 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import React, { useEffect, useState } from 'react'; +import React from 'react'; import { action } from '@storybook/addon-actions'; import { InlineNotification } from '../Notification'; @@ -14,6 +14,7 @@ import ContextMenu, { ContextMenuDivider, ContextMenuSelectableOption, ContextMenuRadioGroup, + useContextMenu, } from '../ContextMenu'; export default { @@ -23,39 +24,6 @@ export default { }, }; -function useContextMenu() { - const [open, setOpen] = useState(false); - const [position, setPosition] = useState([0, 0]); - - function openContextMenu(e) { - e.preventDefault(); - - const { x, y } = e; - - setPosition([x, y]); - setOpen(true); - } - - function onClose() { - setOpen(false); - } - - useEffect(() => { - document.addEventListener('contextmenu', openContextMenu); - - return () => { - document.removeEventListener('contextmenu', openContextMenu); - }; - }); - - return { - open, - x: position[0], - y: position[1], - onClose, - }; -} - const InfoBanners = () => ( <> { + if ( + (trigger && trigger instanceof Element) || + trigger instanceof Document || + trigger instanceof Window + ) { + trigger.addEventListener('contextmenu', openContextMenu); + + return () => { + trigger.removeEventListener('contextmenu', openContextMenu); + }; + } + }); + + return { + open, + x: position[0], + y: position[1], + onClose, + }; +} + +export default useContextMenu; diff --git a/packages/react/src/index.js b/packages/react/src/index.js index 3032d6855429..d0f1bf7b670a 100644 --- a/packages/react/src/index.js +++ b/packages/react/src/index.js @@ -211,4 +211,5 @@ export unstable_ContextMenu, { ContextMenuDivider as unstable_ContextMenuDivider, ContextMenuSelectableOption as unstable_ContextMenuSelectableOption, ContextMenuRadioGroup as unstable_ContextMenuRadioGroup, + useContextMenu as unstable_useContextMenu, } from './components/ContextMenu'; From 37734be530b20cb542667a80b0fe04e531c505be Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 5 Feb 2021 14:36:59 +0100 Subject: [PATCH 37/54] test(context-menu): replace Option component with Item component --- .../ContextMenu/ContextMenu-test.js | 31 ++++++------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu-test.js b/packages/react/src/components/ContextMenu/ContextMenu-test.js index 258245bbd715..b94aa40202ec 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-test.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-test.js @@ -7,14 +7,13 @@ import React from 'react'; import ContextMenu, { - ContextMenuOption, + ContextMenuItem, ContextMenuRadioGroup, ContextMenuSelectableOption, ContextMenuDivider, } from '../ContextMenu'; import { mount } from 'enzyme'; import { settings } from 'carbon-components'; -import { Copy16 } from '@carbon/icons-react'; import { describe, expect } from 'window-or-global'; const { prefix } = settings; @@ -42,14 +41,14 @@ describe('ContextMenu', () => { describe('option', () => { it('receives the expected classes', () => { - const wrapper = mount(); - const container = wrapper.childAt(0); + const wrapper = mount(); + const container = wrapper.childAt(0).childAt(0); expect(container.hasClass(`${prefix}--context-menu-option`)).toBe(true); }); it('renders props.label', () => { - const wrapper = mount(); + const wrapper = mount(); expect( wrapper.find(`span.${prefix}--context-menu-option__label`).text() @@ -61,19 +60,9 @@ describe('ContextMenu', () => { ).toBe('Copy'); }); - it('renders props.renderIcon when provided', () => { - const wrapper = mount( - - ); - - expect( - wrapper.find(`div.${prefix}--context-menu-option__icon`).length - ).toBeGreaterThan(0); - }); - it('renders props.shortcut when provided', () => { const wrapper = mount( - { }); it('respects props.disabled', () => { - const wrapper = mount(); + const wrapper = mount(); const content = wrapper.find( `button.${prefix}--context-menu-option__content` ); @@ -111,10 +100,10 @@ describe('ContextMenu', () => { it('renders props.children as submenu', () => { const wrapper = mount( - - - - + + + + ); From f3fc9d729eb68c55bd97616cfbb9a623f6430da5 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 5 Feb 2021 16:31:37 +0100 Subject: [PATCH 38/54] test: update react index snapshot --- packages/react/src/__tests__/index-test.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/react/src/__tests__/index-test.js b/packages/react/src/__tests__/index-test.js index c2cab273fbe5..24cde183b99c 100644 --- a/packages/react/src/__tests__/index-test.js +++ b/packages/react/src/__tests__/index-test.js @@ -35,11 +35,6 @@ describe('Carbon Components React', () => { "ComposedModal", "Content", "ContentSwitcher", - "ContextMenu", - "ContextMenuDivider", - "ContextMenuOption", - "ContextMenuRadioGroup", - "ContextMenuSelectableOption", "Copy", "CopyButton", "DangerButton", @@ -198,10 +193,16 @@ describe('Carbon Components React', () => { "TooltipDefinition", "TooltipIcon", "UnorderedList", + "unstable_ContextMenu", + "unstable_ContextMenuDivider", + "unstable_ContextMenuItem", + "unstable_ContextMenuRadioGroup", + "unstable_ContextMenuSelectableOption", "unstable_PageSelector", "unstable_Pagination", "unstable_TreeNode", "unstable_TreeView", + "unstable_useContextMenu", ] `); }); From 5d53550e4f29627559d9ba4190fc959baad00a0c Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Wed, 10 Feb 2021 14:01:26 +0100 Subject: [PATCH 39/54] fix(context-menu): open submenus on enter --- .../react/src/components/ContextMenu/ContextMenu.js | 12 ++++++------ .../src/components/ContextMenu/ContextMenuOption.js | 2 +- packages/react/src/components/ContextMenu/_utils.js | 11 +++++++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index 5cf67affa7b1..7fe7c6bf6794 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -13,11 +13,11 @@ import { keys, match } from '../../internal/keyboard'; import ClickListener from '../../internal/ClickListener'; import { + clickedElementHasSubnodes, getValidNodes, resetFocus, focusNode as focusNodeUtil, getNextNode, - getFirstSubNode, getParentNode, getParentMenu, } from './_utils'; @@ -70,8 +70,6 @@ const ContextMenu = function ContextMenu({ nodeToFocus = getNextNode(currentNode, -1); } else if (match(event, keys.ArrowDown)) { nodeToFocus = getNextNode(currentNode, 1); - } else if (match(event, keys.ArrowRight)) { - nodeToFocus = getFirstSubNode(currentNode); } else if (match(event, keys.ArrowLeft)) { nodeToFocus = getParentNode(currentNode); } @@ -93,13 +91,15 @@ const ContextMenu = function ContextMenu({ } function handleClick(e) { - if (e.target.tagName !== 'UL') { + if (!clickedElementHasSubnodes(e) && e.target.tagName !== 'UL') { onClose(); + } else { + e.stopPropagation(); } } - function handleClickOutside() { - if (open && canBeClosed) { + function handleClickOutside(e) { + if (!clickedElementHasSubnodes(e) && open && canBeClosed) { onClose(); } } diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index 3afd03023201..4577bf226e46 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -80,7 +80,7 @@ function ContextMenuOption({ } function handleKeyDown(event) { - if (match(event, keys.ArrowRight)) { + if (match(event, keys.ArrowRight) || match(event, keys.Enter)) { openSubmenu(true); } } diff --git a/packages/react/src/components/ContextMenu/_utils.js b/packages/react/src/components/ContextMenu/_utils.js index 4beb2532df5b..f27795a66aca 100644 --- a/packages/react/src/components/ContextMenu/_utils.js +++ b/packages/react/src/components/ContextMenu/_utils.js @@ -79,3 +79,14 @@ export function getParentMenu(el) { return null; } + +export function clickedElementHasSubnodes(e) { + if (e) { + const closestFocusableElement = e.target.closest('[tabindex]'); + if (closestFocusableElement?.tagName === 'BUTTON') { + return getFirstSubNode(closestFocusableElement.parentNode) !== null; + } + } + + return false; +} From 92115b2132f2e921148ec7e751358c5f692458a6 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Wed, 10 Feb 2021 14:25:31 +0100 Subject: [PATCH 40/54] fix(context-menu): fix lint issue --- .../react/src/components/ContextMenu/ContextMenu.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index 7fe7c6bf6794..aeacd4b39529 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -128,14 +128,20 @@ const ContextMenu = function ContextMenu({ } // make sure menu is visible in y bounds - if (pos[1] > max[1]) pos[1] = max[1]; - if (pos[1] < min[1]) pos[1] = min[1]; + if (pos[1] > max[1]) { + pos[1] = max[1]; + } + if (pos[1] < min[1]) { + pos[1] = min[1]; + } if (localDirection === 1) { // if it won't fit anymore if (pos[0] > max[0]) { pos[0] = x - width - parentWidth; - if (pos[0] + width > bodyWidth - margin) pos[0] = max[0]; + if (pos[0] + width > bodyWidth - margin) { + pos[0] = max[0]; + } localDirection = -1; } else if (pos[0] < min[0]) { // keep distance to left screen edge From b8d7d0fa371bbe41a9e4b4e501d898f93510454d Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 12 Feb 2021 09:20:14 +0100 Subject: [PATCH 41/54] fix(context-menu): use menuitem* roles for checkbox and radio --- .../react/__tests__/__snapshots__/PublicAPI-test.js.snap | 4 ++-- packages/react/src/__tests__/index-test.js | 2 +- .../react/src/components/ContextMenu/ContextMenu-story.js | 4 ++-- packages/react/src/components/ContextMenu/ContextMenu.js | 4 ++-- .../react/src/components/ContextMenu/ContextMenuOption.js | 4 ++-- .../src/components/ContextMenu/ContextMenuRadioGroup.js | 2 +- ...nuSelectableOption.js => ContextMenuSelectableItem.js} | 8 ++++---- packages/react/src/components/ContextMenu/index.js | 6 +++--- packages/react/src/index.js | 2 +- 9 files changed, 18 insertions(+), 18 deletions(-) rename packages/react/src/components/ContextMenu/{ContextMenuSelectableOption.js => ContextMenuSelectableItem.js} (87%) diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 3beb4e3d0551..29f69e342652 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -7624,7 +7624,7 @@ Map { }, }, }, - "ContextMenuSelectableOption": Object { + "ContextMenuSelectableItem": Object { "propTypes": Object { "initialChecked": Object { "type": "bool", @@ -7680,7 +7680,7 @@ Map { }, }, "unstable_ContextMenuDivider" => Object {}, - "unstable_ContextMenuSelectableOption" => Object { + "unstable_ContextMenuSelectableItem" => Object { "propTypes": Object { "initialChecked": Object { "type": "bool", diff --git a/packages/react/src/__tests__/index-test.js b/packages/react/src/__tests__/index-test.js index 69d6f55aae99..91b7322a50c0 100644 --- a/packages/react/src/__tests__/index-test.js +++ b/packages/react/src/__tests__/index-test.js @@ -198,7 +198,7 @@ describe('Carbon Components React', () => { "unstable_ContextMenuDivider", "unstable_ContextMenuItem", "unstable_ContextMenuRadioGroup", - "unstable_ContextMenuSelectableOption", + "unstable_ContextMenuSelectableItem", "unstable_PageSelector", "unstable_Pagination", "unstable_TreeNode", diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index e4bcfcda34f5..3cd6a9c4ae2e 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -12,7 +12,7 @@ import { InlineNotification } from '../Notification'; import ContextMenu, { ContextMenuItem, ContextMenuDivider, - ContextMenuSelectableOption, + ContextMenuSelectableItem, ContextMenuRadioGroup, useContextMenu, } from '../ContextMenu'; @@ -86,7 +86,7 @@ export const _ContextMenu = () => { /> - - node.type === ContextMenuSelectableOption || + node.type === ContextMenuSelectableItem || node.type === ContextMenuRadioGroup ); diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index 4577bf226e46..b6ae4efc344f 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -126,7 +126,7 @@ function ContextMenuOption({ [`${prefix}--context-menu-option--active`]: subOptions && submenuOpen, }); - const allowedRoles = ['radio', 'checkbox']; + const allowedRoles = ['menuitemradio', 'menuitemcheckbox']; const role = rest.role && allowedRoles.includes(rest.role) ? rest.role : 'menuitem'; @@ -136,7 +136,7 @@ function ContextMenuOption({ const submenuPosition = getSubmenuPosition(); return ( - // role is either radio, checkbox, or menuitem which are all interactive + // role is either menuitemradio, menuitemcheckbox, or menuitem which are all interactive // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
  • {}, @@ -24,7 +24,7 @@ function ContextMenuSelectableOption({ return ( Date: Fri, 12 Feb 2021 09:27:41 +0100 Subject: [PATCH 42/54] fix(context-menu): remove aria-label from menuitems --- .../__snapshots__/PublicAPI-test.js.snap | 6 ------ .../components/ContextMenu/ContextMenu-test.js | 4 ---- .../components/ContextMenu/ContextMenuItem.js | 17 ++--------------- .../components/ContextMenu/ContextMenuOption.js | 10 ---------- 4 files changed, 2 insertions(+), 35 deletions(-) diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 29f69e342652..b871e7d7fb0f 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -7596,9 +7596,6 @@ Map { "shortcut": Object { "type": "node", }, - "shortcutText": Object { - "type": "string", - }, }, }, "ContextMenuRadioGroup": Object { @@ -7674,9 +7671,6 @@ Map { "shortcut": Object { "type": "node", }, - "shortcutText": Object { - "type": "string", - }, }, }, "unstable_ContextMenuDivider" => Object {}, diff --git a/packages/react/src/components/ContextMenu/ContextMenu-test.js b/packages/react/src/components/ContextMenu/ContextMenu-test.js index b94aa40202ec..fe0ee3a4d964 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-test.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-test.js @@ -75,9 +75,6 @@ describe('ContextMenu', () => { expect( wrapper.find(`div.${prefix}--context-menu-option__info`).text() ).toBe('⌘C'); - expect( - wrapper.find(`li.${prefix}--context-menu-option`).prop('aria-label') - ).toContain('command c'); }); it('respects props.disabled', () => { @@ -109,7 +106,6 @@ describe('ContextMenu', () => { const level1 = wrapper.find(`li.${prefix}--context-menu-option`).at(0); - expect(level1.prop('aria-label')).toBe('Format'); expect( level1.find(`ul.${prefix}--context-menu`).length ).toBeGreaterThan(0); diff --git a/packages/react/src/components/ContextMenu/ContextMenuItem.js b/packages/react/src/components/ContextMenu/ContextMenuItem.js index 4186633b3903..2cf581420e25 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuItem.js +++ b/packages/react/src/components/ContextMenu/ContextMenuItem.js @@ -10,21 +10,13 @@ import PropTypes from 'prop-types'; import ContextMenuOption from './ContextMenuOption'; -function ContextMenuItem({ - label, - children, - disabled, - shortcut, - shortcutText, - ...rest -}) { +function ContextMenuItem({ label, children, disabled, shortcut, ...rest }) { return ( + shortcut={shortcut}> {children} ); @@ -50,11 +42,6 @@ ContextMenuItem.propTypes = { * Rendered shortcut for the ContextMenuItem */ shortcut: PropTypes.node, - - /** - * The text-only representation of the shortcut read to screen readers - */ - shortcutText: PropTypes.string, }; export default ContextMenuItem; diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index b6ae4efc344f..9262dda16949 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -57,7 +57,6 @@ function ContextMenuOption({ children, disabled, shortcut, - shortcutText, renderIcon, indented, level, @@ -130,9 +129,6 @@ function ContextMenuOption({ const role = rest.role && allowedRoles.includes(rest.role) ? rest.role : 'menuitem'; - const ariaLabel = - shortcut && shortcutText ? `${label}, ${shortcutText}` : label; - const submenuPosition = getSubmenuPosition(); return ( @@ -143,7 +139,6 @@ function ContextMenuOption({ ref={rootRef} className={classes} role={role} - aria-label={ariaLabel} aria-disabled={!subOptions && disabled} onKeyDown={subOptions ? handleKeyDown : null} onMouseEnter={subOptions ? handleMouseEnter : null} @@ -251,11 +246,6 @@ ContextMenuOption.propTypes = { * Rendered shortcut for the ContextMenuOption */ shortcut: PropTypes.node, - - /** - * The text-only representation of the shortcut read to screen readers - */ - shortcutText: PropTypes.string, }; export default ContextMenuOption; From 8baccee3b8c0a66e2ca5244a10530af57f1c1069 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 12 Feb 2021 10:50:48 +0100 Subject: [PATCH 43/54] refactor(context-menu): remove button markup --- .../context-menu/_context-menu.scss | 12 ++--- .../src/components/ContextMenu/ContextMenu.js | 15 +++--- .../ContextMenu/ContextMenuOption.js | 49 +++++++++++-------- .../src/components/ContextMenu/_utils.js | 10 ++-- 4 files changed, 47 insertions(+), 39 deletions(-) diff --git a/packages/components/src/components/context-menu/_context-menu.scss b/packages/components/src/components/context-menu/_context-menu.scss index 4a4d48003dd1..b5f74b7c6211 100644 --- a/packages/components/src/components/context-menu/_context-menu.scss +++ b/packages/components/src/components/context-menu/_context-menu.scss @@ -8,7 +8,6 @@ @import '../../globals/scss/vars'; @import '../../globals/scss/vendor/@carbon/elements/scss/import-once/import-once'; @import '../../globals/scss/helper-mixins'; -@import '../../globals/scss/css--reset'; /// Context Menu styles /// @access private @@ -42,7 +41,12 @@ position: relative; height: $spacing-07; background-color: $ui-01; + cursor: pointer; transition: background-color $duration--fast-01 motion(standard, productive); + + &:focus { + @include focus-outline('outline'); + } } .#{$prefix}--context-menu-option--active, @@ -55,17 +59,11 @@ } .#{$prefix}--context-menu-option__content { - @include button-reset; - display: flex; align-items: center; justify-content: space-between; height: 100%; padding: 0 $spacing-05; - - &:focus { - @include focus-outline('outline'); - } } .#{$prefix}--context-menu-option__content--disabled { diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index c224a715bc11..2e92cf7e71d5 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -52,7 +52,11 @@ const ContextMenu = function ContextMenu({ } function handleKeyDown(event) { - event.stopPropagation(); + if (event.target.tagName === 'LI' && match(event, keys.Enter)) { + handleClick(event); + } else { + event.stopPropagation(); + } if ( match(event, keys.Escape) || @@ -63,8 +67,8 @@ const ContextMenu = function ContextMenu({ let nodeToFocus; - if (event.target.tagName === 'BUTTON') { - const currentNode = event.target.parentNode; + if (event.target.tagName === 'LI') { + const currentNode = event.target; if (match(event, keys.ArrowUp)) { nodeToFocus = getNextNode(currentNode, -1); @@ -77,9 +81,9 @@ const ContextMenu = function ContextMenu({ const validNodes = getValidNodes(event.target); if (validNodes.length > 0 && match(event, keys.ArrowUp)) { - nodeToFocus = validNodes[validNodes.length - 1].firstChild; + nodeToFocus = validNodes[validNodes.length - 1]; } else if (validNodes.length > 0 && match(event, keys.ArrowDown)) { - nodeToFocus = validNodes[0].firstChild; + nodeToFocus = validNodes[0]; } } @@ -230,7 +234,6 @@ const ContextMenu = function ContextMenu({ onClick={handleClick} role="menu" tabIndex={-1} - data-level={level} data-direction={direction} style={{ left: `${position[0]}px`, diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index 9262dda16949..7882d81fe2e0 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -12,7 +12,12 @@ import { settings } from 'carbon-components'; import { CaretRight16 } from '@carbon/icons-react'; import { keys, match } from '../../internal/keyboard'; -import { getFirstSubNode, focusNode, getParentMenu } from './_utils'; +import { + getFirstSubNode, + focusNode, + getParentMenu, + clickedElementHasSubnodes, +} from './_utils'; import ContextMenu from './ContextMenu'; @@ -26,19 +31,13 @@ function ContextMenuOptionContent({ disabled, icon: Icon, indented, - ...rest }) { const classes = classnames(`${prefix}--context-menu-option__content`, { [`${prefix}--context-menu-option__content--disabled`]: disabled, }); return ( - +
    • ); } function ContextMenuOption({ - label, children, disabled, - shortcut, - renderIcon, indented, + label, level, + onClick = () => {}, + renderIcon, + shortcut, ...rest }) { const [submenuOpen, setSubmenuOpen] = useState(false); @@ -79,8 +79,13 @@ function ContextMenuOption({ } function handleKeyDown(event) { - if (match(event, keys.ArrowRight) || match(event, keys.Enter)) { + if ( + clickedElementHasSubnodes(event) && + (match(event, keys.ArrowRight) || match(event, keys.Enter)) + ) { openSubmenu(true); + } else if (match(event, keys.Enter)) { + onClick(event); } } @@ -139,10 +144,13 @@ function ContextMenuOption({ ref={rootRef} className={classes} role={role} + tabIndex={-1} aria-disabled={!subOptions && disabled} - onKeyDown={subOptions ? handleKeyDown : null} + aria-haspopup={subOptions} + onKeyDown={handleKeyDown} onMouseEnter={subOptions ? handleMouseEnter : null} - onMouseLeave={subOptions ? handleMouseLeave : null}> + onMouseLeave={subOptions ? handleMouseLeave : null} + onClick={onClick}> {subOptions ? ( <> } indented={indented} - aria-haspopup /> Date: Fri, 12 Feb 2021 10:53:46 +0100 Subject: [PATCH 44/54] test(context-menu): update test to reflect markup changes --- .../ContextMenu/ContextMenu-story.js | 6 ----- .../ContextMenu/ContextMenu-test.js | 23 +++++++------------ 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 3cd6a9c4ae2e..9e4a56a91ef7 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -62,25 +62,21 @@ export const _ContextMenu = () => { @@ -95,13 +91,11 @@ export const _ContextMenu = () => { diff --git a/packages/react/src/components/ContextMenu/ContextMenu-test.js b/packages/react/src/components/ContextMenu/ContextMenu-test.js index fe0ee3a4d964..412b38c17f74 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-test.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-test.js @@ -9,7 +9,7 @@ import React from 'react'; import ContextMenu, { ContextMenuItem, ContextMenuRadioGroup, - ContextMenuSelectableOption, + ContextMenuSelectableItem, ContextMenuDivider, } from '../ContextMenu'; import { mount } from 'enzyme'; @@ -61,13 +61,7 @@ describe('ContextMenu', () => { }); it('renders props.shortcut when provided', () => { - const wrapper = mount( - - ); + const wrapper = mount(); expect( wrapper.find(`div.${prefix}--context-menu-option__info`).length @@ -80,10 +74,9 @@ describe('ContextMenu', () => { it('respects props.disabled', () => { const wrapper = mount(); const content = wrapper.find( - `button.${prefix}--context-menu-option__content` + `div.${prefix}--context-menu-option__content` ); - expect(content.prop('disabled')).toBe(true); expect( content.hasClass(`${prefix}--context-menu-option__content--disabled`) ).toBe(true); @@ -133,22 +126,22 @@ describe('ContextMenu', () => { expect(container.prop('role')).toBe('radiogroup'); }); - it('children have role "radio"', () => { + it('children have role "menuitemradio"', () => { const wrapper = mount( ); const options = wrapper.find(`li.${prefix}--context-menu-option`); - expect(options.every('li[role="radio"]')).toBe(true); + expect(options.every('li[role="menuitemradio"]')).toBe(true); }); }); describe('selectable', () => { - it('has role "checkbox"', () => { - const wrapper = mount(); + it('has role "menuitemcheckbox"', () => { + const wrapper = mount(); const container = wrapper.childAt(0); - expect(container.prop('role')).toBe('checkbox'); + expect(container.prop('role')).toBe('menuitemcheckbox'); }); }); From e6c44cf12c964860d5d0600742013cd55b266087 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Fri, 12 Feb 2021 11:00:58 +0100 Subject: [PATCH 45/54] fix(context-menu): use ul without role for radio groups --- .../react/src/components/ContextMenu/ContextMenu-test.js | 9 --------- .../src/components/ContextMenu/ContextMenuRadioGroup.js | 7 ++----- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu-test.js b/packages/react/src/components/ContextMenu/ContextMenu-test.js index 412b38c17f74..509a49345ee5 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-test.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-test.js @@ -117,15 +117,6 @@ describe('ContextMenu', () => { ); }); - it('has role "radiogroup"', () => { - const wrapper = mount( - - ); - const container = wrapper.childAt(0); - - expect(container.prop('role')).toBe('radiogroup'); - }); - it('children have role "menuitemradio"', () => { const wrapper = mount( diff --git a/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js b/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js index 2e17a497bec7..3bcdbef3b177 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js +++ b/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js @@ -45,12 +45,9 @@ function ContextMenuRadioGroup({ }); return ( -
    +
      {options} -
    + ); } From f3e6c4ad3cd027f69b98a48e76c9649faece60be Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Tue, 23 Feb 2021 10:01:22 +0100 Subject: [PATCH 46/54] fix(context-menu): fix aria-haspopup attribute on parent node --- packages/react/src/components/ContextMenu/ContextMenuOption.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index 7882d81fe2e0..f9e7260bbfd6 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -146,7 +146,7 @@ function ContextMenuOption({ role={role} tabIndex={-1} aria-disabled={!subOptions && disabled} - aria-haspopup={subOptions} + aria-haspopup={subOptions ? true : null} onKeyDown={handleKeyDown} onMouseEnter={subOptions ? handleMouseEnter : null} onMouseLeave={subOptions ? handleMouseLeave : null} From 13bd35ac6197481f988ece5c0f22edbac01802e0 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Tue, 23 Feb 2021 10:06:52 +0100 Subject: [PATCH 47/54] fix(context-menu): add aria-expanded attribute on items with children --- packages/react/src/components/ContextMenu/ContextMenuOption.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index f9e7260bbfd6..366da57128de 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -147,6 +147,7 @@ function ContextMenuOption({ tabIndex={-1} aria-disabled={!subOptions && disabled} aria-haspopup={subOptions ? true : null} + aria-expanded={subOptions ? submenuOpen : null} onKeyDown={handleKeyDown} onMouseEnter={subOptions ? handleMouseEnter : null} onMouseLeave={subOptions ? handleMouseLeave : null} From ae2b0fd04378ad8dbb81f82878f2c507216c435c Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Tue, 23 Feb 2021 10:15:40 +0100 Subject: [PATCH 48/54] fix(context-menu): add "group" role to radio groups --- .../src/components/ContextMenu/ContextMenuRadioGroup.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js b/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js index 3bcdbef3b177..da34a7525a6f 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js +++ b/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js @@ -45,7 +45,10 @@ function ContextMenuRadioGroup({ }); return ( -
      +
        {options}
      ); From a2272b4ddcca352a9351638fd8aba4f0c1db8377 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Tue, 23 Feb 2021 10:50:03 +0100 Subject: [PATCH 49/54] fix(context-menu): extract radio items from group when top level --- .../src/components/ContextMenu/ContextMenu.js | 46 +++++++++----- .../ContextMenu/ContextMenuRadioGroup.js | 48 +++++--------- .../ContextMenuRadioGroupOptions.js | 63 +++++++++++++++++++ 3 files changed, 109 insertions(+), 48 deletions(-) create mode 100644 packages/react/src/components/ContextMenu/ContextMenuRadioGroupOptions.js diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index 2e92cf7e71d5..83615c4e157e 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -24,6 +24,7 @@ import { import ContextMenuSelectableItem from './ContextMenuSelectableItem'; import ContextMenuRadioGroup from './ContextMenuRadioGroup'; +import ContextMenuRadioGroupOptions from './ContextMenuRadioGroupOptions'; const { prefix } = settings; @@ -225,22 +226,39 @@ const ContextMenu = function ContextMenu({ [`${prefix}--context-menu--root`]: isRootMenu, }); + const ulAttributes = { + className: classes, + onKeyDown: handleKeyDown, + onClick: handleClick, + role: 'menu', + tabIndex: -1, + 'data-direction': direction, + style: { + left: `${position[0]}px`, + top: `${position[1]}px`, + }, + }; + + let childrenToRender = options; + + // if the only child is a radio group, don't render radiogroup component, but + // only the children to prevent duplicate group markup + if (options.length === 1 && options[0].type === ContextMenuRadioGroup) { + const radioGroupProps = options[0].props; + + ulAttributes['aria-label'] = radioGroupProps.label; + childrenToRender = ( + + ); + } + return ( -
        - {options} -
      +
        {childrenToRender}
       ); }; diff --git a/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js b/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js index da34a7525a6f..8f65d2634342 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js +++ b/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js @@ -5,11 +5,10 @@ * LICENSE file in the root directory of this source tree. */ -import React, { useState } from 'react'; +import React from 'react'; import PropTypes from 'prop-types'; import { settings } from 'carbon-components'; -import { Checkmark16 } from '@carbon/icons-react'; -import ContextMenuOption from './ContextMenuOption'; +import ContextMenuRadioGroupOptions from './ContextMenuRadioGroupOptions'; const { prefix } = settings; @@ -19,38 +18,19 @@ function ContextMenuRadioGroup({ label, onChange = () => {}, }) { - const [selected, setSelected] = useState(initialSelectedItem); - - function handleClick(option) { - setSelected(option); - onChange(option); - } - - const options = items.map((option, i) => { - const isSelected = selected === option; - - return ( - { - handleClick(option); - }} - /> - ); - }); - return ( -
        - {options} -
      +
    • +
        + +
      +
      • ); } diff --git a/packages/react/src/components/ContextMenu/ContextMenuRadioGroupOptions.js b/packages/react/src/components/ContextMenu/ContextMenuRadioGroupOptions.js new file mode 100644 index 000000000000..19794116c90f --- /dev/null +++ b/packages/react/src/components/ContextMenu/ContextMenuRadioGroupOptions.js @@ -0,0 +1,63 @@ +/** + * Copyright IBM Corp. 2020 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +import React, { useState } from 'react'; +import PropTypes from 'prop-types'; +import { Checkmark16 } from '@carbon/icons-react'; +import ContextMenuOption from './ContextMenuOption'; + +function ContextMenuRadioGroupOptions({ + items, + initialSelectedItem, + onChange = () => {}, +}) { + const [selected, setSelected] = useState(initialSelectedItem); + + function handleClick(option) { + setSelected(option); + onChange(option); + } + + const options = items.map((option, i) => { + const isSelected = selected === option; + + return ( + { + handleClick(option); + }} + /> + ); + }); + + return options; +} + +ContextMenuRadioGroupOptions.propTypes = { + /** + * Whether the option should be checked by default + */ + initialSelectedItem: PropTypes.string, + + /** + * Array of the radio options + */ + items: PropTypes.arrayOf(PropTypes.string).isRequired, + + /** + * Callback function when selection the has been changed + */ + onChange: PropTypes.func, +}; + +export default ContextMenuRadioGroupOptions; From 14198a3324f94aedabdd3e7e09b0f3e51a2ee44c Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Tue, 23 Feb 2021 11:08:57 +0100 Subject: [PATCH 50/54] chore: add story with multiple radio groups --- .../ContextMenu/ContextMenu-story.js | 133 +++++++++++------- 1 file changed, 82 insertions(+), 51 deletions(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 9e4a56a91ef7..28d7012f729f 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -43,64 +43,95 @@ const InfoBanners = () => ( ); -export const _ContextMenu = () => { +const Story = (items) => { const contextMenuProps = useContextMenu(); - return ( -
      - - - + function renderItem(item) { + switch (item.type) { + case 'item': + return ( + + {item.children && item.children.map(renderItem)} + + ); + case 'divider': + return ; + case 'selectable': + return ( + + ); + case 'radiogroup': + return ( - - - - - - - - - - - - - + ); + } + } + + return ( +
      + + {items.map(renderItem)}
      ); }; +export const _ContextMenu = () => + Story([ + { + type: 'item', + label: 'Share with', + children: [ + { + type: 'radiogroup', + label: 'Share with', + items: ['None', 'Product team', 'Organization', 'Company'], + initialSelectedItem: 'Product team', + }, + ], + }, + { type: 'divider' }, + { type: 'item', label: 'Cut', shortcut: '⌘X' }, + { type: 'item', label: 'Copy', shortcut: '⌘C' }, + { type: 'item', label: 'Copy path', shortcut: '⌥⌘C' }, + { type: 'item', label: 'Paste', shortcut: '⌘V' }, + { type: 'item', label: 'Duplicate' }, + { type: 'divider' }, + { type: 'selectable', label: 'Publish', initialChecked: true }, + { type: 'divider' }, + { type: 'item', label: 'Rename', shortcut: '↩︎' }, + { type: 'item', label: 'Delete', shortcut: '⌘⌫' }, + ]); _ContextMenu.storyName = 'ContextMenu'; + +export const _MultipleGroups = () => + Story([ + { type: 'item', label: 'Bold' }, + { type: 'item', label: 'Italic' }, + { type: 'divider' }, + { + type: 'radiogroup', + label: 'Text color', + items: ['Black', 'Blue', 'Red', 'Green'], + initialSelectedItem: 'Black', + }, + { type: 'divider' }, + { + type: 'radiogroup', + label: 'Text decoration', + items: ['None', 'Overline', 'Line-through', 'Underline'], + initialSelectedItem: 'None', + }, + ]); +_MultipleGroups.storyName = 'MultipleGroups'; From ccd98efb3ddbc9f4c00d1a450db92d6b3f4a635a Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Tue, 23 Feb 2021 15:25:49 +0100 Subject: [PATCH 51/54] feat(context-menu): add ContextMenuGroup component --- .../__snapshots__/PublicAPI-test.js.snap | 46 ++++++++++++++----- packages/react/src/__tests__/index-test.js | 1 + .../ContextMenu/ContextMenu-story.js | 24 ++++++++-- .../ContextMenu/ContextMenu-test.js | 11 ----- .../src/components/ContextMenu/ContextMenu.js | 23 ++++++++-- .../ContextMenu/ContextMenuGroup.js | 33 +++++++++++++ .../ContextMenu/ContextMenuOption.js | 2 +- .../ContextMenu/ContextMenuRadioGroup.js | 23 ++++------ .../src/components/ContextMenu/_utils.js | 24 +++++----- .../react/src/components/ContextMenu/index.js | 15 +++--- packages/react/src/index.js | 5 +- 11 files changed, 138 insertions(+), 69 deletions(-) create mode 100644 packages/react/src/components/ContextMenu/ContextMenuGroup.js diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index b871e7d7fb0f..2b15ee50e6cd 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -7581,6 +7581,17 @@ Map { }, "unstable_ContextMenu" => Object { "ContextMenuDivider": Object {}, + "ContextMenuGroup": Object { + "propTypes": Object { + "children": Object { + "type": "node", + }, + "label": Object { + "isRequired": true, + "type": "node", + }, + }, + }, "ContextMenuItem": Object { "propTypes": Object { "children": Object { @@ -7656,35 +7667,32 @@ Map { }, }, }, - "unstable_ContextMenuItem" => Object { + "unstable_ContextMenuDivider" => Object {}, + "unstable_ContextMenuGroup" => Object { "propTypes": Object { "children": Object { "type": "node", }, - "disabled": Object { - "type": "bool", - }, "label": Object { "isRequired": true, "type": "node", }, - "shortcut": Object { - "type": "node", - }, }, }, - "unstable_ContextMenuDivider" => Object {}, - "unstable_ContextMenuSelectableItem" => Object { + "unstable_ContextMenuItem" => Object { "propTypes": Object { - "initialChecked": Object { + "children": Object { + "type": "node", + }, + "disabled": Object { "type": "bool", }, "label": Object { "isRequired": true, "type": "node", }, - "onChange": Object { - "type": "func", + "shortcut": Object { + "type": "node", }, }, }, @@ -7711,6 +7719,20 @@ Map { }, }, }, + "unstable_ContextMenuSelectableItem" => Object { + "propTypes": Object { + "initialChecked": Object { + "type": "bool", + }, + "label": Object { + "isRequired": true, + "type": "node", + }, + "onChange": Object { + "type": "func", + }, + }, + }, "unstable_useContextMenu" => Object {}, } `; diff --git a/packages/react/src/__tests__/index-test.js b/packages/react/src/__tests__/index-test.js index 91b7322a50c0..90502b81019d 100644 --- a/packages/react/src/__tests__/index-test.js +++ b/packages/react/src/__tests__/index-test.js @@ -196,6 +196,7 @@ describe('Carbon Components React', () => { "UnorderedList", "unstable_ContextMenu", "unstable_ContextMenuDivider", + "unstable_ContextMenuGroup", "unstable_ContextMenuItem", "unstable_ContextMenuRadioGroup", "unstable_ContextMenuSelectableItem", diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 28d7012f729f..823144e0a8f7 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -10,10 +10,11 @@ import { action } from '@storybook/addon-actions'; import { InlineNotification } from '../Notification'; import ContextMenu, { - ContextMenuItem, ContextMenuDivider, - ContextMenuSelectableItem, + ContextMenuGroup, + ContextMenuItem, ContextMenuRadioGroup, + ContextMenuSelectableItem, useContextMenu, } from '../ContextMenu'; @@ -53,6 +54,7 @@ const Story = (items) => { {item.children && item.children.map(renderItem)} @@ -76,6 +78,12 @@ const Story = (items) => { onChange={action('onChange')} /> ); + case 'group': + return ( + + {item.children && item.children.map(renderItem)} + + ); } } @@ -105,7 +113,7 @@ export const _ContextMenu = () => { type: 'item', label: 'Cut', shortcut: '⌘X' }, { type: 'item', label: 'Copy', shortcut: '⌘C' }, { type: 'item', label: 'Copy path', shortcut: '⌥⌘C' }, - { type: 'item', label: 'Paste', shortcut: '⌘V' }, + { type: 'item', label: 'Paste', shortcut: '⌘V', disabled: true }, { type: 'item', label: 'Duplicate' }, { type: 'divider' }, { type: 'selectable', label: 'Publish', initialChecked: true }, @@ -117,8 +125,14 @@ _ContextMenu.storyName = 'ContextMenu'; export const _MultipleGroups = () => Story([ - { type: 'item', label: 'Bold' }, - { type: 'item', label: 'Italic' }, + { + type: 'group', + label: 'Font style', + children: [ + { type: 'selectable', label: 'Bold' }, + { type: 'selectable', label: 'Italic' }, + ], + }, { type: 'divider' }, { type: 'radiogroup', diff --git a/packages/react/src/components/ContextMenu/ContextMenu-test.js b/packages/react/src/components/ContextMenu/ContextMenu-test.js index 509a49345ee5..18ea146ab8ea 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-test.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-test.js @@ -106,17 +106,6 @@ describe('ContextMenu', () => { }); describe('radiogroup', () => { - it('receives the expected classes', () => { - const wrapper = mount( - - ); - const container = wrapper.childAt(0); - - expect(container.hasClass(`${prefix}--context-menu-radio-group`)).toBe( - true - ); - }); - it('children have role "menuitemradio"', () => { const wrapper = mount( diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index 83615c4e157e..8c1bc198d23d 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -22,9 +22,10 @@ import { getParentMenu, } from './_utils'; -import ContextMenuSelectableItem from './ContextMenuSelectableItem'; +import ContextMenuGroup from './ContextMenuGroup'; import ContextMenuRadioGroup from './ContextMenuRadioGroup'; import ContextMenuRadioGroupOptions from './ContextMenuRadioGroupOptions'; +import ContextMenuSelectableItem from './ContextMenuSelectableItem'; const { prefix } = settings; @@ -233,6 +234,7 @@ const ContextMenu = function ContextMenu({ role: 'menu', tabIndex: -1, 'data-direction': direction, + 'data-level': level, style: { left: `${position[0]}px`, top: `${position[1]}px`, @@ -241,9 +243,13 @@ const ContextMenu = function ContextMenu({ let childrenToRender = options; - // if the only child is a radio group, don't render radiogroup component, but - // only the children to prevent duplicate group markup - if (options.length === 1 && options[0].type === ContextMenuRadioGroup) { + // if the only child is a radiogroup, don't render it as radiogroup component, but + // only the items to prevent duplicate markup + if ( + options && + options.length === 1 && + options[0].type === ContextMenuRadioGroup + ) { const radioGroupProps = options[0].props; ulAttributes['aria-label'] = radioGroupProps.label; @@ -256,6 +262,15 @@ const ContextMenu = function ContextMenu({ ); } + // if the only child is a generic group, don't render it as group component, but + // only the children to prevent duplicate markup + if (options && options.length === 1 && options[0].type === ContextMenuGroup) { + const groupProps = options[0].props; + + ulAttributes['aria-label'] = groupProps.label; + childrenToRender = React.Children.toArray(options[0].props.children); + } + return (
        {childrenToRender}
      diff --git a/packages/react/src/components/ContextMenu/ContextMenuGroup.js b/packages/react/src/components/ContextMenu/ContextMenuGroup.js new file mode 100644 index 000000000000..5c31ba58d424 --- /dev/null +++ b/packages/react/src/components/ContextMenu/ContextMenuGroup.js @@ -0,0 +1,33 @@ +/** + * Copyright IBM Corp. 2020 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +import React from 'react'; +import PropTypes from 'prop-types'; + +function ContextMenuGroup({ label, children }) { + return ( +
    • +
        + {children} +
      +
      • + ); +} + +ContextMenuGroup.propTypes = { + /** + * Specify the children of the ContextMenuGroup + */ + children: PropTypes.node, + + /** + * Rendered label for the ContextMenuGroup + */ + label: PropTypes.node.isRequired, +}; + +export default ContextMenuGroup; diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index 366da57128de..ec0dd9b0bf67 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -84,7 +84,7 @@ function ContextMenuOption({ (match(event, keys.ArrowRight) || match(event, keys.Enter)) ) { openSubmenu(true); - } else if (match(event, keys.Enter)) { + } else if (match(event, keys.Enter) && onClick) { onClick(event); } } diff --git a/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js b/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js index 8f65d2634342..eae6164c2e60 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js +++ b/packages/react/src/components/ContextMenu/ContextMenuRadioGroup.js @@ -7,11 +7,9 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { settings } from 'carbon-components'; +import ContextMenuGroup from './ContextMenuGroup'; import ContextMenuRadioGroupOptions from './ContextMenuRadioGroupOptions'; -const { prefix } = settings; - function ContextMenuRadioGroup({ items, initialSelectedItem, @@ -19,18 +17,13 @@ function ContextMenuRadioGroup({ onChange = () => {}, }) { return ( -
    • -
        - -
      -
      • + + + ); } diff --git a/packages/react/src/components/ContextMenu/_utils.js b/packages/react/src/components/ContextMenu/_utils.js index cd60c97d56d2..45b087fdfa38 100644 --- a/packages/react/src/components/ContextMenu/_utils.js +++ b/packages/react/src/components/ContextMenu/_utils.js @@ -18,27 +18,25 @@ export function focusNode(node) { } export function getValidNodes(list) { - const nodes = Array.from(list?.childNodes ?? []).reduce((acc, child) => { - if (child.tagName === 'LI') { - return [...acc, child]; - } + const { level } = list.dataset; - if (child.classList.contains(`${prefix}--context-menu-radio-group`)) { - return [...acc, ...child.childNodes]; - } + let nodes = []; - return acc; - }, []); + if (level) { + const submenus = Array.from(list.querySelectorAll('[data-level]')); + nodes = Array.from( + list.querySelectorAll(`li.${prefix}--context-menu-option`) + ).filter((child) => !submenus.some((submenu) => submenu.contains(child))); + } return nodes.filter((node) => - node.matches( - `li.${prefix}--context-menu-option:not(.${prefix}--context-menu-option--disabled)` - ) + node.matches(`:not(.${prefix}--context-menu-option--disabled)`) ); } export function getNextNode(current, direction) { - const nodes = getValidNodes(current.parentNode); + const menu = getParentMenu(current); + const nodes = getValidNodes(menu); const currentIndex = nodes.indexOf(current); const nextNode = nodes[currentIndex + direction]; diff --git a/packages/react/src/components/ContextMenu/index.js b/packages/react/src/components/ContextMenu/index.js index 21e1189aeb4e..64d423735746 100644 --- a/packages/react/src/components/ContextMenu/index.js +++ b/packages/react/src/components/ContextMenu/index.js @@ -6,23 +6,26 @@ */ import ContextMenu from './ContextMenu'; -import ContextMenuItem from './ContextMenuItem'; import ContextMenuDivider from './ContextMenuDivider'; -import ContextMenuSelectableItem from './ContextMenuSelectableItem'; +import ContextMenuGroup from './ContextMenuGroup'; +import ContextMenuItem from './ContextMenuItem'; import ContextMenuRadioGroup from './ContextMenuRadioGroup'; +import ContextMenuSelectableItem from './ContextMenuSelectableItem'; -ContextMenu.ContextMenuItem = ContextMenuItem; ContextMenu.ContextMenuDivider = ContextMenuDivider; -ContextMenu.ContextMenuSelectableItem = ContextMenuSelectableItem; +ContextMenu.ContextMenuGroup = ContextMenuGroup; +ContextMenu.ContextMenuItem = ContextMenuItem; ContextMenu.ContextMenuRadioGroup = ContextMenuRadioGroup; +ContextMenu.ContextMenuSelectableItem = ContextMenuSelectableItem; import useContextMenu from './useContextMenu'; export { - ContextMenuItem, ContextMenuDivider, - ContextMenuSelectableItem, + ContextMenuGroup, + ContextMenuItem, ContextMenuRadioGroup, + ContextMenuSelectableItem, useContextMenu, }; export default ContextMenu; diff --git a/packages/react/src/index.js b/packages/react/src/index.js index 70dfe03e3ebc..e25e276d436b 100644 --- a/packages/react/src/index.js +++ b/packages/react/src/index.js @@ -208,9 +208,10 @@ export unstable_TreeView, { TreeNode as unstable_TreeNode, } from './components/TreeView'; export unstable_ContextMenu, { - ContextMenuItem as unstable_ContextMenuItem, ContextMenuDivider as unstable_ContextMenuDivider, - ContextMenuSelectableItem as unstable_ContextMenuSelectableItem, + ContextMenuGroup as unstable_ContextMenuGroup, + ContextMenuItem as unstable_ContextMenuItem, ContextMenuRadioGroup as unstable_ContextMenuRadioGroup, + ContextMenuSelectableItem as unstable_ContextMenuSelectableItem, useContextMenu as unstable_useContextMenu, } from './components/ContextMenu'; From a548761d4d6cc69709aca61432b2431c3e80dd2a Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Tue, 23 Feb 2021 15:32:30 +0100 Subject: [PATCH 52/54] fix(context-menu): support "space" to activate items --- packages/react/src/components/ContextMenu/ContextMenu.js | 5 ++++- .../src/components/ContextMenu/ContextMenuOption.js | 9 +++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu.js b/packages/react/src/components/ContextMenu/ContextMenu.js index 8c1bc198d23d..371a6604499b 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu.js +++ b/packages/react/src/components/ContextMenu/ContextMenu.js @@ -54,7 +54,10 @@ const ContextMenu = function ContextMenu({ } function handleKeyDown(event) { - if (event.target.tagName === 'LI' && match(event, keys.Enter)) { + if ( + event.target.tagName === 'LI' && + (match(event, keys.Enter) || match(event, keys.Space)) + ) { handleClick(event); } else { event.stopPropagation(); diff --git a/packages/react/src/components/ContextMenu/ContextMenuOption.js b/packages/react/src/components/ContextMenu/ContextMenuOption.js index ec0dd9b0bf67..6d5b2f0bf8e5 100644 --- a/packages/react/src/components/ContextMenu/ContextMenuOption.js +++ b/packages/react/src/components/ContextMenu/ContextMenuOption.js @@ -81,10 +81,15 @@ function ContextMenuOption({ function handleKeyDown(event) { if ( clickedElementHasSubnodes(event) && - (match(event, keys.ArrowRight) || match(event, keys.Enter)) + (match(event, keys.ArrowRight) || + match(event, keys.Enter) || + match(event, keys.Space)) ) { openSubmenu(true); - } else if (match(event, keys.Enter) && onClick) { + } else if ( + (match(event, keys.Enter) || match(event, keys.Space)) && + onClick + ) { onClick(event); } } From db10c795a9033164bb2e9cc3ef2dda6eb3394201 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Tue, 9 Mar 2021 13:19:48 +0100 Subject: [PATCH 53/54] chore: add context menu story to experimental group --- packages/react/src/components/ContextMenu/ContextMenu-story.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 823144e0a8f7..2572799d1f7d 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -19,7 +19,7 @@ import ContextMenu, { } from '../ContextMenu'; export default { - title: 'unstable_ContextMenu', + title: 'Experimental/unstable_ContextMenu', parameters: { component: ContextMenu, }, From 20b98383a43233f951cb4a7bfaa529817ec73b09 Mon Sep 17 00:00:00 2001 From: Jan Hassel Date: Tue, 9 Mar 2021 13:35:39 +0100 Subject: [PATCH 54/54] fix(context-menu): prevents error where user must click twice to open --- .../src/components/ContextMenu/ContextMenu-story.js | 9 ++++++--- .../react/src/components/ContextMenu/useContextMenu.js | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/react/src/components/ContextMenu/ContextMenu-story.js b/packages/react/src/components/ContextMenu/ContextMenu-story.js index 2572799d1f7d..910480d0b1b9 100644 --- a/packages/react/src/components/ContextMenu/ContextMenu-story.js +++ b/packages/react/src/components/ContextMenu/ContextMenu-story.js @@ -47,11 +47,12 @@ const InfoBanners = () => ( const Story = (items) => { const contextMenuProps = useContextMenu(); - function renderItem(item) { + function renderItem(item, i) { switch (item.type) { case 'item': return ( { ); case 'divider': - return ; + return ; case 'selectable': return ( { case 'radiogroup': return ( { ); case 'group': return ( - + {item.children && item.children.map(renderItem)} ); diff --git a/packages/react/src/components/ContextMenu/useContextMenu.js b/packages/react/src/components/ContextMenu/useContextMenu.js index efa1338403fd..5a06d59850b4 100644 --- a/packages/react/src/components/ContextMenu/useContextMenu.js +++ b/packages/react/src/components/ContextMenu/useContextMenu.js @@ -33,7 +33,7 @@ function useContextMenu(trigger = document) { trigger.removeEventListener('contextmenu', openContextMenu); }; } - }); + }, [trigger]); return { open,