flex menu item (#2075)

* sweet pt-flex-container mixin * icon/icons.scss => icon.scss, remove magic numbers * menu-item is a pt-flex-container row. margin between items cleans up styles. * render submenu caret icon in React * MenuItem overflow ellipsis fixes #984. add `multiline` prop to disable this behavior. * MenuExample demonstrates truncation * navigator uses flex columnm, adjust releases min-width * multiline styles, align icon to top * use Text component for ellipsize + native tooltip behavior
palantir · Feb 5, 2018 · d88e24a · d88e24a · blueprint-bot · Feb 5, 2018
1 parent bbb3da4
commit d88e24a
Show file tree

Hide file tree

Showing 11 changed files with 59 additions and 64 deletions.
diff --git a/packages/core/src/components/_index.scss b/packages/core/src/components/_index.scss
@@ -13,7 +13,7 @@
 @import "editable-text/editable-text";
 @import "forms/index";
 @import "hotkeys/hotkeys";
-@import "icon/icons";
+@import "icon/icon";
 @import "menu/menu";
 @import "navbar/navbar";
 @import "non-ideal-state/non-ideal-state";

diff --git a/...ages/core/src/components/icon/_icons.scss → packages/core/src/components/icon/_icon.scss b/...ages/core/src/components/icon/_icons.scss → packages/core/src/components/icon/_icon.scss
@@ -42,9 +42,6 @@ span.pt-icon {
 svg.pt-icon {
   // respect dimensions exactly
   flex: 0 0 auto;
-  // SVG and DOM elements don't align perfectly - this magic number seems to fix it.
-  // https://blog.prototypr.io/align-svg-icons-to-text-and-say-goodbye-to-font-icons-d44b3d7b26b4#6e0c
-  margin-top: -0.125em;
   vertical-align: middle;
   // inherit text color by default
   fill: currentColor;

diff --git a/packages/core/src/components/menu/_common.scss b/packages/core/src/components/menu/_common.scss
@@ -20,17 +20,23 @@ $dark-menu-item-color-active: $dark-minimal-button-background-color-active !defa
 // customize modifier classes with params.
 // setting modifier to "" will generally apply it as default styles due to & selectors
 @mixin menu-item($disabled-selector: ".pt-disabled", $hover-selector: ":hover") {
-  @include overflow-ellipsis();
-  display: block;
+  @include pt-flex-container(row, $menu-item-padding);
+  align-items: center;
   border-radius: $menu-item-border-radius;
   padding: $menu-item-padding;
+  text-decoration: none;
   line-height: $pt-icon-size-standard;
   color: inherit;
   user-select: none;
 
+  > .pt-fill {
+    word-break: break-word;
+  }
+
   &#{$hover-selector} {
     background-color: $menu-item-color-hover;
     cursor: pointer;
+    text-decoration: none;
   }
 
   &#{$disabled-selector} {

diff --git a/packages/core/src/components/menu/_menu.scss b/packages/core/src/components/menu/_menu.scss
@@ -55,21 +55,13 @@ Styleguide pt-menu
   &::before {
     // support pt-icon-* classes directly on this element
     @include pt-icon();
-    float: left;
     margin-right: $menu-item-padding;
   }
 
   &::before,
-  &::after {
-    color: $pt-icon-color;
-  }
-
   .pt-icon {
+    align-self: flex-start;
     color: $pt-icon-color;
-
-    &:first-child {
-      margin-right: $menu-item-padding;
-    }
   }
 
   .pt-menu-item-label {
@@ -95,7 +87,6 @@ Styleguide pt-menu
     color: $pt-text-color-disabled !important;
 
     &::before,
-    &::after,
     .pt-icon,
     .pt-menu-item-label {
       color: $pt-icon-color-disabled !important;
@@ -113,13 +104,6 @@ Styleguide pt-menu
   }
 }
 
-a.pt-menu-item {
-  &,
-  &:hover {
-    text-decoration: none;
-  }
-}
-
 button.pt-menu-item {
   border: none;
   background: none;
@@ -128,11 +112,6 @@ button.pt-menu-item {
   font-family: inherit;
 }
 
-.pt-menu-item-label {
-  float: right;
-  margin-left: $menu-item-padding;
-}
-
 /*
 Menu headers
 
@@ -170,7 +149,6 @@ Styleguide pt-menu.pt-menu-header
     @include menu-item-intent($pt-dark-intent-text-colors);
 
     &::before,
-    &::after,
     .pt-icon {
       color: $pt-dark-icon-color;
     }
@@ -179,13 +157,6 @@ Styleguide pt-menu.pt-menu-header
       color: $pt-dark-text-color-muted;
     }
 
-    &:hover {
-      &::before,
-      &::after {
-        color: $white;
-      }
-    }
-
     &.pt-active,
     &:active {
       background-color: $dark-menu-item-color-active;
@@ -197,7 +168,6 @@ Styleguide pt-menu.pt-menu-header
       color: $pt-dark-text-color-disabled !important;
 
       &::before,
-      &::after,
       .pt-icon,
       .pt-menu-item-label {
         color: $pt-dark-icon-color-disabled !important;

diff --git a/packages/core/src/components/menu/_submenu.scss b/packages/core/src/components/menu/_submenu.scss
@@ -14,24 +14,6 @@
       // override very specific selector for popovers in button groups
       // stylelint-disable-next-line declaration-no-important
       float: none !important;
-
-      > .pt-menu-item {
-        padding-right: $pt-icon-size-standard + $pt-grid-size;
-
-        &::after {
-          @include pt-icon();
-          position: absolute;
-          right: $half-grid-size;
-          content: $pt-icon-caret-right;
-
-          // need several layers of selectors to make sure we're targeting only
-          // the top level of menu items in the current submenu.
-          // stylelint-disable-next-line
-          .pt-large & {
-            line-height: $pt-icon-size-large;
-          }
-        }
-      }
     }
 
     > .pt-popover-open > .pt-menu-item {

diff --git a/packages/core/src/components/menu/menuItem.tsx b/packages/core/src/components/menu/menuItem.tsx
@@ -14,6 +14,7 @@ import { Position } from "../../common/position";
 import { IActionProps, ILinkProps } from "../../common/props";
 import { Icon } from "../icon/icon";
 import { IPopoverProps, Popover, PopoverInteractionKind } from "../popover/popover";
+import { Text } from "../text/text";
 import { Menu } from "./menu";
 
 export interface IMenuItemProps extends IActionProps, ILinkProps {
@@ -26,6 +27,13 @@ export interface IMenuItemProps extends IActionProps, ILinkProps {
      */
     label?: string | JSX.Element;
 
+    /**
+     * Whether the text should be allowed to wrap to multiple lines.
+     * If `false`, text will be truncated with an ellipsis when it reaches `max-width`.
+     * @default false
+     */
+    multiline?: boolean;
+
     /** Props to spread to `Popover`. Note that `content` and `minimal` cannot be changed. */
     popoverProps?: Partial<IPopoverProps>;
 
@@ -45,6 +53,7 @@ export interface IMenuItemProps extends IActionProps, ILinkProps {
 export class MenuItem extends AbstractPureComponent<IMenuItemProps> {
     public static defaultProps: IMenuItemProps = {
         disabled: false,
+        multiline: false,
         popoverProps: {},
         shouldDismissPopover: true,
         text: "",
@@ -76,8 +85,11 @@ export class MenuItem extends AbstractPureComponent<IMenuItemProps> {
                 target={this.props.target}
             >
                 <Icon iconName={this.props.iconName} />
+                <Text className={Classes.FILL} ellipsize={!this.props.multiline}>
+                    {this.props.text}
+                </Text>
                 {label && <span className={Classes.MENU_ITEM_LABEL}>{label}</span>}
-                {this.props.text}
+                {hasSubmenu && <Icon iconName="caret-right" />}
             </a>
         );
 

diff --git a/packages/core/test/menu/menuTests.tsx b/packages/core/test/menu/menuTests.tsx
@@ -5,7 +5,7 @@
  */
 
 import { assert } from "chai";
-import { mount, shallow, ShallowWrapper } from "enzyme";
+import { mount, ReactWrapper, shallow, ShallowWrapper } from "enzyme";
 import * as React from "react";
 import { spy, stub } from "sinon";
 
@@ -22,13 +22,14 @@ import {
     MenuItem,
     Popover,
     PopoverInteractionKind,
+    Text,
 } from "../../src/index";
 
 describe("MenuItem", () => {
     it("React renders MenuItem", () => {
         const wrapper = shallow(<MenuItem iconName="graph" text="Graph" />);
-        assert.lengthOf(wrapper.find(Icon), 1);
-        assert.match(wrapper.text(), /Graph$/);
+        assert.isTrue(wrapper.find(Icon).exists());
+        assert.strictEqual(findText(wrapper).text(), "Graph");
     });
 
     it("children appear in submenu", () => {
@@ -133,6 +134,19 @@ describe("MenuItem", () => {
         );
         assert.notStrictEqual(wrapper.find(Popover).prop("content"), popoverProps.content);
     });
+
+    it("multiline prop determines if long content is ellipsized", () => {
+        const wrapper = mount(
+            <MenuItem multiline={false} text="multiline prop determines if long content is ellipsized." />,
+        );
+        function assertOverflow(expected: boolean) {
+            assert.strictEqual(findText(wrapper).hasClass(Classes.TEXT_OVERFLOW_ELLIPSIS), expected);
+        }
+
+        assertOverflow(true);
+        wrapper.setProps({ multiline: true });
+        assertOverflow(false);
+    });
 });
 
 describe("MenuDivider", () => {
@@ -166,3 +180,7 @@ function findSubmenu(wrapper: ShallowWrapper<any, any>) {
         IMenuProps & { children: Array<React.ReactElement<IMenuItemProps>> }
     >;
 }
+
+function findText(wrapper: ShallowWrapper | ReactWrapper) {
+    return wrapper.find(Text).children();
+}
diff --git a/packages/docs-app/src/examples/core-examples/menuExample.tsx b/packages/docs-app/src/examples/core-examples/menuExample.tsx
@@ -10,6 +10,7 @@ import { Classes, Icon, Menu, MenuDivider, MenuItem } from "@blueprintjs/core";
 import { BaseExample } from "@blueprintjs/docs-theme";
 
 export class MenuExample extends BaseExample<{}> {
+    public className = "docs-menu-example";
     protected renderExample() {
         return (
             <div>
@@ -39,8 +40,8 @@ export class MenuExample extends BaseExample<{}> {
                     </MenuItem>
                     <MenuItem iconName="asterisk" text="Miscellaneous">
                         <MenuItem iconName="badge" text="Badge" />
-                        <MenuItem iconName="book" text="Book" />
-                        <MenuItem iconName="more" text="More">
+                        <MenuItem iconName="book" text="Long items will truncate when they reach max-width" />
+                        <MenuItem iconName="more" text="Look in here for even more items">
                             <MenuItem iconName="briefcase" text="Briefcase" />
                             <MenuItem iconName="calculator" text="Calculator" />
                             <MenuItem iconName="dollar" text="Dollar" />

diff --git a/packages/docs-app/src/index.scss b/packages/docs-app/src/index.scss
@@ -29,5 +29,5 @@
 }
 
 .docs-releases-menu .pt-menu {
-  width: 320px;
+  min-width: 270px;
 }
diff --git a/packages/docs-app/src/styles/_examples.scss b/packages/docs-app/src/styles/_examples.scss
@@ -346,6 +346,10 @@
   justify-content: center;
 }
 
+.docs-menu-example .pt-menu {
+  max-width: 280px;
+}
+
 .docs-wiggle {
   animation: docs-wiggle-rotate $pt-transition-duration $pt-transition-ease infinite;
 }

diff --git a/packages/docs-theme/src/styles/_navigator.scss b/packages/docs-theme/src/styles/_navigator.scss
@@ -15,8 +15,13 @@ $navigator-min-width: $pt-grid-size * 22;
     overflow: auto;
   }
 
-  .pt-menu-item:not(.pt-active) {
-    background: inherit;
+  .pt-menu-item {
+    flex-direction: column;
+    align-items: flex-start;
+
+    &:not(.pt-active) {
+      background: inherit;
+    }
   }
 }