chore(ui): migrate menu to typescript (#237) (#575)

* 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 <[email protected]> * chore(ui): consistency Co-authored-by: Guoda <[email protected]> * chore(ui): consistency Co-authored-by: Guoda <[email protected]> * chore(ui): use React element type Co-authored-by: Guoda <[email protected]> * chore(ui): make sure href is only passed when element is anchor Co-authored-by: Guoda <[email protected]> * 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 <[email protected]> Co-authored-by: Guoda <[email protected]> Co-authored-by: Wowa Barsukov <[email protected]>
cloudoperators · Nov 7, 2024 · 7cb142d · 7cb142d
1 parent a278544
commit 7cb142d
Show file tree

Hide file tree

Showing 19 changed files with 495 additions and 412 deletions.
diff --git a/.changeset/five-boats-cheat.md 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/...nts/src/components/Menu/Menu.component.js → ...ts/src/components/Menu/Menu.component.tsx b/...nts/src/components/Menu/Menu.component.js → ...ts/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<MenuContextType | undefined>(undefined)
 
 /** A generic menu component */
-export const Menu = ({ children = null, variant = "normal", className = "", ...props }) => {
+export const Menu: React.FC<MenuProps> = ({ children = null, variant = "normal", className = "", ...props }) => {
   return (
     <MenuContext.Provider
       value={{
@@ -48,7 +50,7 @@ export const Menu = ({ children = null, variant = "normal", className = "", ...p
 					juno-menu 
 					juno-menu-${variant} 
 					${baseStyles} 
-					${variantStyles(variant)} 
+					${variant === "small" ? smallStyles : normalStyles} 
 					${className}
 				`}
           role="menu"
@@ -60,12 +62,3 @@ export const Menu = ({ children = null, variant = "normal", className = "", ...p
     </MenuContext.Provider>
   )
 }
-
-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/...nents/src/components/Menu/Menu.stories.js → ...ents/src/components/Menu/Menu.stories.tsx b/...nents/src/components/Menu/Menu.stories.js → ...ents/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 }) => <Menu {...args}>{children}</Menu>
-Template.propTypes = {
-  children: PropTypes.node,
-}
-
 export const Default = {
-  render: Template,
-
   args: {
     children: [
       <MenuItem label="Label only" key="1" />,
@@ -40,13 +33,15 @@ export const Default = {
       <MenuItem label="Item with a Link and icon" href="https://github.com/cloudoperators/juno" icon="help" key="4" />,
       <MenuItem label="Item with OnClick" onClick={() => {}} key="5" />,
       <MenuItem label="Item with OnClick and Icon" onClick={() => {}} icon="deleteForever" key="6" />,
+      <MenuItem key="7">Menu Item with Children</MenuItem>,
+      <MenuItem key="8">
+        <Button label="Delete" size="small" variant="subdued" icon="deleteForever" className="jn-w-full" />
+      </MenuItem>,
     ],
   },
 }
 
 export const Small = {
-  render: Template,
-
   args: {
     variant: "small",
     children: [
@@ -61,8 +56,6 @@ export const Small = {
 }
 
 export const WithASection = {
-  render: Template,
-
   args: {
     children: [
       <MenuSection key="m1">

diff --git a/...mponents/src/components/Menu/Menu.test.js → ...ponents/src/components/Menu/Menu.test.tsx b/...mponents/src/components/Menu/Menu.test.js → ...ponents/src/components/Menu/Menu.test.tsx
@@ -5,16 +5,17 @@
 
 import * as React from "react"
 import { render, screen } from "@testing-library/react"
+import { describe, expect, test } from "vitest"
 import { Menu } from "./index"
 import { MenuItem } from "../MenuItem/index.js"
 
 describe("Menu", () => {
-  test("renders a Menu", async () => {
+  test("renders a Menu", () => {
     render(<Menu />)
     expect(screen.getByRole("menu")).toBeInTheDocument()
   })
 
-  test("renders a Menu with Children as passed", async () => {
+  test("renders a Menu with Children as passed", () => {
     render(
       <Menu>
         <MenuItem label="Menu Item 1" href="#" />
@@ -25,25 +26,25 @@ describe("Menu", () => {
     expect(screen.getByRole("menuitem")).toHaveAttribute("href", "#")
   })
 
-  test("renders a normal Menu variant by default", async () => {
+  test("renders a normal Menu variant by default", () => {
     render(<Menu />)
     expect(screen.getByRole("menu")).toBeInTheDocument()
     expect(screen.getByRole("menu")).toHaveClass("juno-menu-normal")
   })
 
-  test("renders a small Menu variant as passed", async () => {
+  test("renders a small Menu variant as passed", () => {
     render(<Menu variant="small" />)
     expect(screen.getByRole("menu")).toBeInTheDocument()
     expect(screen.getByRole("menu")).toHaveClass("juno-menu-small")
   })
 
-  test("renders a className as passed", async () => {
+  test("renders a className as passed", () => {
     render(<Menu className="my-class" />)
     expect(screen.getByRole("menu")).toBeInTheDocument()
     expect(screen.getByRole("menu")).toHaveClass("my-class")
   })
 
-  test("renders props as passed", async () => {
+  test("renders props as passed", () => {
     render(<Menu data-lolol="1 2 3" />)
     expect(screen.getByRole("menu")).toBeInTheDocument()
     expect(screen.getByRole("menu")).toHaveAttribute("data-lolol", "1 2 3")

diff --git a/...i-components/src/components/Menu/index.js → ...i-components/src/components/Menu/index.ts b/...i-components/src/components/Menu/index.js → ...i-components/src/components/Menu/index.ts
diff --git a/packages/ui-components/src/components/MenuItem/MenuItem.component.js b/packages/ui-components/src/components/MenuItem/MenuItem.component.js
diff --git a/packages/ui-components/src/components/MenuItem/MenuItem.component.tsx b/packages/ui-components/src/components/MenuItem/MenuItem.component.tsx
@@ -0,0 +1,112 @@
+/*
+ * SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Juno contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import React, { useContext, MouseEvent, FC } from "react"
+import { Menu as HLMenu } from "@headlessui/react"
+import { MenuContext } from "../Menu/Menu.component"
+import { Icon, KnownIconsEnum } from "../Icon/Icon.component"
+
+const itemStyles = `
+   jn-text-theme-default
+   jn-flex
+   jn-items-center
+   jn-w-full
+   bg-clip-padding
+   jn-truncate
+   jn-text-left
+   jn-bg-theme-background-lvl-1
+   disabled:jn-cursor-not-allowed
+   data-[headlessui-state="disabled"]:jn-cursor-not-allowed
+ `
+
+const smallStyles = `
+   jn-text-sm
+   jn-p-2
+ `
+
+const normalStyles = `
+   jn-text-base
+   jn-pt-[0.6875rem]
+   jn-pb-[0.5rem]
+   jn-px-[0.875rem]
+ `
+// Define styles applicable to interactive elements only:
+const actionableItemStyles = `
+   hover:jn-bg-theme-background-lvl-3
+   cursor-pointer
+   data-[headlessui-state="disabled"]:jn-bg-theme-background-lvl-3
+ `
+
+export interface MenuItemProps {
+  /** Children of the menu item */
+  children?: React.ReactNode
+  /** Pass a custom className to the menu item */
+  className?: string
+  /** Whether the menu item is disabled */
+  disabled?: boolean
+  /** Pass the name of an icon the button should show. Can be any icon included with Juno. */
+  icon?: keyof typeof KnownIconsEnum
+  /** The label of the menu item. Will take precedence over children passed to the component. */
+  label?: string
+  /** Pass an href to the menu item. Will result in the menu item being rendered as an `<a>`. */
+  href?: string
+  /** Pass an onClick handler to the menu item. Will result in the menu item being rendered as a `<button>`. */
+  // eslint-disable-next-line no-unused-vars
+  onClick?: (event: MouseEvent<HTMLButtonElement>) => void
+}
+
+interface MenuContextType {
+  variant: "small" | "normal"
+}
+
+/** 
+ A menu item to be used inside Menu.
+ Can render `<a>`, `<button>`, or `<div>` based on props.
+ */
+export const MenuItem: FC<MenuItemProps> = ({
+  children = null,
+  className = "",
+  disabled = false,
+  href = "",
+  icon = null,
+  label = "",
+  onClick = undefined,
+  ...props
+}) => {
+  const menuContext = useContext<MenuContextType | undefined>(MenuContext)
+  const variant = menuContext?.variant ?? "normal"
+
+  // Determine which element to render:
+  const Element: React.ElementType = href ? "a" : children ? "div" : onClick ? "button" : "div"
+
+  const handleClick = (event: MouseEvent<HTMLButtonElement>) => {
+    onClick && onClick(event)
+  }
+
+  return (
+    <HLMenu.Item
+      as={Element}
+      // conditionally pass a href attribute only if href was supplied to prevent errors from headless-ui internal checks:
+      {...(href && !disabled && Element === "a" ? { href } : {})}
+      onClick={onClick && Element === "button" && !disabled ? handleClick : undefined}
+      disabled={disabled}
+      className={`
+         juno-menu-item
+         juno-menu-item-${Element}
+         ${itemStyles} 
+         ${Element !== "div" ? actionableItemStyles : ""}
+         ${variant === "small" ? smallStyles : normalStyles}
+         ${disabled ? "jn-cursor-not-allowed" : ""}
+         ${className}
+       `}
+      {...props}
+    >
+      <span className={`${disabled ? "jn-opacity-50" : ""}`}>
+        {icon && <Icon icon={icon} size="18" className="jn-inline-block jn-mr-2" />}
+        {children || label}
+      </span>
+    </HLMenu.Item>
+  )
+}