From 7cb142db10f4d93b0cf1862de6e4dc5805c1f483 Mon Sep 17 00:00:00 2001 From: Franz Heidl Date: Thu, 7 Nov 2024 10:43:48 +0100 Subject: [PATCH] chore(ui): migrate menu to typescript (#237) (#575) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(ui): migrate MenuSection to TypeScript * feat(ui): add more tests * chore(ui): migrate Menu, MenuItem, MenuSection components to TypeScript * chore(ui): prevent headless-ui error * chore(ui): fix button rendering issue detected in story and other tiny adjustments * chore(ui): make tests for rendered element more explicit * Create five-boats-cheat.md * chore(ui): remove type casting when using context don’t cast type when using MenuContext as per Vova’s suggestion * chore(ui): re-introduce changed logic to determine MenuItem element non-intentionally removed this chnage by Guoda * Update .changeset/five-boats-cheat.md Co-authored-by: Guoda <121792659+guoda-puidokaite@users.noreply.github.com> * chore(ui): consistency Co-authored-by: Guoda <121792659+guoda-puidokaite@users.noreply.github.com> * chore(ui): consistency Co-authored-by: Guoda <121792659+guoda-puidokaite@users.noreply.github.com> * chore(ui): use React element type Co-authored-by: Guoda <121792659+guoda-puidokaite@users.noreply.github.com> * chore(ui): make sure href is only passed when element is anchor Co-authored-by: Guoda <121792659+guoda-puidokaite@users.noreply.github.com> * chore(ui): use React.FC to fix typecheck * chore(ui): code formatting * chore(ui): remove file extensions from relevant imports * chore(ui): add more tests * chore(ui): set onClick to null explicitly for standalone stories * chore(ui): remove unnecessary underscores * chore(ui): add more children to story * chore(ui): add back children check to fix link/button issue * chore(ui): remove underscore, eslint-ignore line * chore(ui): apply interactive styles to interactive elements only * chore(ui): disable `children` control in story --------- Co-authored-by: I531348 Co-authored-by: Guoda <121792659+guoda-puidokaite@users.noreply.github.com> Co-authored-by: Wowa Barsukov --- .changeset/five-boats-cheat.md | 5 + .../{Menu.component.js => Menu.component.tsx} | 33 ++-- .../{Menu.stories.js => Menu.stories.tsx} | 23 +-- .../Menu/{Menu.test.js => Menu.test.tsx} | 13 +- .../components/Menu/{index.js => index.ts} | 0 .../components/MenuItem/MenuItem.component.js | 109 ------------ .../MenuItem/MenuItem.component.tsx | 112 ++++++++++++ .../components/MenuItem/MenuItem.stories.js | 107 ------------ .../components/MenuItem/MenuItem.stories.tsx | 107 ++++++++++++ .../src/components/MenuItem/MenuItem.test.js | 80 --------- .../src/components/MenuItem/MenuItem.test.tsx | 159 ++++++++++++++++++ .../MenuItem/{index.js => index.ts} | 0 ...component.js => MenuSection.component.tsx} | 14 +- .../MenuSection/MenuSection.stories.js | 49 ------ .../MenuSection/MenuSection.stories.tsx | 35 ++++ .../MenuSection/MenuSection.test.js | 16 -- .../MenuSection/MenuSection.test.tsx | 37 ++++ .../MenuSection/{index.js => index.ts} | 2 +- packages/ui-components/src/index.js | 6 +- 19 files changed, 495 insertions(+), 412 deletions(-) create mode 100644 .changeset/five-boats-cheat.md rename packages/ui-components/src/components/Menu/{Menu.component.js => Menu.component.tsx} (61%) rename packages/ui-components/src/components/Menu/{Menu.stories.js => Menu.stories.tsx} (85%) rename packages/ui-components/src/components/Menu/{Menu.test.js => Menu.test.tsx} (80%) rename packages/ui-components/src/components/Menu/{index.js => index.ts} (100%) delete mode 100644 packages/ui-components/src/components/MenuItem/MenuItem.component.js create mode 100644 packages/ui-components/src/components/MenuItem/MenuItem.component.tsx delete mode 100644 packages/ui-components/src/components/MenuItem/MenuItem.stories.js create mode 100644 packages/ui-components/src/components/MenuItem/MenuItem.stories.tsx delete mode 100644 packages/ui-components/src/components/MenuItem/MenuItem.test.js create mode 100644 packages/ui-components/src/components/MenuItem/MenuItem.test.tsx rename packages/ui-components/src/components/MenuItem/{index.js => index.ts} (100%) rename packages/ui-components/src/components/MenuSection/{MenuSection.component.js => MenuSection.component.tsx} (68%) delete mode 100644 packages/ui-components/src/components/MenuSection/MenuSection.stories.js create mode 100644 packages/ui-components/src/components/MenuSection/MenuSection.stories.tsx delete mode 100644 packages/ui-components/src/components/MenuSection/MenuSection.test.js create mode 100644 packages/ui-components/src/components/MenuSection/MenuSection.test.tsx rename packages/ui-components/src/components/MenuSection/{index.js => index.ts} (63%) diff --git a/.changeset/five-boats-cheat.md b/.changeset/five-boats-cheat.md new file mode 100644 index 000000000..03b1d1240 --- /dev/null +++ b/.changeset/five-boats-cheat.md @@ -0,0 +1,5 @@ +--- +"@cloudoperators/juno-ui-components": minor +--- + +Migrate Menu, MenuItem and MenuSection components to Typescript (#237) diff --git a/packages/ui-components/src/components/Menu/Menu.component.js b/packages/ui-components/src/components/Menu/Menu.component.tsx similarity index 61% rename from packages/ui-components/src/components/Menu/Menu.component.js rename to packages/ui-components/src/components/Menu/Menu.component.tsx index 6a1257c91..d93dd0463 100644 --- a/packages/ui-components/src/components/Menu/Menu.component.js +++ b/packages/ui-components/src/components/Menu/Menu.component.tsx @@ -4,7 +4,6 @@ */ import React, { createContext } from "react" -import PropTypes from "prop-types" import { Menu as HLMenu } from "@headlessui/react" const baseStyles = ` @@ -23,19 +22,22 @@ const normalStyles = ` jn-text-base ` -const variantStyles = (variant) => { - switch (variant) { - case "small": - return smallStyles - default: - return normalStyles - } +export interface MenuProps { + /** The children to render in the MenuSection */ + children?: React.ReactNode + /** Whether the Menu will be in normal or small variant */ + variant?: "small" | "normal" + /** Pass a custom className to the menu */ + className?: string } -export const MenuContext = createContext() +interface MenuContextType { + variant: "small" | "normal" +} +export const MenuContext = createContext(undefined) /** A generic menu component */ -export const Menu = ({ children = null, variant = "normal", className = "", ...props }) => { +export const Menu: React.FC = ({ children = null, variant = "normal", className = "", ...props }) => { return ( ) } - -Menu.propTypes = { - /* The children of the Menu,, typically MenuItem */ - children: PropTypes.node, - /** Whether the Menu will be in normal or small variant */ - variant: PropTypes.oneOf(["small", "normal"]), - /* Add a className */ - className: PropTypes.string, -} diff --git a/packages/ui-components/src/components/Menu/Menu.stories.js b/packages/ui-components/src/components/Menu/Menu.stories.tsx similarity index 85% rename from packages/ui-components/src/components/Menu/Menu.stories.js rename to packages/ui-components/src/components/Menu/Menu.stories.tsx index b45bcc062..69fbb8c57 100644 --- a/packages/ui-components/src/components/Menu/Menu.stories.js +++ b/packages/ui-components/src/components/Menu/Menu.stories.tsx @@ -4,10 +4,10 @@ */ import React from "react" -import PropTypes from "prop-types" -import { Menu } from "./index.js" -import { MenuItem } from "../MenuItem/index.js" -import { MenuSection } from "../MenuSection/index.js" +import { Menu } from "./" +import { MenuItem } from "../MenuItem/" +import { MenuSection } from "../MenuSection/" +import { Button } from "../Button/" export default { title: "WiP/Menu", @@ -24,14 +24,7 @@ export default { }, } -const Template = ({ children, ...args }) => {children} -Template.propTypes = { - children: PropTypes.node, -} - export const Default = { - render: Template, - args: { children: [ , @@ -40,13 +33,15 @@ export const Default = { , {}} key="5" />, {}} icon="deleteForever" key="6" />, + Menu Item with Children, + + - - - ) - expect(screen.getByRole("button", { name: "Child Button" })).toBeInTheDocument() - }) - - test("executes an onClick handler as passed", async () => { - const onClickSpy = jest.fn() - render( - - - - ) - act(() => screen.getByRole("menuitem").click()) - expect(onClickSpy).toHaveBeenCalled() - }) -}) diff --git a/packages/ui-components/src/components/MenuItem/MenuItem.test.tsx b/packages/ui-components/src/components/MenuItem/MenuItem.test.tsx new file mode 100644 index 000000000..c4386141d --- /dev/null +++ b/packages/ui-components/src/components/MenuItem/MenuItem.test.tsx @@ -0,0 +1,159 @@ +/* + * SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Juno contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as React from "react" +import { render, screen, act } from "@testing-library/react" +import { describe, test, expect, vi } from "vitest" +import { MenuItem } from "./" +import { Menu } from "../Menu/" + +describe("MenuItem", () => { + test("renders a menu item", () => { + render( + + + + ) + expect(screen.getByRole("menuitem")).toBeInTheDocument() + }) + + test("renders an anchor menu item when href prop is passed", () => { + render( + + + + ) + const item = screen.getByRole("menuitem") + expect(item).toBeInTheDocument() + expect(item.tagName).toBe("A") + expect(item).toHaveClass("juno-menu-item") + expect(item).toHaveClass("juno-menu-item-a") + expect(item).not.toHaveClass("juno-menu-item-button") + expect(item).not.toHaveClass("juno-menu-item-div") + }) + + test("renders a button menu item when onClick prop is passed", () => { + render( + + { + console.log("clicked") + }} + /> + + ) + const item = screen.getByRole("menuitem") + expect(item).toBeInTheDocument() + expect(item.tagName).toBe("BUTTON") + expect(item).toHaveClass("juno-menu-item") + expect(item).toHaveClass("juno-menu-item-button") + expect(item).not.toHaveClass("juno-menu-item-a") + expect(item).not.toHaveClass("juno-menu-item-div") + }) + + test("renders a plain div menu item when children are passed", () => { + render( + + + + + + ) + const item = screen.getByRole("menuitem") + expect(item).toBeInTheDocument() + expect(item.tagName).toBe("DIV") + expect(item).toHaveClass("juno-menu-item") + expect(item).toHaveClass("juno-menu-item-div") + expect(item).not.toHaveClass("juno-menu-item-a") + expect(item).not.toHaveClass("juno-menu-item-button") + }) + + test("renders a plain div menu item when neither onClick, href, nor children are passed", () => { + render( + + + + ) + const item = screen.getByRole("menuitem") + expect(item).toBeInTheDocument() + expect(item.tagName).toBe("DIV") + expect(item).toHaveClass("juno-menu-item") + expect(item).toHaveClass("juno-menu-item-div") + expect(item).not.toHaveClass("juno-menu-item-a") + expect(item).not.toHaveClass("juno-menu-item-button") + }) + + test("renders a plain div menu item when label, but neither onClick, href, nor children are passed", () => { + render( + + + + ) + const item = screen.getByRole("menuitem") + expect(item).toBeInTheDocument() + expect(item.tagName).toBe("DIV") + expect(item).toHaveClass("juno-menu-item") + expect(item).toHaveClass("juno-menu-item-div") + expect(item).not.toHaveClass("juno-menu-item-a") + expect(item).not.toHaveClass("juno-menu-item-button") + }) + + test("renders a menu item with an icon as passed", () => { + render( + + + + ) + expect(screen.getByRole("img")).toHaveAttribute("alt", "warning") + }) + + test("renders children as passed", () => { + render( + + + + + + ) + expect(screen.getByRole("button", { name: "Child Button" })).toBeInTheDocument() + }) + + // renders plain div when only label, but no children, no href, and no onClick was passed + + test("renders a label as passed", () => { + render( + + + + ) + expect(screen.getByRole("menuitem")).toBeInTheDocument() + expect(screen.getByRole("menuitem")).toHaveTextContent("some menu item label") + }) + + test("renders children and ignores label when both were passed", () => { + render( + + + + + + ) + expect(screen.getByRole("menuitem")).toBeInTheDocument() + expect(screen.getByRole("button")).toBeInTheDocument() + expect(screen.getByRole("button")).toHaveTextContent("Some Button") + expect(screen.getByRole("menuitem")).not.toHaveTextContent("some menu item label") + }) + + test("executes an onClick handler as passed", () => { + const onClickSpy = vi.fn() // Use `vi.fn()` for mocking + render( + + + + ) + act(() => screen.getByRole("menuitem").click()) + expect(onClickSpy).toHaveBeenCalled() + }) +}) diff --git a/packages/ui-components/src/components/MenuItem/index.js b/packages/ui-components/src/components/MenuItem/index.ts similarity index 100% rename from packages/ui-components/src/components/MenuItem/index.js rename to packages/ui-components/src/components/MenuItem/index.ts diff --git a/packages/ui-components/src/components/MenuSection/MenuSection.component.js b/packages/ui-components/src/components/MenuSection/MenuSection.component.tsx similarity index 68% rename from packages/ui-components/src/components/MenuSection/MenuSection.component.js rename to packages/ui-components/src/components/MenuSection/MenuSection.component.tsx index d469da474..90000fc1f 100644 --- a/packages/ui-components/src/components/MenuSection/MenuSection.component.js +++ b/packages/ui-components/src/components/MenuSection/MenuSection.component.tsx @@ -4,7 +4,6 @@ */ import React from "react" -import PropTypes from "prop-types" const sectionStyles = ` jn-bg-theme-background-lvl-1 @@ -20,7 +19,7 @@ const titleStyles = ` jn-p-2 ` /** Use MenuSection to structure and sub-divide MenuItems in a menu. All but the last MenuSection will render a visible divider at the bottom. Optionally, a MenuSection can have a title.*/ -export const MenuSection = ({ title = "", children = null, className = "", ...props }) => { +export const MenuSection: React.FC = ({ title = "", children = null, className = "", ...props }) => { return (
{title ?
{title}
: ""} @@ -29,8 +28,11 @@ export const MenuSection = ({ title = "", children = null, className = "", ...pr ) } -MenuSection.propTypes = { - title: PropTypes.string, - children: PropTypes.node, - className: PropTypes.string, +export interface MenuSectionProps { + /** The children to render in the MenuSection */ + children?: React.ReactNode + /** Pass a custom className */ + className?: string + /** Pass an optional section title */ + title?: string } diff --git a/packages/ui-components/src/components/MenuSection/MenuSection.stories.js b/packages/ui-components/src/components/MenuSection/MenuSection.stories.js deleted file mode 100644 index 974e13495..000000000 --- a/packages/ui-components/src/components/MenuSection/MenuSection.stories.js +++ /dev/null @@ -1,49 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Juno contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import React from "react" -import PropTypes from "prop-types" -import { MenuSection } from "./index.js" -import { Menu } from "../Menu/index.js" -import { MenuItem } from "../MenuItem/index.js" - -export default { - title: "WiP/Menu/MenuSection", - component: MenuSection, - argTypes: { - items: { - table: { - disable: true, - }, - }, - children: { - control: false, - }, - }, -} - -const Template = ({ children, ...args }) => ( - - {children} - {children} - -) - -Template.propTypes = { - children: PropTypes.array, -} - -export const Default = { - render: Template, - - args: { - title: "Menu Section", - children: [ - Menu Item 1, - Menu Item 2, - Menu Item 3, - ], - }, -} diff --git a/packages/ui-components/src/components/MenuSection/MenuSection.stories.tsx b/packages/ui-components/src/components/MenuSection/MenuSection.stories.tsx new file mode 100644 index 000000000..fcec22db3 --- /dev/null +++ b/packages/ui-components/src/components/MenuSection/MenuSection.stories.tsx @@ -0,0 +1,35 @@ +/* + * SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Juno contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from "react" +import { Menu } from "../Menu/" +import { MenuSection } from "./" +import { MenuItem } from "../MenuItem/" + +type StoryDefinition = () => React.ReactNode + +export default { + title: "WiP/Menu/MenuSection", + component: MenuSection, + argTypes: { + children: { + control: false, + }, + }, + decorators: [(story: StoryDefinition) => {story()}], +} + +export const Default = { + args: { + children: [, ], + }, +} + +export const WithTitle = { + args: { + title: "Menu Section Title", + children: [, ], + }, +} diff --git a/packages/ui-components/src/components/MenuSection/MenuSection.test.js b/packages/ui-components/src/components/MenuSection/MenuSection.test.js deleted file mode 100644 index f604bd208..000000000 --- a/packages/ui-components/src/components/MenuSection/MenuSection.test.js +++ /dev/null @@ -1,16 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Juno contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import * as React from "react" -import { render, screen } from "@testing-library/react" -import { MenuSection } from "./index.js" - -describe("MenuSection", () => { - test("renders a MenuSection", async () => { - render() - expect(screen.getByTestId("menu-section")).toBeInTheDocument() - expect(screen.getByTestId("menu-section")).toHaveClass("juno-menu-section") - }) -}) diff --git a/packages/ui-components/src/components/MenuSection/MenuSection.test.tsx b/packages/ui-components/src/components/MenuSection/MenuSection.test.tsx new file mode 100644 index 000000000..ac89733ea --- /dev/null +++ b/packages/ui-components/src/components/MenuSection/MenuSection.test.tsx @@ -0,0 +1,37 @@ +/* + * SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Juno contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as React from "react" +import { render, screen } from "@testing-library/react" +import { describe, expect, test } from "vitest" + +import { MenuSection } from "./" + +describe("MenuSection", () => { + test("renders a MenuSection", () => { + render() + expect(screen.getByTestId("menu-section")).toBeInTheDocument() + expect(screen.getByTestId("menu-section")).toHaveClass("juno-menu-section") + }) + test("renders a MenuSection with a title as passed", () => { + render() + expect(screen.getByTestId("menu-section")).toBeInTheDocument() + expect(screen.getByTestId("menu-section")).toHaveTextContent("My Menu Section") + }) + test("renders children as passed", () => { + render( + + + + ) + expect(screen.getByRole("button")).toBeInTheDocument() + expect(screen.getByRole("button")).toHaveTextContent("Test Button") + }) + test("renders a custom className as passed", () => { + render() + expect(screen.getByTestId("menu-section")).toBeInTheDocument() + expect(screen.getByTestId("menu-section")).toHaveClass("my-class") + }) +}) diff --git a/packages/ui-components/src/components/MenuSection/index.js b/packages/ui-components/src/components/MenuSection/index.ts similarity index 63% rename from packages/ui-components/src/components/MenuSection/index.js rename to packages/ui-components/src/components/MenuSection/index.ts index 322b61f4a..2f1ab1104 100644 --- a/packages/ui-components/src/components/MenuSection/index.js +++ b/packages/ui-components/src/components/MenuSection/index.ts @@ -3,4 +3,4 @@ * SPDX-License-Identifier: Apache-2.0 */ -export { MenuSection } from "./MenuSection.component" +export { MenuSection, type MenuSectionProps } from "./MenuSection.component" diff --git a/packages/ui-components/src/index.js b/packages/ui-components/src/index.js index 4b18cf042..3a817fc08 100644 --- a/packages/ui-components/src/index.js +++ b/packages/ui-components/src/index.js @@ -57,9 +57,9 @@ export { LoadingIndicator } from "./components/LoadingIndicator" export { MainContainer } from "./components/MainContainer/index.js" export { MainContainerInner } from "./components/MainContainerInner/index.js" export { MainTabs } from "./components/MainTabs/index.js" -export { Menu } from "./components/Menu/index.js" -export { MenuItem } from "./components/MenuItem/index.js" -export { MenuSection } from "./components/MenuSection/index.js" +export { Menu } from "./components/Menu/index" +export { MenuItem } from "./components/MenuItem/index" +export { MenuSection } from "./components/MenuSection/index" export { Message } from "./components/Message" export { Modal } from "./deprecated_js/Modal/index.js" export { ModalFooter } from "./deprecated_js/ModalFooter/index.js"