Repository: uber-web/baseui
Pull request #689 (2018-12-21 02:00:22)

feat(menu): implements child menus on hover
<img width="678" alt="screen shot 2018-12-20 at 5 57 22 pm" src="https://user-images.githubusercontent.com/5317799/50320339-b9bce480-0480-11e9-99ba-b803d31395c8.png">

unsure about this api. feels a bit weird to provide an override format rather than nesting at the jsx level.  compare this to a similar goal in [blueprint](https://blueprintjs.com/docs/#core/components/menu)

maybe we keep this component as a menu for select and other dropdown menus, and provide a separate component for navigation menus?

chasestarr: I searched for examples of other component libraries enabling keyboard nav in child menus, but could not find. are we aware of any? It will be an interesting challenge to enable it. 
nadiia: There is some description of how submenu works with keyboard navigation [here](https://www.w3.org/TR/wai-aria-practices/examples/menubar/menubar-1/menubar-1.html#) but their example doesn't work as expected.

Patch:
diff --git a/src/menu/examples-list.js b/src/menu/examples-list.js
index daa3639c1..09d1afc66 100644
--- a/src/menu/examples-list.js
+++ b/src/menu/examples-list.js
@@ -13,4 +13,5 @@ module.exports = {
   MENU_PROFILE: 'Stateless Menu Profile',
   MENU_STATEFUL: 'Stateful Menu with Keybindings',
   MENU_PROFILE_STATEFUL: 'Stateful Menu Profile with Keybindings',
+  CHILD_MENU: 'Child Menu Hover',
 };
diff --git a/src/menu/examples.js b/src/menu/examples.js
index 2b8ecb178..944cd8ea1 100644
--- a/src/menu/examples.js
+++ b/src/menu/examples.js
@@ -184,4 +184,47 @@ export default {
       />
     );
   },
+
+  [examples.CHILD_MENU]: function ChildMenuStory() {
+    const SecondMenu = () => (
+      <Menu
+        items={ITEMS}
+        overrides={{
+          List: {style: {width: '200px'}},
+          Option: {props: {getChildMenu: ThirdMenu}},
+        }}
+        rootRef={React.createRef()}
+      />
+    );
+
+    const ThirdMenu = () => (
+      <Menu
+        items={ITEMS}
+        overrides={{List: {style: {width: '200px'}}}}
+        rootRef={React.createRef()}
+      />
+    );
+
+    return (
+      <StatefulMenu
+        items={PROFILE_ITEMS}
+        overrides={{
+          List: {style: {width: '350px', overflow: 'auto'}},
+          Option: {
+            component: OptionProfile,
+            props: {
+              getProfileItemLabels: ({title, subtitle, body}) => ({
+                title,
+                subtitle,
+                body,
+              }),
+              getProfileItemImg: item => item.imgUrl,
+              getProfileItemImgText: item => item.title,
+              getChildMenu: SecondMenu,
+            },
+          },
+        }}
+      />
+    );
+  },
 };
