From 613a62793ae1f35ee6fb964b619692fd76fbde88 Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 26 Sep 2023 14:48:10 -0400 Subject: [PATCH 1/6] Create menu item component --- src/components/MenuItem/MenuItem.module.css | 102 +++++++++++++++++ src/components/MenuItem/MenuItem.stories.tsx | 54 +++++++++ src/components/MenuItem/MenuItem.test.tsx | 44 +++++++ src/components/MenuItem/MenuItem.tsx | 92 +++++++++++++++ .../__snapshots__/MenuItem.test.tsx.snap | 108 ++++++++++++++++++ src/index.ts | 1 + 6 files changed, 401 insertions(+) create mode 100644 src/components/MenuItem/MenuItem.module.css create mode 100644 src/components/MenuItem/MenuItem.stories.tsx create mode 100644 src/components/MenuItem/MenuItem.test.tsx create mode 100644 src/components/MenuItem/MenuItem.tsx create mode 100644 src/components/MenuItem/__snapshots__/MenuItem.test.tsx.snap diff --git a/src/components/MenuItem/MenuItem.module.css b/src/components/MenuItem/MenuItem.module.css new file mode 100644 index 00000000..af76234c --- /dev/null +++ b/src/components/MenuItem/MenuItem.module.css @@ -0,0 +1,102 @@ +/* +Copyright 2023 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +.item { + display: flex; + align-items: center; + padding-block: var(--cpd-space-2x); + padding-inline: var(--cpd-space-4x); + box-sizing: border-box; + inline-size: 100%; + color: var(--cpd-color-text-secondary); + background: var(--cpd-color-bg-action-secondary-rest); +} + +.item.interactive { + cursor: pointer; +} + +.icon { + flex-shrink: 0; +} + +.label { + padding-inline: var(--cpd-space-3x); + flex-basis: 100%; + text-align: start; +} + +.nav-hint { + display: none; + flex-shrink: 0; + margin-inline-end: calc(-1 * var(--cpd-space-2x)); +} + +button.item { + appearance: none; + border: none; +} + +.item.with-chevron { + padding-inline-end: var(--cpd-space-2x); +} + +.item[data-kind="primary"] > .label { + color: var(--cpd-color-text-primary); +} + +.item[data-kind="primary"] > .icon { + color: var(--cpd-color-icon-primary); +} + +.item[data-kind="primary"] > .nav-hint { + color: var(--cpd-color-icon-tertiary); +} + +.item[data-kind="critical"] > .label { + color: var(--cpd-color-text-critical-primary); +} + +.item[data-kind="critical"] > .icon, +.item[data-kind="critical"] > .nav-hint { + color: var(--cpd-color-icon-critical-primary); +} + +@media (hover) { + .item.interactive[data-kind="primary"]:hover { + background: var(--cpd-color-bg-action-secondary-hovered); + } + + .item.interactive[data-kind="critical"]:hover { + background: var(--cpd-color-bg-critical-subtle); + } + + .item.interactive:hover > .nav-hint { + display: initial; + } + + .item.interactive:hover > .nav-hint ~ * { + display: none; + } +} + +.item.interactive[data-kind="primary"]:active { + background: var(--cpd-color-bg-action-secondary-pressed); +} + +.item.interactive[data-kind="critical"]:active { + background: var(--cpd-color-bg-critical-subtle); +} diff --git a/src/components/MenuItem/MenuItem.stories.tsx b/src/components/MenuItem/MenuItem.stories.tsx new file mode 100644 index 00000000..5d83f9d0 --- /dev/null +++ b/src/components/MenuItem/MenuItem.stories.tsx @@ -0,0 +1,54 @@ +/* +Copyright 2023 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from "react"; +import { Meta, StoryFn } from "@storybook/react"; +import ExtensionsIcon from "@vector-im/compound-design-tokens/icons/extensions.svg"; +import ChatIcon from "@vector-im/compound-design-tokens/icons/chat.svg"; + +import { MenuItem as MenuItemComponent } from "./MenuItem"; +import { Text } from "../Typography/Text"; + +export default { + title: "MenuItem", + component: MenuItemComponent, + argTypes: {}, + args: {}, +} as Meta; + +const Template: StoryFn = (args) => ( +
+ + + 99 + + + +
+); + +export const Primary = Template.bind({}); +Primary.args = { kind: "primary" }; + +export const Critical = Template.bind({}); +Critical.args = { kind: "critical" }; + +Primary.parameters = Critical.parameters = { + design: { + type: "figma", + url: "https://www.figma.com/file/rTaQE2nIUSLav4Tg3nozq7/Compound-Web-Components?type=design&node-id=712-6909&mode=dev", + }, +}; diff --git a/src/components/MenuItem/MenuItem.test.tsx b/src/components/MenuItem/MenuItem.test.tsx new file mode 100644 index 00000000..add6bf87 --- /dev/null +++ b/src/components/MenuItem/MenuItem.test.tsx @@ -0,0 +1,44 @@ +/* +Copyright 2023 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { describe, it, expect } from "vitest"; +import { render } from "@testing-library/react"; +import React from "react"; +import LeaveIcon from "@vector-im/compound-design-tokens/icons/leave.svg"; +import UserProfileIcon from "@vector-im/compound-design-tokens/icons/user-profile.svg"; + +import { MenuItem } from "./MenuItem"; +import { Text } from "../Typography/Text"; + +describe("MenuItem", () => { + it("renders", () => { + const { asFragment } = render( + , + ); + expect(asFragment()).toMatchSnapshot(); + }); + + it("renders with a child", () => { + const { asFragment } = render( + + + 10 + + , + ); + expect(asFragment()).toMatchSnapshot(); + }); +}); diff --git a/src/components/MenuItem/MenuItem.tsx b/src/components/MenuItem/MenuItem.tsx new file mode 100644 index 00000000..95bea261 --- /dev/null +++ b/src/components/MenuItem/MenuItem.tsx @@ -0,0 +1,92 @@ +/* +Copyright 2023 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import classnames from "classnames"; +import React, { + ComponentPropsWithoutRef, + ComponentType, + ElementType, + SVGAttributes, +} from "react"; +import styles from "./MenuItem.module.css"; +import { Text } from "../Typography/Text"; +import ChevronRightIcon from "@vector-im/compound-design-tokens/icons/chevron-right.svg"; + +type MenuItemElement = "button" | "label" | "div"; + +type Props = { + /** + * The element type of this menu item. + * @default button + */ + as?: C; + className?: string; + /** + * The icon to show on this menu item. + */ + Icon: ComponentType>; + /** + * The label to show on this menu item. + */ + label: string; + /** + * The color variant of the menu item. + * @default primary + */ + kind?: "primary" | "critical"; + /** + * Whether to replace the children with a navigation hint on hover. + * @default true + */ + navHint?: boolean; +} & ComponentPropsWithoutRef; + +export const MenuItem = ({ + as, + className, + Icon, + label, + kind = "primary", + navHint = true, + children, + ...props +}: Props) => { + const Component = as ?? ("button" as ElementType); + + return ( + + + + {label} + + {navHint && ( + + )} + {children} + + ); +}; diff --git a/src/components/MenuItem/__snapshots__/MenuItem.test.tsx.snap b/src/components/MenuItem/__snapshots__/MenuItem.test.tsx.snap new file mode 100644 index 00000000..0e12f7b7 --- /dev/null +++ b/src/components/MenuItem/__snapshots__/MenuItem.test.tsx.snap @@ -0,0 +1,108 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`MenuItem > renders 1`] = ` + + + +`; + +exports[`MenuItem > renders with a child 1`] = ` + + + +`; diff --git a/src/index.ts b/src/index.ts index 16637933..5ffa46eb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -41,6 +41,7 @@ export { export { IconButton } from "./components/IconButton/IconButton"; export { Label } from "./components/Form/Label"; export { Link } from "./components/Link/Link"; +export { MenuItem } from "./components/MenuItem/MenuItem"; export { Message } from "./components/Form/Message"; export { PasswordControl } from "./components/Form/Controls/Password"; export { Radio } from "./components/Radio/Radio"; From 3bc6c1c1da0906617772379c8363b4e1e6eb4874 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 27 Sep 2023 13:37:01 -0400 Subject: [PATCH 2/6] Handle overflowing labels more elegantly for i18n purposes --- src/components/MenuItem/MenuItem.module.css | 11 +++++++---- src/components/MenuItem/MenuItem.stories.tsx | 6 +++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/components/MenuItem/MenuItem.module.css b/src/components/MenuItem/MenuItem.module.css index af76234c..76865f70 100644 --- a/src/components/MenuItem/MenuItem.module.css +++ b/src/components/MenuItem/MenuItem.module.css @@ -15,8 +15,10 @@ limitations under the License. */ .item { - display: flex; + display: grid; + grid-template: "icon label ." auto "empty1 label empty2" auto / auto auto 1fr; align-items: center; + justify-items: end; padding-block: var(--cpd-space-2x); padding-inline: var(--cpd-space-4x); box-sizing: border-box; @@ -30,12 +32,13 @@ limitations under the License. } .icon { - flex-shrink: 0; + grid-area: icon; + margin-inline-end: var(--cpd-space-3x); } .label { - padding-inline: var(--cpd-space-3x); - flex-basis: 100%; + grid-area: label; + margin-inline-end: var(--cpd-space-4x); text-align: start; } diff --git a/src/components/MenuItem/MenuItem.stories.tsx b/src/components/MenuItem/MenuItem.stories.tsx index 5d83f9d0..e4a0810e 100644 --- a/src/components/MenuItem/MenuItem.stories.tsx +++ b/src/components/MenuItem/MenuItem.stories.tsx @@ -36,7 +36,11 @@ const Template: StoryFn = (args) => ( 99 - + ); From 9cc42c6bffa816e6a38aa2064481bb5a920929fa Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 27 Sep 2023 13:51:48 -0400 Subject: [PATCH 3/6] Allow menu items to be unlabeled For example, the new volume slider menu items in Element Call call for this behavior. --- src/components/MenuItem/MenuItem.module.css | 8 ++++ src/components/MenuItem/MenuItem.stories.tsx | 6 +++ src/components/MenuItem/MenuItem.test.tsx | 10 +++++ src/components/MenuItem/MenuItem.tsx | 12 +++-- .../__snapshots__/MenuItem.test.tsx.snap | 45 +++++++++++++++++++ 5 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/components/MenuItem/MenuItem.module.css b/src/components/MenuItem/MenuItem.module.css index 76865f70..3000f6f3 100644 --- a/src/components/MenuItem/MenuItem.module.css +++ b/src/components/MenuItem/MenuItem.module.css @@ -31,11 +31,19 @@ limitations under the License. cursor: pointer; } +.item.no-label { + grid-template: "icon ." auto / auto 1fr; +} + .icon { grid-area: icon; margin-inline-end: var(--cpd-space-3x); } +.item.no-label .icon { + margin-inline-end: var(--cpd-space-4x); +} + .label { grid-area: label; margin-inline-end: var(--cpd-space-4x); diff --git a/src/components/MenuItem/MenuItem.stories.tsx b/src/components/MenuItem/MenuItem.stories.tsx index e4a0810e..2239a048 100644 --- a/src/components/MenuItem/MenuItem.stories.tsx +++ b/src/components/MenuItem/MenuItem.stories.tsx @@ -18,6 +18,7 @@ import React from "react"; import { Meta, StoryFn } from "@storybook/react"; import ExtensionsIcon from "@vector-im/compound-design-tokens/icons/extensions.svg"; import ChatIcon from "@vector-im/compound-design-tokens/icons/chat.svg"; +import SettingsLabel from "@vector-im/compound-design-tokens/icons/settings.svg"; import { MenuItem as MenuItemComponent } from "./MenuItem"; import { Text } from "../Typography/Text"; @@ -41,6 +42,11 @@ const Template: StoryFn = (args) => ( Icon={ExtensionsIcon} label="Second item with a name that's quite long" /> + + + Third item without a label + + ); diff --git a/src/components/MenuItem/MenuItem.test.tsx b/src/components/MenuItem/MenuItem.test.tsx index add6bf87..dee176d3 100644 --- a/src/components/MenuItem/MenuItem.test.tsx +++ b/src/components/MenuItem/MenuItem.test.tsx @@ -19,6 +19,7 @@ import { render } from "@testing-library/react"; import React from "react"; import LeaveIcon from "@vector-im/compound-design-tokens/icons/leave.svg"; import UserProfileIcon from "@vector-im/compound-design-tokens/icons/user-profile.svg"; +import MicOnOutlineIcon from "@vector-im/compound-design-tokens/icons/mic-on-outline.svg"; import { MenuItem } from "./MenuItem"; import { Text } from "../Typography/Text"; @@ -41,4 +42,13 @@ describe("MenuItem", () => { ); expect(asFragment()).toMatchSnapshot(); }); + + it("renders without a label", () => { + const { asFragment } = render( + + Imagine that there might be a volume slider here in place of the label + , + ); + expect(asFragment()).toMatchSnapshot(); + }); }); diff --git a/src/components/MenuItem/MenuItem.tsx b/src/components/MenuItem/MenuItem.tsx index 95bea261..b47920ec 100644 --- a/src/components/MenuItem/MenuItem.tsx +++ b/src/components/MenuItem/MenuItem.tsx @@ -41,7 +41,8 @@ type Props = { /** * The label to show on this menu item. */ - label: string; + // This prop is required because it's rare to not want a label + label: string | undefined; /** * The color variant of the menu item. * @default primary @@ -71,13 +72,16 @@ export const MenuItem = ({ {...props} className={classnames(className, styles.item, { [styles.interactive]: as !== "div", + [styles["no-label"]]: label === undefined, })} data-kind={kind} > - - {label} - + {label !== undefined && ( + + {label} + + )} {navHint && ( renders with a child 1`] = ` `; + +exports[`MenuItem > renders without a label 1`] = ` + + + +`; From 0533ba2918a0d2517816627c23351dd084aaa28b Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 27 Sep 2023 13:53:04 -0400 Subject: [PATCH 4/6] Incorporate feedback from review --- src/components/MenuItem/MenuItem.module.css | 2 ++ src/components/MenuItem/MenuItem.stories.tsx | 7 ------- src/components/MenuItem/MenuItem.tsx | 19 +++++++++++-------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/components/MenuItem/MenuItem.module.css b/src/components/MenuItem/MenuItem.module.css index 3000f6f3..d75c22b1 100644 --- a/src/components/MenuItem/MenuItem.module.css +++ b/src/components/MenuItem/MenuItem.module.css @@ -51,6 +51,7 @@ limitations under the License. } .nav-hint { + /* Hidden until the item is hovered over */ display: none; flex-shrink: 0; margin-inline-end: calc(-1 * var(--cpd-space-2x)); @@ -95,6 +96,7 @@ button.item { background: var(--cpd-color-bg-critical-subtle); } + /* Replace the children with the navigation hint on hover */ .item.interactive:hover > .nav-hint { display: initial; } diff --git a/src/components/MenuItem/MenuItem.stories.tsx b/src/components/MenuItem/MenuItem.stories.tsx index 2239a048..7301ba6c 100644 --- a/src/components/MenuItem/MenuItem.stories.tsx +++ b/src/components/MenuItem/MenuItem.stories.tsx @@ -55,10 +55,3 @@ Primary.args = { kind: "primary" }; export const Critical = Template.bind({}); Critical.args = { kind: "critical" }; - -Primary.parameters = Critical.parameters = { - design: { - type: "figma", - url: "https://www.figma.com/file/rTaQE2nIUSLav4Tg3nozq7/Compound-Web-Components?type=design&node-id=712-6909&mode=dev", - }, -}; diff --git a/src/components/MenuItem/MenuItem.tsx b/src/components/MenuItem/MenuItem.tsx index b47920ec..08d34e36 100644 --- a/src/components/MenuItem/MenuItem.tsx +++ b/src/components/MenuItem/MenuItem.tsx @@ -25,7 +25,7 @@ import styles from "./MenuItem.module.css"; import { Text } from "../Typography/Text"; import ChevronRightIcon from "@vector-im/compound-design-tokens/icons/chevron-right.svg"; -type MenuItemElement = "button" | "label" | "div"; +type MenuItemElement = "button" | "label" | "a" | "div"; type Props = { /** @@ -33,6 +33,9 @@ type Props = { * @default button */ as?: C; + /** + * The CSS class name. + */ className?: string; /** * The icon to show on this menu item. @@ -48,20 +51,18 @@ type Props = { * @default primary */ kind?: "primary" | "critical"; - /** - * Whether to replace the children with a navigation hint on hover. - * @default true - */ - navHint?: boolean; } & ComponentPropsWithoutRef; +/** + * An item within a menu, acting either as a navigation button, or simply a + * container for other interactive elements. + */ export const MenuItem = ({ as, className, Icon, label, kind = "primary", - navHint = true, children, ...props }: Props) => { @@ -82,7 +83,9 @@ export const MenuItem = ({ {label} )} - {navHint && ( + {/* We use CSS to swap between this navigation hint and the provided + children on hover - see the styles module. */} + {(Component === "button" || Component === "a") && ( Date: Wed, 27 Sep 2023 14:11:21 -0400 Subject: [PATCH 5/6] Add menuitem role to menu items --- src/components/MenuItem/MenuItem.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/MenuItem/MenuItem.tsx b/src/components/MenuItem/MenuItem.tsx index 08d34e36..f914b69c 100644 --- a/src/components/MenuItem/MenuItem.tsx +++ b/src/components/MenuItem/MenuItem.tsx @@ -70,6 +70,7 @@ export const MenuItem = ({ return ( Date: Wed, 27 Sep 2023 14:13:59 -0400 Subject: [PATCH 6/6] Fix tests --- src/components/MenuItem/__snapshots__/MenuItem.test.tsx.snap | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/MenuItem/__snapshots__/MenuItem.test.tsx.snap b/src/components/MenuItem/__snapshots__/MenuItem.test.tsx.snap index 1b8b796c..58e4d5e0 100644 --- a/src/components/MenuItem/__snapshots__/MenuItem.test.tsx.snap +++ b/src/components/MenuItem/__snapshots__/MenuItem.test.tsx.snap @@ -5,6 +5,7 @@ exports[`MenuItem > renders 1`] = `