From d88e24a861f4a805faf1b838fdc59c3bb016a2d3 Mon Sep 17 00:00:00 2001 From: Gilad Gray Date: Mon, 5 Feb 2018 14:38:18 -0800 Subject: [PATCH] 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 --- packages/core/src/components/_index.scss | 2 +- .../icon/{_icons.scss => _icon.scss} | 3 -- .../core/src/components/menu/_common.scss | 10 ++++-- packages/core/src/components/menu/_menu.scss | 32 +------------------ .../core/src/components/menu/_submenu.scss | 18 ----------- .../core/src/components/menu/menuItem.tsx | 14 +++++++- packages/core/test/menu/menuTests.tsx | 24 ++++++++++++-- .../examples/core-examples/menuExample.tsx | 5 +-- packages/docs-app/src/index.scss | 2 +- packages/docs-app/src/styles/_examples.scss | 4 +++ .../docs-theme/src/styles/_navigator.scss | 9 ++++-- 11 files changed, 59 insertions(+), 64 deletions(-) rename packages/core/src/components/icon/{_icons.scss => _icon.scss} (81%) diff --git a/packages/core/src/components/_index.scss b/packages/core/src/components/_index.scss index da5821f7e9..ca8fd02692 100644 --- 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/packages/core/src/components/icon/_icons.scss b/packages/core/src/components/icon/_icon.scss similarity index 81% rename from packages/core/src/components/icon/_icons.scss rename to packages/core/src/components/icon/_icon.scss index 3752aeb5d4..cfe2381304 100644 --- a/packages/core/src/components/icon/_icons.scss +++ b/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 index b6df6eb726..1c4fcc3aaa 100644 --- 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 index e2a898ef62..9e61b6a2c8 100644 --- 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 index c2b29b5bcd..357e63bcbf 100644 --- 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 index 4262c55187..4d4ce5b092 100644 --- 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; @@ -45,6 +53,7 @@ export interface IMenuItemProps extends IActionProps, ILinkProps { export class MenuItem extends AbstractPureComponent { public static defaultProps: IMenuItemProps = { disabled: false, + multiline: false, popoverProps: {}, shouldDismissPopover: true, text: "", @@ -76,8 +85,11 @@ export class MenuItem extends AbstractPureComponent { target={this.props.target} > + + {this.props.text} + {label && {label}} - {this.props.text} + {hasSubmenu && } ); diff --git a/packages/core/test/menu/menuTests.tsx b/packages/core/test/menu/menuTests.tsx index 96200b437e..10a72c036f 100644 --- 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(); - 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( + , + ); + 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) { IMenuProps & { children: Array> } >; } + +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 index 2215469163..e1860dcc2f 100644 --- 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 (
@@ -39,8 +40,8 @@ export class MenuExample extends BaseExample<{}> { - - + + diff --git a/packages/docs-app/src/index.scss b/packages/docs-app/src/index.scss index 63d0d6200a..3f912b50f8 100644 --- 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 index 4b6f75dc55..970fd387b2 100644 --- 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 index f0b76a28da..659c70bb81 100644 --- 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; + } } }