diff --git a/cypress/component/Menu.spec.tsx b/cypress/component/Menu.spec.tsx index eca0e87322..3ce6a9303b 100644 --- a/cypress/component/Menu.spec.tsx +++ b/cypress/component/Menu.spec.tsx @@ -1,5 +1,7 @@ import {Basic} from '../../modules/react/menu/stories/examples/Basic'; import {MenuWithFallbackPlacements} from '../../modules/react/menu/stories/examples/MenuWithFallbackPlacements'; +import {NestedDynamic} from '../../modules/react/menu/stories/examples/NestedDynamic'; +import {NestedSiblings} from '../../modules/react/menu/stories/examples/NestedSiblings'; describe('Menu', () => { context(`given the [Components/Popups/Menu, Basic] story is rendered`, () => { @@ -183,6 +185,27 @@ describe('Menu', () => { }); }); + context('when a disabled item has focus and the menu is closed and reopened', () => { + beforeEach(() => { + cy.findByRole('menuitem', {name: 'First Item'}) + .should('be.focused') + .realType('{uparrow}'); + cy.findByRole('menuitem', {name: 'Fourth Item'}).should('be.focused'); + cy.focused().realType('{esc}'); + cy.findByRole('menu').should('not.exist'); + cy.findByRole('button', {name: 'Open Menu'}).click(); + cy.findByRole('menu').should('be.visible'); + }); + + it('should move focus to the first item instead of the disabled item', () => { + cy.findByRole('menuitem', {name: 'First Item'}).should('be.focused'); + }); + + it('should not leave focus on the disabled item', () => { + cy.findByRole('menuitem', {name: 'Fourth Item'}).should('not.be.focused'); + }); + }); + context('when the enter key is pressed', () => { beforeEach(() => { cy.findByRole('menu').should('exist'); @@ -209,6 +232,141 @@ describe('Menu', () => { }); }); + context(`given the [Components/Popups/Menu, NestedSiblings] story is rendered`, () => { + beforeEach(() => { + cy.mount(); + cy.findByRole('button', {name: 'Open Menu'}).click(); + cy.findByRole('menu').should('be.visible'); + }); + + context('when the "Second Item" submenu is opened by hovering', () => { + beforeEach(() => { + cy.findByRole('menuitem', {name: 'Second Item'}).realHover(); + cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('be.visible'); + }); + + it('should set aria-expanded to true on "Second Item"', () => { + cy.findByRole('menuitem', {name: 'Second Item'}).should( + 'have.attr', + 'aria-expanded', + 'true' + ); + }); + + context('when the sibling "Third Item" is then hovered', () => { + beforeEach(() => { + cy.findByRole('menuitem', {name: 'Third Item'}).realHover(); + cy.findByRole('menuitem', {name: 'Third: First Sub Item'}).should('be.visible'); + }); + + it('should close the "Second Item" submenu', () => { + cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('not.exist'); + }); + + it('should set aria-expanded to false on "Second Item"', () => { + cy.findByRole('menuitem', {name: 'Second Item'}).should( + 'have.attr', + 'aria-expanded', + 'false' + ); + }); + + it('should leave only the root menu and one submenu open', () => { + cy.findAllByRole('menu').should('have.length', 2); + }); + + context('when the original "Second Item" is hovered again', () => { + beforeEach(() => { + cy.findByRole('menuitem', {name: 'Second Item'}).realHover(); + cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('be.visible'); + }); + + it('should close the "Third Item" submenu', () => { + cy.findByRole('menuitem', {name: 'Third: First Sub Item'}).should('not.exist'); + }); + + it('should leave only the root menu and one submenu open', () => { + cy.findAllByRole('menu').should('have.length', 2); + }); + }); + }); + }); + + context('when a submenu is opened with the right arrow key', () => { + beforeEach(() => { + cy.findByRole('menuitem', {name: 'First Item'}).should('be.focused'); + cy.focused().realType('{downarrow}'); + cy.findByRole('menuitem', {name: 'Second Item'}).should('be.focused'); + cy.focused().realType('{rightarrow}'); + }); + + it('should open the submenu', () => { + cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('be.visible'); + }); + + it('should keep the submenu open and move focus into it', () => { + cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('be.focused'); + }); + + context('when the left arrow key is pressed', () => { + beforeEach(() => { + cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('be.focused'); + cy.focused().realType('{leftarrow}'); + }); + + it('should close the submenu', () => { + cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('not.exist'); + }); + + it('should return focus to the "Second Item" target', () => { + cy.findByRole('menuitem', {name: 'Second Item'}).should('be.focused'); + }); + }); + }); + }); + + context(`given the [Components/Popups/Menu, NestedDynamic] story is rendered`, () => { + beforeEach(() => { + cy.mount(); + cy.findByRole('button', {name: 'Open Menu'}).click(); + cy.findByRole('menu').should('be.visible'); + }); + + context('when hovering deeper into a chain of nested submenus', () => { + beforeEach(() => { + cy.findByRole('menuitem', {name: 'Second Item'}).realHover(); + cy.findByRole('menuitem', {name: 'Second Sub Item'}).should('be.visible'); + cy.findByRole('menuitem', {name: 'Second Sub Item'}).realHover(); + cy.findByRole('menuitem', {name: 'Second Sub Sub Item'}).should('be.visible'); + }); + + it('should keep the ancestor submenu open', () => { + cy.findByRole('menuitem', {name: 'Second Item'}).should( + 'have.attr', + 'aria-expanded', + 'true' + ); + }); + + it('should keep every menu in the open chain visible', () => { + cy.findAllByRole('menu').should('have.length', 3); + }); + + // Re-entering an item that is already part of the open trail should not collapse the trail, + // matching the platform menu convention of only closing when a sibling is hovered. + context('when hovering back onto the ancestor item in the open trail', () => { + beforeEach(() => { + cy.findByRole('menuitem', {name: 'Second Item'}).realHover(); + }); + + it('should keep the open trail intact', () => { + cy.findByRole('menuitem', {name: 'Second Sub Sub Item'}).should('be.visible'); + cy.findAllByRole('menu').should('have.length', 3); + }); + }); + }); + }); + context(`given the [Testing/Popups/Menu, MenuWithFallbackPlacements] example is rendered`, () => { beforeEach(() => { cy.mount(); diff --git a/modules/react/collection/index.ts b/modules/react/collection/index.ts index 1606adf54d..8755e17acd 100644 --- a/modules/react/collection/index.ts +++ b/modules/react/collection/index.ts @@ -18,7 +18,8 @@ export * from './lib/useListItemAllowChildStrings'; export * from './lib/useListItemRemoveOnDeleteKey'; export * from './lib/focusOnCurrentCursor'; export * from './lib/listItemRemove'; -export {ListBox, type ListBoxProps} from './lib/ListBox'; +export * from './lib/isElementDisabled'; +export {ListBox, type ListBoxProps, listBoxContainerStencil} from './lib/ListBox'; export {keyboardEventToCursorEvents} from './lib/keyUtils'; export { singleSelectionManager, diff --git a/modules/react/collection/lib/ListBox.tsx b/modules/react/collection/lib/ListBox.tsx index edcd7959fa..b810249cae 100644 --- a/modules/react/collection/lib/ListBox.tsx +++ b/modules/react/collection/lib/ListBox.tsx @@ -51,7 +51,7 @@ export const useListBox = createElemPropsHook(useListModel)(model => { }; }); -const listBoxContainerStencil = createStencil({ +export const listBoxContainerStencil = createStencil({ parts: { listBoxContainer: 'list-box-container', }, diff --git a/modules/react/collection/lib/isElementDisabled.ts b/modules/react/collection/lib/isElementDisabled.ts new file mode 100644 index 0000000000..9b90ee7034 --- /dev/null +++ b/modules/react/collection/lib/isElementDisabled.ts @@ -0,0 +1,16 @@ +/** + * Checks whether an element is disabled, either via the native `disabled` DOM property (set, for + * example, by `useListItemRegister` when an item's id is included in `state.nonInteractiveIds`) or + * via the `aria-disabled="true"` attribute (set by consumers directly on an item, e.g. + * ``). + * + * Collection items should use this check instead of testing `aria-disabled` alone so that items + * disabled through either mechanism are consistently blocked from activating (click, Enter/Space, + * hover-intent, etc.). + */ +export const isElementDisabled = (element: Element | null | undefined): boolean => { + if (!element) { + return false; + } + return element.getAttribute('aria-disabled') === 'true' || element.hasAttribute('disabled'); +}; diff --git a/modules/react/collection/lib/useListItemSelect.tsx b/modules/react/collection/lib/useListItemSelect.tsx index ca7cc16ea0..8af63a7193 100644 --- a/modules/react/collection/lib/useListItemSelect.tsx +++ b/modules/react/collection/lib/useListItemSelect.tsx @@ -2,6 +2,7 @@ import React from 'react'; import {createElemPropsHook} from '@workday/canvas-kit-react/common'; +import {isElementDisabled} from './isElementDisabled'; import {useListModel} from './useListModel'; /** @@ -23,12 +24,11 @@ export const useListItemSelect = createElemPropsHook(useListModel)(( ) => { const name = elemProps['data-id'] || ''; const onClick = (event: React.MouseEvent) => { - if ( - !state.nonInteractiveIds.includes(name) && - event.currentTarget.getAttribute('aria-disabled') !== 'true' - ) { - events.select({id: name}); + if (isElementDisabled(event.currentTarget) || state.nonInteractiveIds.includes(name)) { + return null; } + events.select({id: name}); + return undefined; }; return {onClick}; diff --git a/modules/react/menu/lib/MenuCard.tsx b/modules/react/menu/lib/MenuCard.tsx index 7481f61cc9..99340ef250 100644 --- a/modules/react/menu/lib/MenuCard.tsx +++ b/modules/react/menu/lib/MenuCard.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import {Card} from '@workday/canvas-kit-react/card'; +import {listBoxContainerStencil} from '@workday/canvas-kit-react/collection'; import { ExtractProps, createElemPropsHook, @@ -8,7 +9,7 @@ import { } from '@workday/canvas-kit-react/common'; import {mergeStyles} from '@workday/canvas-kit-react/layout'; import {getTransformFromPlacement} from '@workday/canvas-kit-react/popup'; -import {calc, createStencil, px2rem} from '@workday/canvas-kit-styling'; +import {calc, createStencil, cssVar, px2rem} from '@workday/canvas-kit-styling'; import {system} from '@workday/canvas-tokens-web'; import {useMenuModel} from './useMenuModel'; @@ -42,12 +43,22 @@ export const menuCardStencil = createStencil({ maxWidth: calc.subtract('100vw', system.legacy.size.sm), boxShadow: system.depth[3], minWidth, - maxHeight, + maxHeight: cssVar(maxHeight, '60vh'), transformOrigin: `${transformOriginVertical} ${transformOriginHorizontal}`, // Allow overriding of animation in special cases '.wd-no-animation &': { animation: 'none', }, + [`&:where(:has(${listBoxContainerStencil.parts.listBoxContainer.selector}))`]: { + overflow: 'hidden', + }, + [`& :where(${listBoxContainerStencil.parts.listBoxContainer.selector})`]: { + borderRadius: system.legacy.shape.xxl, + // Card is a flex column container. Without this, a flex child won't shrink below its + // content size, so `maxHeight` on the Card would be ignored and content would overflow + // instead of scrolling inside the list-box-container. + minHeight: 0, + }, }), }); diff --git a/modules/react/menu/lib/MenuItem.tsx b/modules/react/menu/lib/MenuItem.tsx index d8e196d4ec..199f1f4b12 100644 --- a/modules/react/menu/lib/MenuItem.tsx +++ b/modules/react/menu/lib/MenuItem.tsx @@ -2,11 +2,13 @@ import * as React from 'react'; import { isCursor, + isElementDisabled, useListItemRegister, useListItemRovingFocus, useListItemSelect, } from '@workday/canvas-kit-react/collection'; import { + changeFocus, composeHooks, createComponent, createElemPropsHook, @@ -179,7 +181,12 @@ export const useMenuItemArrowReturn = createElemPropsHook(useMenuModel)(model => onKeyDown(event: React.KeyboardEvent) { const styles = getComputedStyle(event.currentTarget); if (event.key === 'ArrowLeft' && styles.direction === 'ltr' && model.UNSTABLE_parentModel) { + event.preventDefault(); + const target = model.state.targetRef.current as HTMLElement | null; model.events.hide(event); + requestAnimationFrame(() => { + changeFocus(target); + }); } }, }; @@ -192,6 +199,27 @@ export const useMenuItemFocus = createElemPropsHook(useMenuModel)(( ) => { const {localRef, elementRef} = useLocalRef(ref as React.Ref); const id = elemProps['data-id']; + + // A menu keeps its cursor between openings, so reopening would restore focus to a disabled item. + // Items remount whenever the menu opens, which distinguishes reopening from navigating onto a + // disabled item while the menu is already open. Clearing the cursor lets the roving focus + // fallback in `useListItemRovingFocus` move focus to the first item. The check waits for the + // first render where `id` is known, since items without an explicit `data-id` register it later. + const hasCheckedDisabledCursor = React.useRef(false); + React.useLayoutEffect(() => { + if (hasCheckedDisabledCursor.current || !id) { + return; + } + hasCheckedDisabledCursor.current = true; + + const isItemDisabled = isElementDisabled(localRef.current); + + if (model.state.mode === 'single' && isCursor(model.state, id) && isItemDisabled) { + model.events.goToFirst(); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [id]); + // focus on the item with the cursor React.useLayoutEffect(() => { if (model.state.mode === 'single') { @@ -227,11 +255,12 @@ export const useMenuItem = composeHooks( onClick: model.state.mode === 'single' ? (event: React.SyntheticEvent) => { - // only hide if the item isn't disabled - if (event.currentTarget.getAttribute('aria-disabled') !== 'true') { - model.events.hide(event); - hideParent(model); + if (isElementDisabled(event.currentTarget as Element)) { + return null; } + model.events.hide(event); + hideParent(model); + return undefined; } : undefined, }; diff --git a/modules/react/menu/lib/MenuList.tsx b/modules/react/menu/lib/MenuList.tsx index 7ebdd00e83..00bc36f233 100644 --- a/modules/react/menu/lib/MenuList.tsx +++ b/modules/react/menu/lib/MenuList.tsx @@ -54,12 +54,13 @@ export const MenuList = createSubcomponent('div')({ displayName: 'Menu.List', modelHook: useMenuModel, elemPropsHook: useMenuList, -})(({children, ...elemProps}, Element, model) => { +})(({children, maxHeight, ...elemProps}, Element, model) => { return ( {children} diff --git a/modules/react/menu/lib/Submenu.tsx b/modules/react/menu/lib/Submenu.tsx index 00e7e7b6f7..4360ffe06a 100644 --- a/modules/react/menu/lib/Submenu.tsx +++ b/modules/react/menu/lib/Submenu.tsx @@ -1,6 +1,10 @@ import React from 'react'; -import {useListItemRegister, useListItemRovingFocus} from '@workday/canvas-kit-react/collection'; +import { + isElementDisabled, + useListItemRegister, + useListItemRovingFocus, +} from '@workday/canvas-kit-react/collection'; import { ExtractProps, PropsWithModel, @@ -82,6 +86,23 @@ export const useSubmenuTargetItem = composeHooks( subModelHook(model => (model as any).UNSTABLE_parentModel!, useListItemRegister), createElemPropsHook(useMenuModel)(model => { const currentTargetIdRef = React.useRef(); + // A submenu owns its own `visibility`, so opening a sibling submenu would otherwise leave this + // one open. The parent's cursor always moves to the item being hovered or focused, so treat the + // cursor moving away from our target item as a signal to close. + const parentCursorId = model.UNSTABLE_parentModel.state.cursorId; + const isVisible = model.state.visibility === 'visible'; + React.useEffect(() => { + if (!isVisible) { + return; + } + const targetId = (model.state.targetRef.current as HTMLElement | null)?.getAttribute( + 'data-id' + ); + if (targetId && parentCursorId && parentCursorId !== targetId) { + model.events.hide(); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [parentCursorId, isVisible]); const mouseEnterTimer = useIntentTimer(() => { model.UNSTABLE_parentModel.events.goTo({id: currentTargetIdRef.current || ''}); model.events.show(event); @@ -95,6 +116,9 @@ export const useSubmenuTargetItem = composeHooks( model.UNSTABLE_parentModel.events.goTo({id: event.currentTarget.getAttribute('data-id')!}); }, onMouseEnter(event: React.MouseEvent) { + if (isElementDisabled(event.currentTarget)) { + return; + } currentTargetIdRef.current = event.currentTarget.getAttribute('data-id')!; mouseEnterTimer.start(); }, @@ -102,6 +126,9 @@ export const useSubmenuTargetItem = composeHooks( mouseEnterTimer.clear(); }, onClick(event: React.MouseEvent) { + if (isElementDisabled(event.currentTarget)) { + return null; + } // If we're wrapping a target component that doesn't handle ref forwarding, update the // `state.targetRef` manually. This ensures that custom target components don't need to handle // ref forwarding since ref forwarding is only really needed to programmatically open popups @@ -114,9 +141,13 @@ export const useSubmenuTargetItem = composeHooks( } else { model.events.show(event); } + return undefined; }, 'data-has-children': true, onKeyDown(event: React.KeyboardEvent) { + if (isElementDisabled(event.currentTarget)) { + return; + } if (model.state.orientation === 'vertical') { // eslint-disable-next-line default-case switch (event.key) { diff --git a/modules/react/menu/spec/MenuDisabled.spec.tsx b/modules/react/menu/spec/MenuDisabled.spec.tsx new file mode 100644 index 0000000000..bb3d0d925b --- /dev/null +++ b/modules/react/menu/spec/MenuDisabled.spec.tsx @@ -0,0 +1,232 @@ +import {fireEvent, render, screen, waitFor} from '@testing-library/react'; +import React from 'react'; +import {describe, expect, it, vi} from 'vitest'; + +import {Menu} from '../lib/Menu'; +import {NestedSiblings} from '../stories/examples/NestedSiblings'; + +describe('Menu disabled and submenu bugs', () => { + it('should not fire onSelect when disabled Menu.Item is clicked', async () => { + const onSelect = vi.fn(); + const onClick = vi.fn(); + + render( + + Open Menu + + + + Enabled + + Disabled + + + + + + ); + + fireEvent.click(screen.getByRole('button', {name: 'Open Menu'})); + const disabledItem = await screen.findByRole('menuitem', {name: 'Disabled'}); + + expect(disabledItem).toHaveAttribute('aria-disabled', 'true'); + + fireEvent.click(disabledItem); + + expect(onSelect).not.toHaveBeenCalled(); + expect(onClick).not.toHaveBeenCalled(); + }); + + it('should not open submenu when disabled TargetItem is clicked', async () => { + render( + + Open Menu + + + + + + Submenu + + + + + Sub Item + + + + + + + + + ); + + fireEvent.click(screen.getByRole('button', {name: 'Open Menu'})); + const target = await screen.findByRole('menuitem', {name: 'Submenu'}); + + expect(target).toHaveAttribute('aria-disabled', 'true'); + + fireEvent.click(target); + + expect(screen.queryByRole('menuitem', {name: 'Sub Item'})).not.toBeInTheDocument(); + expect(target).toHaveAttribute('aria-expanded', 'false'); + }); + + it('should not open submenu when TargetItem is disabled via nonInteractiveIds', async () => { + render( + + Open Menu + + + + + Submenu + + + + Sub Item + + + + + + + + + ); + + fireEvent.click(screen.getByRole('button', {name: 'Open Menu'})); + const target = await screen.findByRole('menuitem', {name: 'Submenu'}); + + expect(target).toHaveAttribute('disabled'); + + fireEvent.click(target); + + expect(screen.queryByRole('menuitem', {name: 'Sub Item'})).not.toBeInTheDocument(); + expect(target).toHaveAttribute('aria-expanded', 'false'); + }); + + it('should not fire onSelect when Menu.Item is disabled via nonInteractiveIds', async () => { + const onSelect = vi.fn(); + + render( + + Open Menu + + + + Enabled + Disabled + + + + + ); + + fireEvent.click(screen.getByRole('button', {name: 'Open Menu'})); + const disabledItem = await screen.findByRole('menuitem', {name: 'Disabled'}); + + expect(disabledItem).toHaveAttribute('disabled'); + + fireEvent.click(disabledItem); + + expect(onSelect).not.toHaveBeenCalled(); + expect(screen.getByRole('menu')).toBeInTheDocument(); + }); + + it('should move focus to the first item when reopened while a disabled item holds the cursor', async () => { + render( + + Open Menu + + + + First + Second + + Last + + + + + + ); + + const target = screen.getByRole('button', {name: 'Open Menu'}); + + fireEvent.click(target); + await screen.findByRole('menu'); + + // Arrow up from the first item wraps the cursor onto the disabled last item. + fireEvent.keyDown(screen.getByRole('menuitem', {name: 'First'}), {key: 'ArrowUp'}); + await waitFor(() => { + expect(screen.getByRole('menuitem', {name: 'Last'})).toHaveFocus(); + }); + + fireEvent.keyDown(screen.getByRole('menuitem', {name: 'Last'}), {key: 'Escape'}); + await waitFor(() => { + expect(screen.queryByRole('menu')).not.toBeInTheDocument(); + }); + + fireEvent.click(target); + await screen.findByRole('menu'); + + await waitFor(() => { + expect(screen.getByRole('menuitem', {name: 'First'})).toHaveFocus(); + }); + expect(screen.getByRole('menuitem', {name: 'Last'})).not.toHaveFocus(); + }); + + it('should apply default maxHeight to Menu.List for scrolling', async () => { + render( + + Open Menu + + + + {Array.from({length: 25}, (_, i) => ( + + Item {i} + + ))} + + + + + ); + + fireEvent.click(screen.getByRole('button', {name: 'Open Menu'})); + const menu = await screen.findByRole('menu'); + const scrollContainer = menu.parentElement; + + expect(scrollContainer).toHaveStyle({maxHeight: '100%'}); + expect(scrollContainer).toHaveStyle({overflowY: 'auto'}); + }); + + it('should close sibling submenu when another is opened by click', async () => { + render(); + + fireEvent.click(screen.getByRole('button', {name: 'Open Menu'})); + await screen.findByRole('menu'); + + const secondItem = screen.getByRole('menuitem', {name: 'Second Item'}); + const thirdItem = screen.getByRole('menuitem', {name: 'Third Item'}); + + fireEvent.click(secondItem); + await waitFor(() => { + expect(screen.getByRole('menuitem', {name: 'Second: First Sub Item'})).toBeInTheDocument(); + }); + expect(secondItem).toHaveAttribute('aria-expanded', 'true'); + + fireEvent.click(thirdItem); + await waitFor(() => { + expect( + screen.queryByRole('menuitem', {name: 'Second: First Sub Item'}) + ).not.toBeInTheDocument(); + }); + expect(secondItem).toHaveAttribute('aria-expanded', 'false'); + expect(screen.getByRole('menuitem', {name: 'Third: First Sub Item'})).toBeInTheDocument(); + expect(screen.getAllByRole('menu')).toHaveLength(2); + }); +}); diff --git a/modules/react/menu/stories/Menu.mdx b/modules/react/menu/stories/Menu.mdx index 59db6cae48..ccc36c1015 100644 --- a/modules/react/menu/stories/Menu.mdx +++ b/modules/react/menu/stories/Menu.mdx @@ -38,6 +38,8 @@ yarn add @workday/canvas-kit-react `Menu` will automatically focus on the cursor item (first item by default). The `Menu` uses a menu model which composes a list model and a popup model and sets up accessibility features for you. +> **Note:** By default the menu has a max height of `60vh` where content is clipped and items are scrollable. + ### Context Menu diff --git a/modules/react/menu/stories/Menu.stories.ts b/modules/react/menu/stories/Menu.stories.ts index 6f540d56de..4d6c5ab3e1 100644 --- a/modules/react/menu/stories/Menu.stories.ts +++ b/modules/react/menu/stories/Menu.stories.ts @@ -9,6 +9,7 @@ import {Grouping as GroupingExample} from './examples/Grouping'; import {Icons as IconsExample} from './examples/Icons'; import {Nested as NestedExample} from './examples/Nested'; import {NestedDynamic as NestedDynamicExample} from './examples/NestedDynamic'; +import {NestedSiblings as NestedSiblingsExample} from './examples/NestedSiblings'; import {SelectableMenu as SelectableMenuExample} from './examples/SelectableMenu'; export default { @@ -45,3 +46,6 @@ export const Nested: Story = { export const NestedDynamic: Story = { render: NestedDynamicExample, }; +export const NestedSiblings: Story = { + render: NestedSiblingsExample, +}; diff --git a/modules/react/menu/stories/examples/NestedSiblings.tsx b/modules/react/menu/stories/examples/NestedSiblings.tsx new file mode 100644 index 0000000000..51389194d5 --- /dev/null +++ b/modules/react/menu/stories/examples/NestedSiblings.tsx @@ -0,0 +1,60 @@ +import React from 'react'; + +import {Menu} from '@workday/canvas-kit-react/menu'; +import {BodyText} from '@workday/canvas-kit-react/text'; +import {system} from '@workday/canvas-tokens-web'; + +export const NestedSiblings = () => { + const [selected, setSelected] = React.useState(''); + return ( + { + setSelected(data.id); + }} + > + Open Menu + + + + First Item + + Second Item + + + + + Second: First Sub Item + + + Second: Second Sub Item + + + + + + + Third Item + + + + + Third: First Sub Item + + + Third: Second Sub Item + + + + + + Fourth Item + + + + + Selected: {selected} + + + ); +}; diff --git a/modules/styling/lib/cs.ts b/modules/styling/lib/cs.ts index 895ac4d16c..5c6292a819 100644 --- a/modules/styling/lib/cs.ts +++ b/modules/styling/lib/cs.ts @@ -1061,6 +1061,22 @@ export interface StencilConfig< * ) * } * ``` + * + * To target a part from another stencil's style object, use `.selector`. It is non-enumerable, so + * spreading the part onto an element still only applies `data-part`. + * + * ```ts + * const cardStencil = createStencil({ + * base: { + * [`&:where(:has(${myButtonStencil.parts.icon.selector}))`]: { + * paddingInlineStart: 0, + * }, + * [`& ${myButtonStencil.parts.label.selector}`]: { + * fontWeight: 700, + * }, + * }, + * }) + * ``` */ parts?: P; /** @@ -1301,17 +1317,36 @@ function makeParts>(parts: T): StencilVar }, {} as StencilVarsParts); } +export type StencilPart = { + 'data-part': V; + /** + * CSS selector matching this part (`[data-part="..."]`). Non-enumerable so it is not applied to + * the DOM when the part is spread onto an element. + */ + readonly selector: `[data-part="${V}"]`; +}; + export type StencilPartProps = { - [K in keyof T]: {'data-part': T[K]}; + [K in keyof T]: StencilPart; }; +function makePartProp(value: V): StencilPart { + const part = {'data-part': value} as StencilPart; + // Non-enumerable so `{...stencil.parts.foo}` only spreads `data-part` onto the DOM. + Object.defineProperty(part, 'selector', { + value: `[data-part="${value}"]`, + enumerable: false, + }); + return part; +} + function makePartProps>(parts?: T): StencilPartProps { if (!parts) { return {} as StencilPartProps; } return Object.keys(parts).reduce((result, key: any) => { - (result as any)[key] = {'data-part': parts[key]}; + (result as any)[key] = makePartProp(parts[key]); return result; }, {} as StencilPartProps); } diff --git a/modules/styling/spec/cs.spec.tsx b/modules/styling/spec/cs.spec.tsx index 52932ea05e..677ea77f94 100644 --- a/modules/styling/spec/cs.spec.tsx +++ b/modules/styling/spec/cs.spec.tsx @@ -740,7 +740,10 @@ describe('cs', () => { expectTypeOf(myStencil).toHaveProperty('parts'); expectTypeOf(myStencil.parts).toHaveProperty('separator'); - expectTypeOf(myStencil.parts.separator).toEqualTypeOf<{'data-part': 'my-separator'}>(); + expectTypeOf(myStencil.parts.separator).toEqualTypeOf<{ + 'data-part': 'my-separator'; + readonly selector: '[data-part="my-separator"]'; + }>(); expect(myStencil).toHaveProperty( 'parts.separator.data-part', @@ -748,6 +751,45 @@ describe('cs', () => { ); }); + it('should return a CSS selector on stencil parts', () => { + const myStencil = createStencil({ + parts: { + separator: 'my-separator', + }, + base: {}, + }); + + expectTypeOf( + myStencil.parts.separator.selector + ).toEqualTypeOf<'[data-part="my-separator"]'>(); + expect(myStencil.parts.separator.selector).toEqual('[data-part="my-separator"]'); + }); + + it('should not include selector when spreading part props onto an element', () => { + const myStencil = createStencil({ + parts: { + separator: 'my-separator', + }, + base: {}, + }); + + expect({...myStencil.parts.separator}).toEqual({'data-part': 'my-separator'}); + }); + + it('should not apply selector as a DOM attribute when spreading part props', () => { + const myStencil = createStencil({ + parts: { + separator: 'my-separator', + }, + base: {}, + }); + + render(
); + + expect(screen.getByTestId('part')).toHaveAttribute('data-part', 'my-separator'); + expect(screen.getByTestId('part')).not.toHaveAttribute('selector'); + }); + it('should coerce a variable input to a type of string', () => { const myStencil = createStencil({ vars: { @@ -1330,6 +1372,7 @@ describe('cs', () => { expectTypeOf(extendedStencil.parts).toHaveProperty('separator'); expectTypeOf(extendedStencil.parts.separator).toEqualTypeOf<{ 'data-part': 'base-separator'; + readonly selector: '[data-part="base-separator"]'; }>(); expect(extendedStencil).toHaveProperty( 'parts.separator.data-part', @@ -1341,6 +1384,7 @@ describe('cs', () => { expectTypeOf(extendedStencil.parts).toHaveProperty('border'); expectTypeOf(extendedStencil.parts.border).toEqualTypeOf<{ 'data-part': 'extended-border'; + readonly selector: '[data-part="extended-border"]'; }>(); expect(extendedStencil).toHaveProperty( 'parts.border.data-part', diff --git a/modules/styling/stories/mdx/Stencils.mdx b/modules/styling/stories/mdx/Stencils.mdx index 96300e57ba..59d7cedb35 100644 --- a/modules/styling/stories/mdx/Stencils.mdx +++ b/modules/styling/stories/mdx/Stencils.mdx @@ -408,7 +408,9 @@ To style elements in the render function, we'll need to choose what elements to the example below, we're able to spread the parts directly to elements. The Stencil will generate the type and value most appropriate for the context the part is used. In the Stencil, the part is represented by a string that looks like `[data-part="{partValue}"]` and in the render function, it -is an object that looks like `{'data-part': partValue}`. +is an object that looks like `{'data-part': partValue}`. That object also has a non-enumerable +`selector` property (`[data-part="{partValue}"]`) for targeting the part from another stencil's +style object without writing the attribute selector by hand. ```jsx import {createStencil, handleCsProp} from '@workday/canvas-kit-styling'; @@ -447,6 +449,22 @@ const MyButton = ({children, ...elemProps}) => { }; ``` +To target a part from another stencil, use `.selector`. Spreading the part onto an element still +only applies `data-part`. + +```jsx +const cardStencil = createStencil({ + base: { + [`&:where(:has(${myButtonStencil.parts.icon.selector}))`]: { + paddingInlineStart: 0, + }, + [`& ${myButtonStencil.parts.label.selector}`]: { + fontWeight: 700, + }, + }, +}); +``` + As a reusable component, you can use component parts to style elements that are not exposed in the API. Consumers can also use the type safe Stencil to target that element to style it as well. As a general rule, a Stencil maps to a component. Multiple Stencils per component usually means nested