From 860d9ef9358a4bf5b8c27126c2e0d047d62a7642 Mon Sep 17 00:00:00 2001 From: Juan Andrade Date: Tue, 5 Dec 2023 13:53:36 -0500 Subject: [PATCH] WB-1588: Allow custom ActionItem elements (#2135) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: 1. Allow custom `ActionItem` components by using `Cell` internally. - Removed `ClickableBehavior` from `ActionItem` and replaced it with `CompactCell` (which internally uses `Clickable`). - Removed `skipClientNav` from `ActionItem` as it is no longer used/needed. - Added stories and better documentation for `ActionMenu` and `ActionItem`. 2. Modified Cell to support two new props required for the `ActionItem` changes: - `role` is used to set the `role` attribute on the cell's root element. - `rootStyle` is used to override the `style` attribute on the cell's root element. Issue: WB-1588 ## Test plan: ### ActionMenu: 1. Navigate to http://localhost:6061/?path=/story/dropdown-actionmenu--custom-action-items 2. Verify that the custom action items are rendered as expected. 3. Verify that the custom action items are clickable and that the click handler is called as expected. https://github.com/Khan/wonder-blocks/assets/843075/b4ba22c8-5472-466c-a72a-045c27e22942 ### ActionItem docs: 1. Navigate to http://localhost:6061/?path=/docs/dropdown-actionitem--docs 2. Verify that the documentation for `ActionItem` is correct and up to date. Screenshot 2023-12-05 at 12 52 01 PM Author: jandrade Reviewers: jeresig, jandrade Required Reviewers: Approved By: jeresig Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 16.x), ✅ codecov/project, ✅ Check build sizes (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 2/2), ✅ Lint (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 1/2), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 16.x), ✅ Publish npm snapshot (ubuntu-latest, 16.x), ⏭ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 16.x), ✅ gerald, ⏭ dependabot Pull Request URL: https://github.com/Khan/wonder-blocks/pull/2135 --- .changeset/empty-kids-breathe.md | 9 + .changeset/new-lamps-doubt.md | 9 + .../compact-cell.argtypes.tsx | 36 +++ .../action-item.argtypes.tsx | 109 ++++++++ .../action-item.stories.tsx | 158 ++++++++++++ .../action-menu.stories.tsx | 134 +++++++++- .../phosphor-icon.argtypes.ts | 2 + .../__tests__/clickables.test.tsx | 5 +- .../internal/__tests__/cell-core.test.tsx | 30 +++ .../src/components/internal/cell-core.tsx | 8 +- packages/wonder-blocks-cell/src/util/types.ts | 13 + packages/wonder-blocks-dropdown/package.json | 1 + .../components/__tests__/action-item.test.tsx | 38 ++- .../src/components/action-item.tsx | 244 ++++++++---------- .../tsconfig-build.json | 1 + 15 files changed, 640 insertions(+), 157 deletions(-) create mode 100644 .changeset/empty-kids-breathe.md create mode 100644 .changeset/new-lamps-doubt.md create mode 100644 __docs__/wonder-blocks-dropdown/action-item.argtypes.tsx create mode 100644 __docs__/wonder-blocks-dropdown/action-item.stories.tsx diff --git a/.changeset/empty-kids-breathe.md b/.changeset/empty-kids-breathe.md new file mode 100644 index 000000000..d18694523 --- /dev/null +++ b/.changeset/empty-kids-breathe.md @@ -0,0 +1,9 @@ +--- +"@khanacademy/wonder-blocks-dropdown": major +--- + +Allow custom `ActionItem` components by using Cell internally. + +- Removed `ClickableBehavior` from `ActionItem` and replaced it with + `CompactCell` (which internally uses `Clickable`). +- Removed `skipClientNav` from `ActionItem` as it is no longer used/needed. diff --git a/.changeset/new-lamps-doubt.md b/.changeset/new-lamps-doubt.md new file mode 100644 index 000000000..d67e866e2 --- /dev/null +++ b/.changeset/new-lamps-doubt.md @@ -0,0 +1,9 @@ +--- +"@khanacademy/wonder-blocks-cell": minor +--- + +Add `role` and `rootStyle` props. + +- `role` is used to set the `role` attribute on the cell's root element. +- `rootStyle` is used to override the `style` attribute on the cell's root + element. diff --git a/__docs__/wonder-blocks-cell/compact-cell.argtypes.tsx b/__docs__/wonder-blocks-cell/compact-cell.argtypes.tsx index dd4ecfb07..e9a3f4c6f 100644 --- a/__docs__/wonder-blocks-cell/compact-cell.argtypes.tsx +++ b/__docs__/wonder-blocks-cell/compact-cell.argtypes.tsx @@ -111,6 +111,20 @@ export default { }, }, }, + rootStyle: { + description: + `Optional custom styles applied to the top node.\n\n` + + `**NOTE:** This is the top node of the cell, not the cell ` + + `container. If possible, try to use this prop carefully and use ` + + `\`style\` instead.`, + control: {type: "object"}, + table: { + category: "Styling", + type: { + summary: "StyleType", + }, + }, + }, style: { description: "Optional custom styles.", control: {type: "object"}, @@ -195,4 +209,26 @@ export default { }, }, }, + role: { + description: + "The role of the Cell component, can be a role of type `ClickableRole`", + control: {type: "select"}, + options: [ + "button", + "checkbox", + "link", + "listbox", + "menu", + "menuitem", + "radio", + "tab", + ], + table: { + category: "Accessibility", + type: { + summary: "ClickableRole", + detail: `"button" | "link" | "checkbox" | "radio" | "listbox" | "option" | "menuitem" | "menu" | "tab"`, + }, + }, + }, } satisfies Record; diff --git a/__docs__/wonder-blocks-dropdown/action-item.argtypes.tsx b/__docs__/wonder-blocks-dropdown/action-item.argtypes.tsx new file mode 100644 index 000000000..d9b5faef4 --- /dev/null +++ b/__docs__/wonder-blocks-dropdown/action-item.argtypes.tsx @@ -0,0 +1,109 @@ +import * as React from "react"; +import {PhosphorIcon} from "@khanacademy/wonder-blocks-icon"; +import Pill from "@khanacademy/wonder-blocks-pill"; +import {IconMappings} from "../wonder-blocks-icon/phosphor-icon.argtypes"; + +const AccessoryMappings = { + none: null, + icon: , + pill: New, +}; + +export default { + label: { + control: {type: "text"}, + description: "Display text of the action item.", + table: { + type: {summary: "string"}, + }, + type: {name: "string", required: true}, + }, + disabled: { + control: {type: "boolean"}, + description: "Whether or not the action item is disabled.", + table: { + defaultValue: {summary: false}, + type: {summary: "boolean"}, + }, + type: {name: "boolean", required: true}, + }, + onClick: { + control: {type: null}, + description: + `Callback when the action item is clicked.\n\n` + + `Note: \`onClick\` is optional if \`href\` is present, but must ` + + `be defined if \`href\` is not present.`, + table: { + type: {summary: "() => void"}, + }, + type: {name: "function", required: false}, + }, + href: { + control: {type: "text"}, + description: + `URL to navigate to when the action item is clicked.\n\n` + + `Note: \`href\` must be defined if \`onClick\` is not present.`, + + table: { + type: {summary: "string"}, + }, + type: {name: "string", required: false}, + }, + target: { + control: {type: "text"}, + description: "A target destination window for a link to open in.", + table: { + type: {summary: "string"}, + }, + type: {name: "string", required: false}, + }, + lang: { + control: {type: "text"}, + description: + `Optional attribute to indicate to the Screen Reader which ` + + `language the item text is in.`, + table: { + type: {summary: "string"}, + }, + type: {name: "string", required: false}, + }, + testId: { + control: {type: "text"}, + description: "Test ID used for e2e testing.", + table: { + type: {summary: "string"}, + }, + type: {name: "string", required: false}, + }, + horizontalRule: { + options: ["none", "inset", "full-width"], + control: {type: "select"}, + description: "Adds a horizontal rule at the bottom of the action item.", + defaultValue: "none", + table: { + defaultValue: {summary: "none"}, + type: {summary: `"none" | "inset" | "full-width"`}, + }, + type: {name: `"none" | "inset" | "full-width"`, required: false}, + }, + leftAccessory: { + options: Object.keys(AccessoryMappings) as Array, + mapping: AccessoryMappings, + control: {type: "select"}, + description: "Adds an accessory to the left of the action item.", + table: { + type: {summary: "React.Node"}, + }, + type: {name: "React.Node", required: false}, + }, + rightAccessory: { + options: Object.keys(AccessoryMappings) as Array, + mapping: AccessoryMappings, + control: {type: "select"}, + description: "Adds an accessory to the right of the action item.", + table: { + type: {summary: "React.Node"}, + }, + type: {name: "React.Node", required: false}, + }, +}; diff --git a/__docs__/wonder-blocks-dropdown/action-item.stories.tsx b/__docs__/wonder-blocks-dropdown/action-item.stories.tsx new file mode 100644 index 000000000..a2d14a396 --- /dev/null +++ b/__docs__/wonder-blocks-dropdown/action-item.stories.tsx @@ -0,0 +1,158 @@ +import {Meta} from "@storybook/react"; +import * as React from "react"; +import {StyleSheet} from "aphrodite"; +import Color from "@khanacademy/wonder-blocks-color"; +import {PropsFor, View} from "@khanacademy/wonder-blocks-core"; +import {ActionItem} from "@khanacademy/wonder-blocks-dropdown"; +import {PhosphorIcon} from "@khanacademy/wonder-blocks-icon"; +import Spacing from "@khanacademy/wonder-blocks-spacing"; + +import ComponentInfo from "../../.storybook/components/component-info"; +import packageConfig from "../../packages/wonder-blocks-dropdown/package.json"; +import {IconMappings} from "../wonder-blocks-icon/phosphor-icon.argtypes"; +import actionItemArgtypes from "./action-item.argtypes"; + +const defaultArgs = { + label: "Action Item", + onClick: () => {}, + disabled: false, + testId: "", + lang: "", + role: "menuitem", + style: {}, + horizontalRule: "none", + leftAccessory: null, + rightAccessory: null, +}; + +const styles = StyleSheet.create({ + example: { + background: Color.offWhite, + padding: Spacing.medium_16, + width: 300, + }, + items: { + background: Color.white, + }, +}); + +/** + * The action item trigger actions, such as navigating to a different page or + * opening a modal. Supply the `href` and/or `onClick` props. This component is + * as a child of `ActionMenu`. + * + * ### Usage + * + * ```tsx + * import {ActionItem, ActionMenu} from "@khanacademy/wonder-blocks-dropdown"; + * + * + * {}} /> + * + * ``` + */ +export default { + title: "Dropdown/ActionItem", + component: ActionItem, + argTypes: actionItemArgtypes, + args: defaultArgs, + decorators: [ + (Story): React.ReactElement> => ( + + + + ), + ], + parameters: { + componentSubtitle: ( + + ), + }, +} as Meta; + +/** + * The default action item with a `label` and an `onClick` handler. This is used + * to trigger actions, such as opening a modal. + */ +export const Default = { + args: { + label: "Action Item", + onClick: () => {}, + }, +}; + +/** + * The action item with a `label` and an `href` prop. This is used to trigger + * navigation to a different page. + */ +export const WithHref = { + args: { + label: "Action Item", + href: "https://khanacademy.org", + }, + parameters: { + chromatic: { + // Disabling because this doesn't test anything visual. + disableSnapshot: true, + }, + }, +}; + +/** + * ActionItem can be `disabled`. This is used to indicate that the action is not + * available. + */ +export const Disabled = { + args: { + label: "Action Item", + onClick: () => {}, + disabled: true, + }, +}; + +/** + * ActionItem can have more complex content, such as icons. + * + * This can be done by passing in a `leftAccessory` and/or `rightAccessory` + * prop. These can be any React node, and internally use the WB Cell component + * to render. + */ +export const CustomActionItem = { + args: { + label: "Action Item", + onClick: () => {}, + leftAccessory: ( + + ), + rightAccessory: ( + + ), + }, +}; + +/** + * `horizontalRule` can be used to separate items within ActionMenu instances. + * It defaults to `none`, but can be set to `inset` or `full-width` to add a + * horizontal rule at the bottom of the cell. + */ +export const HorizontalRule = { + args: { + label: "Action Item", + onClick: () => {}, + }, + render: (args: PropsFor): React.ReactNode => ( + + + + + + + ), +}; diff --git a/__docs__/wonder-blocks-dropdown/action-menu.stories.tsx b/__docs__/wonder-blocks-dropdown/action-menu.stories.tsx index fe1ad3daa..c55fa3c92 100644 --- a/__docs__/wonder-blocks-dropdown/action-menu.stories.tsx +++ b/__docs__/wonder-blocks-dropdown/action-menu.stories.tsx @@ -1,11 +1,17 @@ +import {expect} from "@storybook/jest"; /* eslint-disable no-console */ import * as React from "react"; import {StyleSheet} from "aphrodite"; import type {Meta, StoryObj} from "@storybook/react"; +import {useArgs} from "@storybook/preview-api"; +import {action} from "@storybook/addon-actions"; +import {userEvent, within} from "@storybook/testing-library"; import Color from "@khanacademy/wonder-blocks-color"; import {View} from "@khanacademy/wonder-blocks-core"; import {Checkbox} from "@khanacademy/wonder-blocks-form"; +import {PhosphorIcon} from "@khanacademy/wonder-blocks-icon"; +import Pill from "@khanacademy/wonder-blocks-pill"; import Spacing from "@khanacademy/wonder-blocks-spacing"; import {LabelLarge} from "@khanacademy/wonder-blocks-typography"; import { @@ -15,6 +21,7 @@ import { SeparatorItem, } from "@khanacademy/wonder-blocks-dropdown"; +import {IconMappings} from "../wonder-blocks-icon/phosphor-icon.argtypes"; import actionMenuArgtypes from "./action-menu.argtypes"; import ComponentInfo from "../../.storybook/components/component-info"; import packageConfig from "../../packages/wonder-blocks-dropdown/package.json"; @@ -23,35 +30,41 @@ import type {Item} from "../../packages/wonder-blocks-dropdown/src/util/types"; const actionItems: Array = [ , , console.log("user clicked on settings")} testId="settings" />, console.log("this item is disabled...")} testId="help" />, , - , + , ), - docs: { - description: { - component: null, - }, - source: { - // See https://github.com/storybookjs/storybook/issues/12596 - excludeDecorators: true, - }, - }, }, } as Meta; @@ -109,6 +113,9 @@ const styles = StyleSheet.create({ background: Color.offWhite, padding: Spacing.medium_16, }, + exampleExtended: { + height: 300, + }, rowRight: { flexDirection: "row", justifyContent: "flex-end", @@ -431,3 +438,110 @@ const locales = [ {id: "fr", locale: "fr", localName: "français"}, {id: "it", locale: "it", localName: "italiano"}, ]; + +/** + * ActionMenu can be used with custom action items. This is useful when you + * want to use more rich action items, such as the ones used in context menus. + * + * ActionItem internally uses the `CompactCell` component, which is a component + * that allows you to pass left and right accessories. + */ +export const CustomActionItems: StoryComponentType = { + args: { + menuText: "Custom Action Items", + children: [ + + } + onClick={action("Set date clicked")} + />, + + } + onClick={action("Edit clicked!")} + />, + + } + onClick={action("preferences clicked!")} + />, + User profile} + horizontalRule="full-width" + leftAccessory={ + + } + rightAccessory={ + + New + + } + onClick={action("user profile clicked!")} + style={{ + [":hover [data-test-id=new-pill]" as any]: { + backgroundColor: Color.white, + color: Color.blue, + }, + }} + />, + console.log(`Show homework assignments toggled`)} + />, + ], + } as Partial, + render: function Render(args) { + const [{selectedValues}, updateArgs] = useArgs(); + const handleChange = (selectedItems: Array) => { + updateArgs({selectedValues: selectedItems}); + }; + + return ( + + ); + }, + decorators: [ + (Story): React.ReactElement> => ( + + + + ), + ], + play: async ({canvasElement}) => { + // Arrange + // NOTE: Using `body` here to work with React Portals. + const canvas = within(canvasElement.ownerDocument.body); + + // Act + await userEvent.click(canvas.getByRole("button")); + + // Assert + const actionMenu = await canvas.findByRole("menu"); + expect(actionMenu).toBeInTheDocument(); + }, +}; diff --git a/__docs__/wonder-blocks-icon/phosphor-icon.argtypes.ts b/__docs__/wonder-blocks-icon/phosphor-icon.argtypes.ts index 90bf0c6d4..1f2c55be3 100644 --- a/__docs__/wonder-blocks-icon/phosphor-icon.argtypes.ts +++ b/__docs__/wonder-blocks-icon/phosphor-icon.argtypes.ts @@ -46,6 +46,7 @@ import play from "@phosphor-icons/core/regular/play.svg"; import playBold from "@phosphor-icons/core/bold/play-bold.svg"; import playCircle from "@phosphor-icons/core/regular/play-circle.svg"; import playCircleBold from "@phosphor-icons/core/bold/play-circle-bold.svg"; +import gear from "@phosphor-icons/core/regular/gear.svg"; import {tokens} from "@khanacademy/wonder-blocks-theming"; @@ -99,6 +100,7 @@ export const IconMappings = { playBold, playCircle, playCircleBold, + gear, } as const; export default { diff --git a/consistency-tests/__tests__/clickables.test.tsx b/consistency-tests/__tests__/clickables.test.tsx index 0576cc894..2aa4085b5 100644 --- a/consistency-tests/__tests__/clickables.test.tsx +++ b/consistency-tests/__tests__/clickables.test.tsx @@ -10,7 +10,7 @@ import {render, screen} from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import plus from "@phosphor-icons/core/regular/plus.svg"; -import {ActionItem, OptionItem} from "@khanacademy/wonder-blocks-dropdown"; +import {OptionItem} from "@khanacademy/wonder-blocks-dropdown"; import Button from "@khanacademy/wonder-blocks-button"; import Clickable from "@khanacademy/wonder-blocks-clickable"; import { @@ -37,7 +37,6 @@ const IconButtonWrapper = (props: any) => ; describe.each` Component | name | role - ${ActionItem} | ${"ActionItem"} | ${"menuitem"} ${Button} | ${"Button"} | ${"button"} ${ClickableWrapper} | ${"Clickable"} | ${"link"} ${CompactCell} | ${"CompactCell"} | ${"link"} @@ -112,7 +111,6 @@ describe.each` // NOTE: Link doesn't work without an href so it isn't included in this suite describe.each` Component | name | role - ${ActionItem} | ${"ActionItem"} | ${"menuitem"} ${Button} | ${"Button"} | ${"button"} ${ClickableWrapper} | ${"Clickable"} | ${"button"} ${CompactCell} | ${"CompactCell"} | ${"button"} @@ -171,7 +169,6 @@ describe.each` // have an added tabIndex of 0. describe.each` Component | name | hasTabIndex - ${ActionItem} | ${"ActionItem"} | ${false} ${Button} | ${"Button"} | ${false} ${ClickableWrapper} | ${"Clickable"} | ${false} ${CompactCell} | ${"CompactCell"} | ${false} diff --git a/packages/wonder-blocks-cell/src/components/internal/__tests__/cell-core.test.tsx b/packages/wonder-blocks-cell/src/components/internal/__tests__/cell-core.test.tsx index 28d8cc37e..a793416f4 100644 --- a/packages/wonder-blocks-cell/src/components/internal/__tests__/cell-core.test.tsx +++ b/packages/wonder-blocks-cell/src/components/internal/__tests__/cell-core.test.tsx @@ -108,4 +108,34 @@ describe("CellCore", () => { // eslint-disable-next-line testing-library/no-node-access expect(container.firstChild).toHaveAttribute("aria-current", "true"); }); + + it("should allow passing a role", () => { + // Arrange + + // Act + render( + +
cell core content
+ , + ); + + // Assert + expect( + screen.getByRole("menuitem", {name: "cell core content"}), + ).toBeInTheDocument(); + }); + + it("should pass an style to the top node", () => { + // Arrange + + // Act + render( + +
cell core content
+ , + ); + + // Assert + expect(screen.getByRole("button")).toHaveStyle("color: blue"); + }); }); diff --git a/packages/wonder-blocks-cell/src/components/internal/cell-core.tsx b/packages/wonder-blocks-cell/src/components/internal/cell-core.tsx index 11a776ce0..b269b1a22 100644 --- a/packages/wonder-blocks-cell/src/components/internal/cell-core.tsx +++ b/packages/wonder-blocks-cell/src/components/internal/cell-core.tsx @@ -173,6 +173,8 @@ const CellCore = (props: CellCoreProps): React.ReactElement => { onClick, "aria-label": ariaLabel, target, + role, + rootStyle, } = props; // Pressable cell. @@ -185,10 +187,12 @@ const CellCore = (props: CellCoreProps): React.ReactElement => { href={href} hideDefaultFocusRing={true} aria-label={ariaLabel ? ariaLabel : undefined} + role={role} target={target} style={[ styles.wrapper, styles.clickable, + rootStyle, active && styles.active, disabled && styles.disabled, ]} @@ -203,8 +207,9 @@ const CellCore = (props: CellCoreProps): React.ReactElement => { // wrapper. return ( @@ -218,6 +223,7 @@ const styles = StyleSheet.create({ display: "flex", minHeight: CellMeasurements.cellMinHeight, textAlign: "left", + width: "100%", }, innerWrapper: { diff --git a/packages/wonder-blocks-cell/src/util/types.ts b/packages/wonder-blocks-cell/src/util/types.ts index 910534732..6e8a881dc 100644 --- a/packages/wonder-blocks-cell/src/util/types.ts +++ b/packages/wonder-blocks-cell/src/util/types.ts @@ -2,6 +2,7 @@ import * as React from "react"; import type {StyleType} from "@khanacademy/wonder-blocks-core"; import type {Typography} from "@khanacademy/wonder-blocks-typography"; +import {ClickableRole} from "@khanacademy/wonder-blocks-clickable"; /** * A set of values that can be used to configure the horizontal rule appearance. @@ -87,6 +88,18 @@ export type CellProps = { * separate cells within groups such as lists. Defaults to `inset`. */ horizontalRule?: HorizontalRuleVariant; + + /** + * A custom role for the cell. + */ + role?: ClickableRole; + /** + * Optional custom styles applied to the top node. + * + * _NOTE:_ This is the top node of the cell, not the cell container. If + * possible, try to use this prop carefully and use `style` instead. + */ + rootStyle?: StyleType; /** * Optional custom styles applied to the cell container. */ diff --git a/packages/wonder-blocks-dropdown/package.json b/packages/wonder-blocks-dropdown/package.json index 5742864c6..696812667 100644 --- a/packages/wonder-blocks-dropdown/package.json +++ b/packages/wonder-blocks-dropdown/package.json @@ -16,6 +16,7 @@ }, "dependencies": { "@babel/runtime": "^7.18.6", + "@khanacademy/wonder-blocks-cell": "^3.0.31", "@khanacademy/wonder-blocks-clickable": "^4.0.11", "@khanacademy/wonder-blocks-color": "^3.0.0", "@khanacademy/wonder-blocks-core": "^6.3.0", diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/action-item.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/action-item.test.tsx index 359ab1d04..7ea6ec734 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/action-item.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/action-item.test.tsx @@ -2,6 +2,10 @@ import * as React from "react"; import {render, screen} from "@testing-library/react"; import * as ReactRouterDOM from "react-router-dom"; +import {HeadingSmall} from "@khanacademy/wonder-blocks-typography"; +import {PhosphorIcon} from "@khanacademy/wonder-blocks-icon"; + +import plusIcon from "@phosphor-icons/core/regular/plus.svg"; import ActionItem from "../action-item"; jest.mock("react-router-dom", () => ({ @@ -18,7 +22,10 @@ describe("ActionItem", () => { render(); // Assert - expect(screen.getByRole("menuitem")).toBeDisabled(); + expect(screen.getByRole("menuitem")).toHaveAttribute( + "aria-disabled", + "true", + ); }); it("should render an anchor if there's no router", () => { @@ -56,4 +63,33 @@ describe("ActionItem", () => { // Assert expect(screen.getByText("Español")).toHaveAttribute("lang", "es"); }); + + it("should allow passing an accessory", () => { + // Arrange + + // Act + render( + } + />, + ); + + // Assert + expect(screen.getByRole("img")).toBeInTheDocument(); + }); + + it("should allow passing a custom label", () => { + // Arrange + + // Act + render( + A heading as an item} + />, + ); + + // Assert + expect(screen.getByRole("heading")).toBeInTheDocument(); + }); }); diff --git a/packages/wonder-blocks-dropdown/src/components/action-item.tsx b/packages/wonder-blocks-dropdown/src/components/action-item.tsx index f8560d6ff..cacc2cdb2 100644 --- a/packages/wonder-blocks-dropdown/src/components/action-item.tsx +++ b/packages/wonder-blocks-dropdown/src/components/action-item.tsx @@ -1,28 +1,24 @@ import * as React from "react"; import {StyleSheet} from "aphrodite"; -import {Link} from "react-router-dom"; -import {__RouterContext} from "react-router"; +import {CompactCell} from "@khanacademy/wonder-blocks-cell"; import Color, {mix, fade} from "@khanacademy/wonder-blocks-color"; import Spacing from "@khanacademy/wonder-blocks-spacing"; import {LabelMedium} from "@khanacademy/wonder-blocks-typography"; -import { - getClickableBehavior, - isClientSideUrl, -} from "@khanacademy/wonder-blocks-clickable"; -import {addStyle} from "@khanacademy/wonder-blocks-core"; -import type {StyleType} from "@khanacademy/wonder-blocks-core"; +import type {PropsFor, StyleType} from "@khanacademy/wonder-blocks-core"; import {DROPDOWN_ITEM_HEIGHT} from "../util/constants"; const {blue, white, offBlack, offBlack32} = Color; +type CompactCellProps = PropsFor; + type ActionProps = { /** * Display text of the action item. */ - label: string; + label: string | CompactCellProps["title"]; /** * Whether this action item is disabled. */ @@ -41,22 +37,9 @@ type ActionProps = { /** * A target destination window for a link to open in. * - * TODO(WB-1262): only allow this prop when `href` is also set.t + * TODO(WB-1262): only allow this prop when `href` is also set. */ target?: "_blank"; - /** - * Whether to avoid using client-side navigation. - * - * If the URL passed to href is local to the client-side, e.g. - * /math/algebra/eval-exprs, then it tries to use react-router-dom's Link - * component which handles the client-side navigation. You can set - * `skipClientNav` to true avoid using client-side nav entirely. - * - * NOTE: All URLs containing a protocol are considered external, e.g. - * https://khanacademy.org/math/algebra/eval-exprs will trigger a full - * page reload. - */ - skipClientNav?: boolean; /** * Test ID used for e2e testing. */ @@ -91,18 +74,35 @@ type ActionProps = { * @ignore */ style?: StyleType; + + /** + * Inherited from WB Cell. + */ + + /** + * Adds a horizontal rule at the bottom of the cell that can be used to + * separate items within ActionMenu instances. Defaults to `none`. + */ + horizontalRule?: CompactCellProps["horizontalRule"]; + + /** + * Optional left accessory to display in the `ActionItem` element. + */ + leftAccessory?: CompactCellProps["leftAccessory"]; + + /** + * Optional right accessory to display in the `ActionItem` element. + */ + rightAccessory?: CompactCellProps["rightAccessory"]; }; type DefaultProps = { disabled: ActionProps["disabled"]; + horizontalRule: ActionProps["horizontalRule"]; indent: ActionProps["indent"]; role: ActionProps["role"]; }; -const StyledAnchor = addStyle("a"); -const StyledButton = addStyle("button"); -const StyledLink = addStyle(Link); - /** * The action item trigger actions, such as navigating to a different page or * opening a modal. Supply the href and/or onClick props. Used as a child of @@ -115,131 +115,110 @@ export default class ActionItem extends React.Component { } static defaultProps: DefaultProps = { disabled: false, + horizontalRule: "none", indent: false, role: "menuitem", }; static __IS_ACTION_ITEM__ = true; - renderClickableBehavior(router: any): React.ReactNode { + render(): React.ReactNode { const { - skipClientNav, disabled, + horizontalRule, href, target, indent, label, lang, + leftAccessory, + rightAccessory, onClick, role, style, testId, } = this.props; - const ClickableBehavior = getClickableBehavior( - href, - skipClientNav, - router, - ); + const defaultStyle = [ + styles.wrapper, + // pass optional styles from react-window (if applies) + style, + ]; + + const labelComponent = + typeof label === "string" ? ( + + {label} + + ) : ( + React.cloneElement(label, { + lang, + style: styles.label, + ...label.props, + }) + ); return ( - - {(state, childrenProps) => { - const {pressed, hovered, focused} = state; - - const defaultStyle = [ - styles.shared, - disabled && styles.disabled, - !disabled && - (pressed - ? styles.active - : (hovered || focused) && styles.focus), - // pass optional styles from react-window (if applies) - style, - ]; - - const props = { - "data-test-id": testId, - disabled, - role, - style: [defaultStyle], - ...childrenProps, - } as const; - - const children = ( - - - {label} - - - ); - - if (href && !disabled) { - return router && - !skipClientNav && - isClientSideUrl(href) ? ( - - {children} - - ) : ( - - {children} - - ); - } else { - return ( - - {children} - - ); - } - }} - - ); - } - - render(): React.ReactNode { - return ( - <__RouterContext.Consumer> - {(router) => this.renderClickableBehavior(router)} - + onClick={onClick} + /> ); } } const styles = StyleSheet.create({ - shared: { - background: white, - color: offBlack, - textDecoration: "none", - border: "none", - outline: "none", - flexDirection: "row", - alignItems: "center", - display: "flex", - height: DROPDOWN_ITEM_HEIGHT, + wrapper: { minHeight: DROPDOWN_ITEM_HEIGHT, - paddingLeft: Spacing.medium_16, - paddingRight: Spacing.medium_16, - // This removes the 300ms click delay on mobile browsers by indicating that - // "double-tap to zoom" shouldn't be used on this element. + // This removes the 300ms click delay on mobile browsers by indicating + // that "double-tap to zoom" shouldn't be used on this element. touchAction: "manipulation", + + /** + * States + */ + ":focus": { + // Override the default focus state for the cell element, so that it + // can be added programmatically to the button element. + borderRadius: Spacing.xxxSmall_4, + outline: `${Spacing.xxxxSmall_2}px solid ${Color.blue}`, + outlineOffset: -Spacing.xxxxSmall_2, + }, + + // Overrides the default cell state for the button element. + [":hover[aria-disabled=false]" as any]: { + color: white, + background: blue, + }, + // Allow hover styles on non-touch devices only. This prevents an + // issue with hover being sticky on touch devices (e.g. mobile). + ["@media not (hover: hover)" as any]: { + // Revert the hover styles to the default/resting state (mobile + // only). + [":hover[aria-disabled=false]" as any]: { + color: white, + background: offBlack, + }, + }, + + // active and pressed states + [":active[aria-disabled=false]" as any]: { + color: mix(fade(blue, 0.32), white), + background: mix(offBlack32, blue), + }, + }, + shared: { + minHeight: DROPDOWN_ITEM_HEIGHT, + height: DROPDOWN_ITEM_HEIGHT, }, label: { @@ -248,24 +227,7 @@ const styles = StyleSheet.create({ }, indent: { - marginLeft: Spacing.medium_16, - }, - - // hover and focus states - focus: { - color: white, - background: blue, - }, - - // active and pressed states - active: { - color: mix(fade(blue, 0.32), white), - background: mix(offBlack32, blue), - }, - - // disabled state - disabled: { - color: offBlack32, - cursor: "default", + // Cell's internal padding + checkbox width + checkbox margin + paddingLeft: Spacing.medium_16 * 2, }, }); diff --git a/packages/wonder-blocks-dropdown/tsconfig-build.json b/packages/wonder-blocks-dropdown/tsconfig-build.json index 9d22293ef..cab0f6bf1 100644 --- a/packages/wonder-blocks-dropdown/tsconfig-build.json +++ b/packages/wonder-blocks-dropdown/tsconfig-build.json @@ -6,6 +6,7 @@ "rootDir": "src", }, "references": [ + {"path": "../wonder-blocks-cell/tsconfig-build.json"}, {"path": "../wonder-blocks-clickable/tsconfig-build.json"}, {"path": "../wonder-blocks-color/tsconfig-build.json"}, {"path": "../wonder-blocks-core/tsconfig-build.json"},