diff --git a/src/menu/maybe-child-menu.js b/src/menu/maybe-child-menu.js
new file mode 100644
index 000000000..b0770f50e
--- /dev/null
+++ b/src/menu/maybe-child-menu.js
@@ -0,0 +1,37 @@
+/*
+Copyright (c) 2018 Uber Technologies, Inc.
+
+This source code is licensed under the MIT license found in the
+LICENSE file in the root directory of this source tree.
+*/
+
+// @flow
+
+import * as React from 'react';
+
+import {StatefulPopover} from '../popover/index.js';
+
+type PropsT = {
+  children: React.Node,
+  getChildMenu: ?(item: *) => React.Node,
+  item: *,
+};
+
+export default function MaybeChildMenu(props: PropsT) {
+  if (!props.getChildMenu) {
+    return props.children;
+  }
+
+  return (
+    <StatefulPopover
+      content={props.getChildMenu(props.item)}
+      ignoreBoundary
+      onMouseEnterDelay={100}
+      onMouseLeaveDelay={100}
+      placement="rightTop"
+      triggerType="hover"
+    >
+      {props.children}
+    </StatefulPopover>
+  );
+}
diff --git a/src/menu/menu.js b/src/menu/menu.js
index ec37f12bd..2a62a9f0a 100644
--- a/src/menu/menu.js
+++ b/src/menu/menu.js
@@ -15,10 +15,10 @@ import {getOverrides} from '../helpers/overrides.js';
 import type {StatelessMenuPropsT} from './types.js';
 
 export default function Menu({
-  items,
   getRequiredItemProps = (item, index) => ({}),
-  rootRef = React.createRef(),
+  items,
   overrides = {},
+  rootRef = React.createRef(),
 }: StatelessMenuPropsT) {
   const [List, listProps] = getOverrides(overrides.List, StyledList);
   const [Option, optionProps] = getOverrides(overrides.Option, OptionList);
@@ -28,11 +28,10 @@ export default function Menu({
         const requiredProps = getRequiredItemProps(item, index);
         return (
           <Option
-            key={index}
             item={item}
+            key={index}
             role="option"
-            // Allows tab focus into first element
-            tabIndex={index === 0 ? 0 : -1}
+            tabIndex={index === 0 ? 0 : -1} // Allows tab focus into first element
             {...getSharedProps(requiredProps)}
             {...optionProps}
           />
diff --git a/src/menu/option-list.js b/src/menu/option-list.js
index f0fe01383..7d6fe0920 100644
--- a/src/menu/option-list.js
+++ b/src/menu/option-list.js
@@ -4,18 +4,20 @@ Copyright (c) 2018 Uber Technologies, Inc.
 This source code is licensed under the MIT license found in the
 LICENSE file in the root directory of this source tree.
 */
-
 // @flow
+
 import * as React from 'react';
-// Components
-import {ListItem as StyledListItem} from './styled-components.js';
-import {OPTION_LIST_SIZE} from './constants.js';
+
 import {getOverrides} from '../helpers/overrides.js';
-// Types
+
+import {OPTION_LIST_SIZE} from './constants.js';
+import MaybeChildMenu from './maybe-child-menu.js';
+import {ListItem as StyledListItem} from './styled-components.js';
 import type {OptionListPropsT} from './types.js';
 
 export default function OptionList({
   item,
+  getChildMenu,
   getItemLabel,
   size,
   overrides,
@@ -30,10 +32,13 @@ export default function OptionList({
     $size: size,
     $isHighlighted,
   };
+
   return (
-    <ListItem {...sharedProps} {...restProps} {...listItemProps}>
-      {getItemLabel({isHighlighted: $isHighlighted, ...item})}
-    </ListItem>
+    <MaybeChildMenu getChildMenu={getChildMenu} item={item}>
+      <ListItem {...sharedProps} {...restProps} {...listItemProps}>
+        {getItemLabel({isHighlighted: $isHighlighted, ...item})}
+      </ListItem>
+    </MaybeChildMenu>
   );
 }
 
diff --git a/src/menu/option-profile.js b/src/menu/option-profile.js
index f7fff0899..44ee2654c 100644
--- a/src/menu/option-profile.js
+++ b/src/menu/option-profile.js
@@ -8,6 +8,7 @@ LICENSE file in the root directory of this source tree.
 // @flow
 import * as React from 'react';
 // Components
+import MaybeChildMenu from './maybe-child-menu.js';
 import {
   ListItemProfile as StyledListItemProfile,
   ProfileImgContainer as StyledProfileImgContainer,
@@ -23,6 +24,7 @@ import type {OptionProfilePropsT} from './types.js';
 
 export default function OptionProfile({
   item,
+  getChildMenu,
   getProfileItemLabels,
   getProfileItemImg,
   getProfileItemImgText,
@@ -62,30 +64,32 @@ export default function OptionProfile({
   const {title, subtitle, body} = getProfileItemLabels(item);
 
   return (
-    <ListItemProfile {...restProps} {...listItemProfileProps}>
-      <ProfileImgContainer {...profileImgContainerProps}>
-        {ItemImg &&
-          (typeof ItemImg === 'string' ? (
-            // Render img src string wrapped with image component
-            <ProfileImg
-              src={ItemImg}
-              alt={getProfileItemImgText(item)}
-              {...profileImgProps}
-            />
-          ) : (
-            // Or just render the entire component user specified
-            <ItemImg {...profileImgProps} />
-          ))}
-      </ProfileImgContainer>
-      <ProfileLabelsContainer {...profileLabelsContainerProps}>
-        {title && <ProfileTitle {...profileTitleProps}>{title}</ProfileTitle>}
-        {subtitle && (
-          <ProfileSubtitle {...profileSubtitleProps}>
-            {subtitle}
-          </ProfileSubtitle>
-        )}
-        {body && <ProfileBody {...profileBodyProps}>{body}</ProfileBody>}
-      </ProfileLabelsContainer>
-    </ListItemProfile>
+    <MaybeChildMenu getChildMenu={getChildMenu} item={item}>
+      <ListItemProfile {...restProps} {...listItemProfileProps}>
+        <ProfileImgContainer {...profileImgContainerProps}>
+          {ItemImg &&
+            (typeof ItemImg === 'string' ? (
+              // Render img src string wrapped with image component
+              <ProfileImg
+                src={ItemImg}
+                alt={getProfileItemImgText(item)}
+                {...profileImgProps}
+              />
+            ) : (
+              // Or just render the entire component user specified
+              <ItemImg {...profileImgProps} />
+            ))}
+        </ProfileImgContainer>
+        <ProfileLabelsContainer {...profileLabelsContainerProps}>
+          {title && <ProfileTitle {...profileTitleProps}>{title}</ProfileTitle>}
+          {subtitle && (
+            <ProfileSubtitle {...profileSubtitleProps}>
+              {subtitle}
+            </ProfileSubtitle>
+          )}
+          {body && <ProfileBody {...profileBodyProps}>{body}</ProfileBody>}
+        </ProfileLabelsContainer>
+      </ListItemProfile>
+    </MaybeChildMenu>
   );
 }
diff --git a/src/menu/stateful-container.js b/src/menu/stateful-container.js
index ce3f7bdce..6b68411f2 100644
--- a/src/menu/stateful-container.js
+++ b/src/menu/stateful-container.js
@@ -150,10 +150,10 @@ export default class MenuStatefulContainer extends React.Component<
     const {children, items} = this.props;
     return children(
       ({
+        getRequiredItemProps: this.getRequiredItemProps,
         highlightedIndex,
         items,
         rootRef: this.rootRef,
-        getRequiredItemProps: this.getRequiredItemProps,
       }: RenderPropsT),
     );
   }
diff --git a/src/menu/styled-components.js b/src/menu/styled-components.js
index 33053565e..c22b97c72 100644
--- a/src/menu/styled-components.js
+++ b/src/menu/styled-components.js
@@ -22,14 +22,13 @@ export const List = styled('ul', ({$theme}: StyledPropsT) => ({
   marginBottom: '0',
   marginLeft: '0',
   marginRight: '0',
-  paddingTop: '0',
-  paddingBottom: '0',
+  paddingTop: $theme.sizing.scale300,
+  paddingBottom: $theme.sizing.scale300,
   paddingLeft: '0',
   paddingRight: '0',
   backgroundColor: $theme.colors.backgroundAlt,
   borderRadius: $theme.borders.radius300,
   boxShadow: $theme.lighting.shadow600,
-  overflow: 'auto',
 }));
 List.displayName = 'StyledList';
 
@@ -68,12 +67,6 @@ export const ListItem = styled(
       $size === OPTION_LIST_SIZE.compact
         ? $theme.sizing.scale900
         : $theme.sizing.scale600,
-    ':first-child': {
-      marginTop: $theme.sizing.scale300,
-    },
-    ':last-child': {
-      marginBottom: $theme.sizing.scale300,
-    },
   }),
 );
 ListItem.displayName = 'StyledListItem';
@@ -93,12 +86,6 @@ export const ListItemProfile = styled('li', ({$theme}: StyledPropsT) => ({
   ':hover': {
     backgroundColor: $theme.colors.menuFillHover,
   },
-  ':first-child': {
-    marginTop: $theme.sizing.scale300,
-  },
-  ':last-child': {
-    marginBottom: $theme.sizing.scale300,
-  },
 }));
 ListItemProfile.displayName = 'StyledListItemProfile';
 
diff --git a/src/menu/types.js b/src/menu/types.js
index e140d05b4..9e282aee2 100644
--- a/src/menu/types.js
+++ b/src/menu/types.js
@@ -13,7 +13,7 @@ import {STATE_CHANGE_TYPES, OPTION_LIST_SIZE} from './constants.js';
 export type ItemT = *;
 export type ItemsT = $ReadOnlyArray<ItemT>;
 
-export type GetItemLabelFnT = (item: ItemT) => React$Node;
+export type GetItemLabelFnT = (item: ItemT) => React.Node;
 
 export type GetProfileItemLabelsFnT = (
   item: ItemT,
@@ -110,10 +110,12 @@ export type MenuProfilePropsT = {
 };
 
 export type SharedStatelessPropsT = {
-  items: ItemsT,
-  rootRef: RootRefT,
   getRequiredItemProps?: GetRequiredItemPropsFnT,
   highlightedIndex?: number,
+  items: ItemsT,
+  onBlur?: (event: SyntheticFocusEvent<HTMLElement>) => mixed,
+  onFocus?: (event: SyntheticFocusEvent<HTMLElement>) => mixed,
+  rootRef: RootRefT,
 };
 
 export type StatefulMenuPropsT = StatefulContainerPropsT & MenuPropsT;
@@ -129,6 +131,7 @@ export type StatelessMenuProfilePropsT = SharedStatelessPropsT &
 export type OptionListPropsT = {
   item: ItemT,
   getItemLabel: GetItemLabelFnT,
+  getChildMenu?: (item: ItemT) => React.Node,
   size: $Keys<typeof OPTION_LIST_SIZE>,
   overrides: {
     ListItem?: OverrideT<*>,
@@ -138,6 +141,7 @@ export type OptionListPropsT = {
 
 export type OptionProfilePropsT = {
   item: ItemT,
+  getChildMenu?: (item: ItemT) => React.Node,
   getProfileItemLabels: GetProfileItemLabelsFnT,
   getProfileItemImg: GetProfileItemImgFnT,
   getProfileItemImgText: GetProfileItemImgTextFnT,
diff --git a/src/popover/default-props.js b/src/popover/default-props.js
index 3f9fc421e..d7cc05229 100644
--- a/src/popover/default-props.js
+++ b/src/popover/default-props.js
@@ -10,6 +10,7 @@ import type {BasePopoverPropsT} from './types.js';
 
 const baseDefaultProps: $Shape<BasePopoverPropsT> = {
   accessibilityType: ACCESSIBILITY_TYPE.menu,
+  ignoreBoundary: false,
   overrides: {},
   onMouseEnterDelay: 200,
   onMouseLeaveDelay: 200,
diff --git a/src/popover/popover.js b/src/popover/popover.js
index a4e473100..06a0d4822 100644
--- a/src/popover/popover.js
+++ b/src/popover/popover.js
@@ -141,6 +141,7 @@ class Popover extends React.Component<PopoverPropsT, PopoverPrivateStateT> {
           fn: this.onPopperUpdate,
           order: 900,
         },
+        preventOverflow: {enabled: !this.props.ignoreBoundary},
       },
     });
   }
diff --git a/src/popover/stateful-container.js b/src/popover/stateful-container.js
index 150b20f7a..b77810625 100644
--- a/src/popover/stateful-container.js
+++ b/src/popover/stateful-container.js
@@ -120,6 +120,7 @@ class StatefulContainer extends React.Component<
       accessibilityType,
       dismissOnClickOutside,
       dismissOnEsc,
+      ignoreBoundary,
       overrides,
       onMouseEnterDelay,
       onMouseLeaveDelay,
@@ -130,6 +131,7 @@ class StatefulContainer extends React.Component<
 
     const popoverProps: PopoverPropsWithoutChildrenT = {
       accessibilityType,
+      ignoreBoundary,
       isOpen: this.state.isOpen,
       overrides,
       content: this.renderContent,
diff --git a/src/popover/types.js b/src/popover/types.js
index facfcf16e..73d384500 100644
--- a/src/popover/types.js
+++ b/src/popover/types.js
@@ -54,6 +54,7 @@ export type ChildrenT = React.ChildrenArray<ChildT>;
 export type BasePopoverPropsT = {
   accessibilityType?: AccessibilityTypeT,
   id?: string,
+  ignoreBoundary?: boolean,
   onMouseEnterDelay?: number,
   onMouseLeaveDelay?: number,
   overrides?: OverridesT,


Test patch:
diff --git a/src/menu/__tests__/__snapshots__/styled-components.test.js.snap b/src/menu/__tests__/__snapshots__/styled-components.test.js.snap
index 9fa776eb4..0bfcad1e9 100644
--- a/src/menu/__tests__/__snapshots__/styled-components.test.js.snap
+++ b/src/menu/__tests__/__snapshots__/styled-components.test.js.snap
@@ -9,26 +9,19 @@ Object {
   "marginLeft": "0",
   "marginRight": "0",
   "marginTop": "0",
-  "overflow": "auto",
-  "paddingBottom": "0",
+  "paddingBottom": "$theme.sizing.scale300",
   "paddingLeft": "0",
   "paddingRight": "0",
-  "paddingTop": "0",
+  "paddingTop": "$theme.sizing.scale300",
   "position": "relative",
 }
 `;
 
 exports[`Menu Styled Components ListItem - basic render: correct styles 1`] = `
 Object {
-  ":first-child": Object {
-    "marginTop": "$theme.sizing.scale300",
-  },
   ":hover": Object {
     "backgroundColor": "$theme.colors.menuFillHover",
   },
-  ":last-child": Object {
-    "marginBottom": "$theme.sizing.scale300",
-  },
   "backgroundColor": "transparent",
   "color": "$theme.colors.foreground",
   "cursor": "pointer",
@@ -50,15 +43,9 @@ Object {
 
 exports[`Menu Styled Components ListItem - compact: correct styles 1`] = `
 Object {
-  ":first-child": Object {
-    "marginTop": "$theme.sizing.scale300",
-  },
   ":hover": Object {
     "backgroundColor": "$theme.colors.menuFillHover",
   },
-  ":last-child": Object {
-    "marginBottom": "$theme.sizing.scale300",
-  },
   "backgroundColor": "transparent",
   "color": "$theme.colors.foreground",
   "cursor": "pointer",
@@ -80,15 +67,9 @@ Object {
 
 exports[`Menu Styled Components ListItem - highlighted render: correct styles 1`] = `
 Object {
-  ":first-child": Object {
-    "marginTop": "$theme.sizing.scale300",
-  },
   ":hover": Object {
     "backgroundColor": "$theme.colors.menuFillHover",
   },
-  ":last-child": Object {
-    "marginBottom": "$theme.sizing.scale300",
-  },
   "backgroundColor": "$theme.colors.menuFillHover",
   "color": "$theme.colors.primary",
   "cursor": "pointer",
@@ -110,15 +91,9 @@ Object {
 
 exports[`Menu Styled Components ListItemProfile - basic render: correct styles 1`] = `
 Object {
-  ":first-child": Object {
-    "marginTop": "$theme.sizing.scale300",
-  },
   ":hover": Object {
     "backgroundColor": "$theme.colors.menuFillHover",
   },
-  ":last-child": Object {
-    "marginBottom": "$theme.sizing.scale300",
-  },
   "alignItems": "center",
   "cursor": "pointer",
   "display": "flex",
diff --git a/src/menu/__tests__/maybe-child-menu.test.js b/src/menu/__tests__/maybe-child-menu.test.js
new file mode 100644
index 000000000..1c63b9b54
--- /dev/null
+++ b/src/menu/__tests__/maybe-child-menu.test.js
@@ -0,0 +1,46 @@
+/*
+Copyright (c) 2018 Uber Technologies, Inc.
+
+This source code is licensed under the MIT license found in the
+LICENSE file in the root directory of this source tree.
+*/
+// @flow
+
+import React from 'react';
+import {mount} from 'enzyme';
+
+import MaybeChildMenu from '../maybe-child-menu.js';
+
+describe('MaybeChildMenu', () => {
+  it('does not render popover if getChildMenu is undefined', () => {
+    const wrapper = mount(
+      <MaybeChildMenu getChildMenu={null} item={{label: 'item'}}>
+        <div>child</div>
+      </MaybeChildMenu>,
+    );
+
+    expect(
+      wrapper
+        .children()
+        .first()
+        .name(),
+    ).toBe('div');
+  });
+  it('renders popover if getChildMenu is provided', () => {
+    const wrapper = mount(
+      <MaybeChildMenu
+        getChildMenu={() => <button>child menu</button>}
+        item={{label: 'item'}}
+      >
+        <div>child</div>
+      </MaybeChildMenu>,
+    );
+
+    expect(
+      wrapper
+        .children()
+        .first()
+        .name(),
+    ).toBe('StatefulPopover');
+  });
+});
diff --git a/src/popover/__tests__/__snapshots__/stateful-container.test.js.snap b/src/popover/__tests__/__snapshots__/stateful-container.test.js.snap
index 2f8c247e5..558d18b82 100644
--- a/src/popover/__tests__/__snapshots__/stateful-container.test.js.snap
+++ b/src/popover/__tests__/__snapshots__/stateful-container.test.js.snap
@@ -5,6 +5,7 @@ Array [
   Object {
     "accessibilityType": "menu",
     "content": [Function],
+    "ignoreBoundary": false,
     "isOpen": true,
     "onBlur": [Function],
     "onClickOutside": [Function],
diff --git a/src/popover/__tests__/__snapshots__/stateful-popover.test.js.snap b/src/popover/__tests__/__snapshots__/stateful-popover.test.js.snap
index 933ab71ca..b81cbc157 100644
--- a/src/popover/__tests__/__snapshots__/stateful-popover.test.js.snap
+++ b/src/popover/__tests__/__snapshots__/stateful-popover.test.js.snap
@@ -4,6 +4,7 @@ exports[`StatefulPopover basic render: renders <Popover/> as child to container
 <Popover
   accessibilityType="menu"
   content={[Function]}
+  ignoreBoundary={false}
   isOpen={true}
   onBlur={[Function]}
   onClickOutside={[Function]}
@@ -34,6 +35,7 @@ exports[`StatefulPopover basic render: renders <StatefulContainer/> 1`] = `
   content={[MockFunction]}
   dismissOnClickOutside={true}
   dismissOnEsc={true}
+  ignoreBoundary={false}
   initialState={
     Object {
       "isOpen": true,
diff --git a/src/tooltip/__tests__/__snapshots__/stateful-tooltip.test.js.snap b/src/tooltip/__tests__/__snapshots__/stateful-tooltip.test.js.snap
index 8553d92f3..8909fdf62 100644
--- a/src/tooltip/__tests__/__snapshots__/stateful-tooltip.test.js.snap
+++ b/src/tooltip/__tests__/__snapshots__/stateful-tooltip.test.js.snap
@@ -29,6 +29,7 @@ exports[`StatefulTooltip basic render: renders <Tooltip/> 1`] = `
 <Tooltip
   accessibilityType="tooltip"
   content={[Function]}
+  ignoreBoundary={false}
   isOpen={true}
   onBlur={[Function]}
   onClickOutside={[Function]}
diff --git a/src/tooltip/__tests__/stateful-tooltip-container.test.js b/src/tooltip/__tests__/stateful-tooltip-container.test.js
index 76a03433f..d88997d12 100644
--- a/src/tooltip/__tests__/stateful-tooltip-container.test.js
+++ b/src/tooltip/__tests__/stateful-tooltip-container.test.js
@@ -47,6 +47,7 @@ describe('StatefulTooltipContainer', () => {
       children,
       dismissOnClickOutside: true,
       dismissOnEsc: true,
+      ignoreBoundary: false,
     });
   });
 });
diff --git a/src/tooltip/__tests__/stateful-tooltip.test.js b/src/tooltip/__tests__/stateful-tooltip.test.js
index 601ed38d0..9f7681c3e 100644
--- a/src/tooltip/__tests__/stateful-tooltip.test.js
+++ b/src/tooltip/__tests__/stateful-tooltip.test.js
@@ -58,6 +58,7 @@ describe('StatefulTooltip', () => {
       ...props,
       dismissOnClickOutside: true,
       dismissOnEsc: true,
+      ignoreBoundary: false,
     });
 
     // But ultimately render a tooltip
diff --git a/src/tooltip/__tests__/tooltip.test.js b/src/tooltip/__tests__/tooltip.test.js
index d9e0c7a7d..53b35bab9 100644
--- a/src/tooltip/__tests__/tooltip.test.js
+++ b/src/tooltip/__tests__/tooltip.test.js
@@ -54,6 +54,7 @@ describe('Tooltip', () => {
       ...baseDefaultProps,
       ...props,
       children: button,
+      ignoreBoundary: false,
       overrides: {
         Arrow: {
           component: StyledArrow,