From fbee107b8483c78bd5bb5a591dc80bc559fc0ade Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 7 Jun 2024 10:00:28 +0100 Subject: [PATCH 1/9] Add comment to *MenuItem about menu/bar roles --- src/components/Menu/MenuItem.tsx | 1 + src/components/Menu/ToggleMenuItem.tsx | 1 + 2 files changed, 2 insertions(+) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 36b04375..e4132868 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -71,6 +71,7 @@ type Props = { /** * An item within a menu, acting either as a navigation button, or simply a * container for other interactive elements. + * Must be used within a compound Menu or other `menu` or `menubar` aria role subtree. */ export const MenuItem = ({ as, diff --git a/src/components/Menu/ToggleMenuItem.tsx b/src/components/Menu/ToggleMenuItem.tsx index 3e5e4ff2..905eab6e 100644 --- a/src/components/Menu/ToggleMenuItem.tsx +++ b/src/components/Menu/ToggleMenuItem.tsx @@ -28,6 +28,7 @@ type Props = Pick< /** * A menu item with a toggle control. Clicking anywhere on the surface will * activate the toggle. + * Must be used within a compound Menu or other `menu` or `menubar` aria role subtree. */ export const ToggleMenuItem = forwardRef( function ToggleMenuItem( From 278d67af32b8649c21363d787d43f66e10e9020d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 7 Jun 2024 10:03:36 +0100 Subject: [PATCH 2/9] Add missing aria-checked attribute to ToggleMenuItem --- src/components/Menu/ToggleMenuItem.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Menu/ToggleMenuItem.tsx b/src/components/Menu/ToggleMenuItem.tsx index 905eab6e..62cf59d8 100644 --- a/src/components/Menu/ToggleMenuItem.tsx +++ b/src/components/Menu/ToggleMenuItem.tsx @@ -45,6 +45,7 @@ export const ToggleMenuItem = forwardRef( Icon={Icon} label={label} onSelect={onSelect} + aria-checked={toggleProps.checked} > From f6f124044cd3ac670d041fec9e30079931b2f744 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 7 Jun 2024 10:07:58 +0100 Subject: [PATCH 3/9] Fix ToggleMenuItem role & label validity --- src/components/Menu/ToggleMenuItem.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/components/Menu/ToggleMenuItem.tsx b/src/components/Menu/ToggleMenuItem.tsx index 62cf59d8..190ae93e 100644 --- a/src/components/Menu/ToggleMenuItem.tsx +++ b/src/components/Menu/ToggleMenuItem.tsx @@ -18,6 +18,8 @@ import React, { ComponentProps, forwardRef } from "react"; import { MenuItem } from "./MenuItem"; import { ToggleInput } from "../Form/Controls/Toggle"; import useId from "../../utils/useId"; +import styles from "./MenuItem.module.css"; +import { Text } from "../Typography/Text"; type Props = Pick< ComponentProps, @@ -36,17 +38,19 @@ export const ToggleMenuItem = forwardRef( ref, ) { const toggleId = useId(); + // We render the label ourselves as a `label` element cannot have role `menuitemcheckbox` return ( + + {label} + ); From 6c50637c35e36c0f85435e0af6316ebfe87cc04f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 7 Jun 2024 10:09:53 +0100 Subject: [PATCH 4/9] Prettier --- src/components/Menu/ToggleMenuItem.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/components/Menu/ToggleMenuItem.tsx b/src/components/Menu/ToggleMenuItem.tsx index 190ae93e..af20cf59 100644 --- a/src/components/Menu/ToggleMenuItem.tsx +++ b/src/components/Menu/ToggleMenuItem.tsx @@ -48,7 +48,13 @@ export const ToggleMenuItem = forwardRef( onSelect={onSelect} aria-checked={toggleProps.checked} > - + {label} From ee2577a8cef28c30f0f4bbb9ee26280f7df233e8 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 7 Jun 2024 10:10:37 +0100 Subject: [PATCH 5/9] Update snapshot --- src/components/Menu/ToggleMenuItem.tsx | 1 + .../Menu/__snapshots__/ToggleMenuItem.test.tsx.snap | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/components/Menu/ToggleMenuItem.tsx b/src/components/Menu/ToggleMenuItem.tsx index af20cf59..23c32ad8 100644 --- a/src/components/Menu/ToggleMenuItem.tsx +++ b/src/components/Menu/ToggleMenuItem.tsx @@ -41,6 +41,7 @@ export const ToggleMenuItem = forwardRef( // We render the label ourselves as a `label` element cannot have role `menuitemcheckbox` return ( renders 1`] = ` -
@@ -38,6 +38,6 @@ exports[`ToggleMenuItem > renders 1`] = ` class="_ui_adcb29" />
- +
`; From 0e13a685b811c62933611d9dd15e07f91a9a88d7 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 7 Jun 2024 10:28:30 +0100 Subject: [PATCH 6/9] Override no-label styles --- src/components/Menu/ToggleMenuItem.module.css | 25 +++++++++++++++++++ src/components/Menu/ToggleMenuItem.tsx | 8 +++--- 2 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 src/components/Menu/ToggleMenuItem.module.css diff --git a/src/components/Menu/ToggleMenuItem.module.css b/src/components/Menu/ToggleMenuItem.module.css new file mode 100644 index 00000000..97fdace9 --- /dev/null +++ b/src/components/Menu/ToggleMenuItem.module.css @@ -0,0 +1,25 @@ +/* +Copyright 2024 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 { + /* Important to override the no-label styles */ + grid-template: "icon label ." auto "empty1 label empty2" auto / auto auto minmax( + /* Reserve space for the chevron so that the layout doesn't shift on + hover */ + var(--cpd-space-2x), + 1fr + ) !important; +} diff --git a/src/components/Menu/ToggleMenuItem.tsx b/src/components/Menu/ToggleMenuItem.tsx index 23c32ad8..5e81341f 100644 --- a/src/components/Menu/ToggleMenuItem.tsx +++ b/src/components/Menu/ToggleMenuItem.tsx @@ -15,11 +15,13 @@ limitations under the License. */ import React, { ComponentProps, forwardRef } from "react"; +import classNames from "classnames"; import { MenuItem } from "./MenuItem"; import { ToggleInput } from "../Form/Controls/Toggle"; import useId from "../../utils/useId"; -import styles from "./MenuItem.module.css"; import { Text } from "../Typography/Text"; +import menuItemStyles from "./MenuItem.module.css"; +import styles from "./ToggleMenuItem.module.css"; type Props = Pick< ComponentProps, @@ -43,14 +45,14 @@ export const ToggleMenuItem = forwardRef( Date: Fri, 7 Jun 2024 10:33:12 +0100 Subject: [PATCH 7/9] iterate --- src/components/Menu/ToggleMenuItem.module.css | 2 +- src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Menu/ToggleMenuItem.module.css b/src/components/Menu/ToggleMenuItem.module.css index 97fdace9..13a3b837 100644 --- a/src/components/Menu/ToggleMenuItem.module.css +++ b/src/components/Menu/ToggleMenuItem.module.css @@ -20,6 +20,6 @@ limitations under the License. /* Reserve space for the chevron so that the layout doesn't shift on hover */ var(--cpd-space-2x), - 1fr + 1fr ) !important; } diff --git a/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap b/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap index e77b6f2f..4aaabd79 100644 --- a/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap +++ b/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap @@ -3,7 +3,7 @@ exports[`ToggleMenuItem > renders 1`] = `
From 1eaae3f33dee0599f914fe21aba0f7f892d78489 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 7 Jun 2024 11:31:45 +0100 Subject: [PATCH 8/9] Simplify --- src/components/Menu/MenuItem.tsx | 14 ++++++++++- src/components/Menu/ToggleMenuItem.module.css | 25 ------------------- src/components/Menu/ToggleMenuItem.tsx | 21 +++++----------- .../ToggleMenuItem.test.tsx.snap | 2 +- 4 files changed, 20 insertions(+), 42 deletions(-) delete mode 100644 src/components/Menu/ToggleMenuItem.module.css diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index e4132868..80e45e5a 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -53,6 +53,10 @@ type Props = { */ // This prop is required because it's rare to not want a label label: string | null; + /** + * Additional properties to pass to the Text label component. + */ + labelProps?: ComponentPropsWithoutRef; /** * Event callback for when the item is selected via mouse, touch, or keyboard. * Calling event.preventDefault in this handler will prevent the menu from @@ -78,6 +82,7 @@ export const MenuItem = ({ className, Icon, label, + labelProps, onSelect, kind = "primary", children, @@ -121,6 +126,7 @@ export const MenuItem = ({ data-kind={kind} onClick={onClick} disabled={disabled} + manualLabel > {iconIsReactElement ? ( {componentIcon} @@ -134,7 +140,13 @@ export const MenuItem = ({ )} {label !== null && ( - + {label} )} diff --git a/src/components/Menu/ToggleMenuItem.module.css b/src/components/Menu/ToggleMenuItem.module.css deleted file mode 100644 index 13a3b837..00000000 --- a/src/components/Menu/ToggleMenuItem.module.css +++ /dev/null @@ -1,25 +0,0 @@ -/* -Copyright 2024 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 { - /* Important to override the no-label styles */ - grid-template: "icon label ." auto "empty1 label empty2" auto / auto auto minmax( - /* Reserve space for the chevron so that the layout doesn't shift on - hover */ - var(--cpd-space-2x), - 1fr - ) !important; -} diff --git a/src/components/Menu/ToggleMenuItem.tsx b/src/components/Menu/ToggleMenuItem.tsx index 5e81341f..1a1bfbb1 100644 --- a/src/components/Menu/ToggleMenuItem.tsx +++ b/src/components/Menu/ToggleMenuItem.tsx @@ -15,13 +15,9 @@ limitations under the License. */ import React, { ComponentProps, forwardRef } from "react"; -import classNames from "classnames"; import { MenuItem } from "./MenuItem"; import { ToggleInput } from "../Form/Controls/Toggle"; import useId from "../../utils/useId"; -import { Text } from "../Typography/Text"; -import menuItemStyles from "./MenuItem.module.css"; -import styles from "./ToggleMenuItem.module.css"; type Props = Pick< ComponentProps, @@ -45,21 +41,16 @@ export const ToggleMenuItem = forwardRef( - - {label} - ); diff --git a/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap b/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap index 4aaabd79..95cde932 100644 --- a/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap +++ b/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap @@ -3,7 +3,7 @@ exports[`ToggleMenuItem > renders 1`] = `
From 5a34068b8d585e0010e6db988d1bde6c5785be15 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 7 Jun 2024 11:53:18 +0100 Subject: [PATCH 9/9] Remove stale prop --- src/components/Menu/MenuItem.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 80e45e5a..1a75a3af 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -126,7 +126,6 @@ export const MenuItem = ({ data-kind={kind} onClick={onClick} disabled={disabled} - manualLabel > {iconIsReactElement ? ( {componentIcon}