feat: replace deprecated nav components (#1492)

Relates to #1477

refactor navigation components

- `OnyxFlyoutMenu`: rename default slot to options
- previous `OnyxNavItem` component is now `OnyxNavButton` with changed
API
- new `OnyxNavItem` is now only intended to be used as children for the
new `OnyxNavButton`
- `OnyxUserMenu`: property `avatar` now only accepts a string, not an
object
- fix(OnyxUserMenu): use correct font weight for username

.changeset/loud-radios-pay.md

@@ -0,0 +1,11 @@
+---
+"sit-onyx": major
+---
+
+refactor navigation components
+
+- `OnyxFlyoutMenu`: rename default slot to options
+- previous `OnyxNavItem` component is now `OnyxNavButton` with changed API
+- new `OnyxNavItem` is now only intended to be used as children for the new `OnyxNavButton`
+- `OnyxUserMenu`: property `avatar` now only accepts a string, not an object
+- fix(OnyxUserMenu): use correct font weight for username

.changeset/mighty-icons-learn.md

@@ -0,0 +1,7 @@
+---
+"sit-onyx": patch
+---
+
+fix(OnyxMenuItem): make whole button/link clickable
+
+Also add missing export for the component itself

apps/alpha-test-app/src/App.vue

@@ -5,12 +5,12 @@ import {
   OnyxAppLayout,
   OnyxColorSchemeMenuItem,
   OnyxIcon,
-  OnyxListItem,
+  OnyxMenuItem,
   OnyxNavBar,
-  OnyxNavItem,
+  OnyxNavButton,
   OnyxToastProvider,
   OnyxUserMenu,
-  type OnyxNavItemProps,
+  type OnyxNavButtonProps,
 } from "sit-onyx";
 import { ref, watch } from "vue";
 import { RouterView, useRoute, useRouter } from "vue-router";
@@ -26,7 +26,7 @@ const navItems = [
   { label: "Form Demo", href: "/form-demo" },
   { label: "Layout Demo", href: "/layout-demo" },
   { label: "Grid Demo", href: "/grid" },
-] satisfies OnyxNavItemProps[];
+] satisfies OnyxNavButtonProps[];
 
 const { store: colorScheme } = useColorMode();
 
@@ -64,7 +64,7 @@ watch(
         @back-button-click="router.back"
         @app-area-click="router.push('/')"
       >
-        <OnyxNavItem
+        <OnyxNavButton
           v-for="item in navItems"
           :key="item.href"
           v-bind="item"
@@ -76,10 +76,10 @@ watch(
           <OnyxUserMenu username="John Doe">
             <OnyxColorSchemeMenuItem v-model="colorScheme" />
 
-            <OnyxListItem color="danger">
+            <OnyxMenuItem color="danger">
               <OnyxIcon :icon="logout" />
               Logout
-            </OnyxListItem>
+            </OnyxMenuItem>
 
             <template #footer>
               App Version

packages/headless/src/composables/menuButton/createMenuButton.ts

@@ -112,6 +112,14 @@ export const createMenuButton = createBuilder(() => {
         "aria-labelledby": buttonId,
         onKeydown: handleKeydown,
       },
+      ...createMenuItem({}).elements,
+    },
+  };
+});
+
+export const createMenuItem = createBuilder(() => {
+  return {
+    elements: {
       listItem: {
         role: "none",
       },

packages/sit-onyx/src/components/OnyxExternalLinkIcon/OnyxExternalLinkIcon.stories.ts

@@ -0,0 +1,32 @@
+import { defineStorybookActionsAndVModels } from "@sit-onyx/storybook-utils";
+import type { Meta, StoryObj } from "@storybook/vue3";
+import { textColorDecorator } from "../../utils/storybook";
+import OnyxExternalLinkIcon from "./OnyxExternalLinkIcon.vue";
+
+/**
+ * Icon for external links. Will be hidden if the link is not external or icon is disabled.
+ * Mainly used internally inside other components like the OnyxLink, OnyxNavButton etc.
+ */
+const meta: Meta<typeof OnyxExternalLinkIcon> = {
+  title: "Support/ExternalLinkIcon",
+  ...defineStorybookActionsAndVModels({
+    component: OnyxExternalLinkIcon,
+    events: [],
+    argTypes: {
+      withExternalIcon: {
+        options: ["auto", true, false],
+        control: { type: "radio" },
+      },
+    },
+    decorators: [textColorDecorator],
+  }),
+};
+
+export default meta;
+type Story = StoryObj<typeof OnyxExternalLinkIcon>;
+
+export const Default = {
+  args: {
+    href: "https://onyx.schwarz",
+  },
+} satisfies Story;

packages/sit-onyx/src/components/OnyxExternalLinkIcon/OnyxExternalLinkIcon.vue

@@ -0,0 +1,36 @@
+<script lang="ts" setup>
+import arrowSmallUpRight from "@sit-onyx/icons/arrow-small-up-right.svg?raw";
+import { computed } from "vue";
+import { isExternalLink } from "../../utils";
+import OnyxIcon from "../OnyxIcon/OnyxIcon.vue";
+import type { OnyxExternalLinkIcon } from "./types";
+
+const props = withDefaults(defineProps<OnyxExternalLinkIcon>(), {
+  withExternalIcon: "auto",
+});
+
+const isVisible = computed(() => {
+  const withExternalIcon = props.withExternalIcon;
+  if (withExternalIcon !== "auto") return withExternalIcon;
+  return isExternalLink(props.href ?? "");
+});
+</script>
+
+<template>
+  <OnyxIcon
+    v-if="isVisible"
+    class="onyx-external-link-icon"
+    :icon="arrowSmallUpRight"
+    size="16px"
+  />
+</template>
+
+<style lang="scss">
+@use "../../styles/mixins/layers.scss";
+
+.onyx-external-link-icon {
+  @include layers.component() {
+    align-self: flex-start;
+  }
+}
+</style>

packages/sit-onyx/src/components/OnyxExternalLinkIcon/types.ts

@@ -0,0 +1,12 @@
+export type OnyxExternalLinkIcon = {
+  /**
+   * The URL/link to point to.
+   */
+  href?: string;
+  /**
+   * Whether to show the external link icon.
+   * If set to `auto`, it will be shown when the `href` leads to another website
+   * (starting with "http://" or "https://") and will be hidden otherwise.
+   */
+  withExternalIcon?: boolean | "auto";
+};

packages/sit-onyx/src/components/OnyxLink/OnyxLink.vue

@@ -1,9 +1,6 @@
 <script lang="ts" setup>
-import arrowSmallUpRight from "@sit-onyx/icons/arrow-small-up-right.svg?raw";
-import { computed } from "vue";
 import { injectI18n } from "../../i18n";
-import { isExternalLink } from "../../utils";
-import OnyxIcon from "../OnyxIcon/OnyxIcon.vue";
+import OnyxExternalLinkIcon from "../OnyxExternalLinkIcon/OnyxExternalLinkIcon.vue";
 import OnyxVisuallyHidden from "../OnyxVisuallyHidden/OnyxVisuallyHidden.vue";
 import type { OnyxLinkProps } from "./types";
 
@@ -27,11 +24,6 @@ defineSlots<{
 }>();
 
 const { t } = injectI18n();
-
-const shouldShowExternalIcon = computed(() => {
-  if (props.withExternalIcon !== "auto") return props.withExternalIcon;
-  return isExternalLink(props.href);
-});
 </script>
 
 <template>
@@ -43,15 +35,12 @@ const shouldShowExternalIcon = computed(() => {
     @click="emit('click')"
   >
     <slot></slot>
+
     <OnyxVisuallyHidden v-if="props.target === '_blank'">
       {{ t("link.opensExternally") }}
     </OnyxVisuallyHidden>
-    <OnyxIcon
-      v-if="shouldShowExternalIcon"
-      class="onyx-link__icon"
-      :icon="arrowSmallUpRight"
-      size="16px"
-    />
+
+    <OnyxExternalLinkIcon v-bind="props" />
   </a>
 </template>
 
@@ -89,7 +78,7 @@ const shouldShowExternalIcon = computed(() => {
       }
     }
 
-    &__icon {
+    .onyx-external-link-icon {
       margin-left: var(--onyx-spacing-5xs);
     }
   }

packages/sit-onyx/src/components/OnyxLink/types.ts

@@ -1,19 +1,11 @@
-export type OnyxLinkProps = {
-  /**
-   * URL that the link points to.
-   */
-  href: string;
+import type { OnyxExternalLinkIcon } from "../OnyxExternalLinkIcon/types";
+
+export type OnyxLinkProps = OnyxExternalLinkIcon & {
   /**
    * Where to display the linked URL (same tab, new tab etc.).
    * For `_blank`, the `rel="noreferrer"` will be set automatically.
    */
   target?: LinkTarget;
-  /**
-   * Whether to show the external link icon.
-   * If set to `auto`, it will be shown when the `href` leads to another website
-   * (starting with "http://" or "https://") and will be hidden otherwise.
-   */
-  withExternalIcon?: boolean | "auto";
 };
 
 export const LINK_TARGETS = ["_self", "_blank", "_parent", "_top"] as const;

packages/sit-onyx/src/components/OnyxNavBar/OnyxNavBar.ct.tsx

@@ -1,4 +1,3 @@
-import { OnyxNavBar, OnyxNavItem } from "../..";
 import { expect, test } from "../../playwright/a11y";
 import {
   MOCK_PLAYWRIGHT_LOGO_URL,
@@ -11,10 +10,13 @@ import OnyxAppLayout from "../OnyxAppLayout/OnyxAppLayout.vue";
 import OnyxBadge from "../OnyxBadge/OnyxBadge.vue";
 import OnyxIcon from "../OnyxIcon/OnyxIcon.vue";
 import OnyxIconButton from "../OnyxIconButton/OnyxIconButton.vue";
-import OnyxListItem from "../OnyxListItem/OnyxListItem.vue";
 import OnyxPageLayout from "../OnyxPageLayout/OnyxPageLayout.vue";
 import OnyxTag from "../OnyxTag/OnyxTag.vue";
+import OnyxMenuItem from "./modules/OnyxMenuItem/OnyxMenuItem.vue";
+import OnyxNavButton from "./modules/OnyxNavButton/OnyxNavButton.vue";
+import OnyxNavItem from "./modules/OnyxNavItem/OnyxNavItem.vue";
 import OnyxUserMenu from "./modules/OnyxUserMenu/OnyxUserMenu.vue";
+import OnyxNavBar from "./OnyxNavBar.vue";
 
 test.beforeEach(async ({ page }) => {
   await defineLogoMockRoutes(page);
@@ -36,8 +38,8 @@ test.describe("Screenshot tests", () => {
           style={{ width: `${breakpointWidth}px` }}
           withBackButton={row.includes("back")}
         >
-          <OnyxNavItem label="Item" active />
-          <OnyxNavItem label="Item" />
+          <OnyxNavButton label="Item" active />
+          <OnyxNavButton label="Item" />
 
           <template v-slot:mobileActivePage>Item</template>
 
@@ -62,31 +64,21 @@ test("Screenshot tests (mobile)", async ({ mount, page }) => {
 
   const component = await mount(
     <OnyxNavBar appName="App name" logoUrl={MOCK_PLAYWRIGHT_LOGO_URL}>
-      <OnyxNavItem href="/1" label="Item 1" onClick={(href) => clickEvents.push(href)} />
-      <OnyxNavItem
-        href="/2"
-        label="Item 2"
-        options={[
-          {
-            label: "Nested item 1",
-            href: "/2/1",
-          },
-          {
-            label: "Nested item 2",
-            href: "/2/2",
-            active: true,
-          },
-          {
-            label: "Nested item 3",
-            href: "/2/3",
-          },
-        ]}
-        onClick={(href) => clickEvents.push(href)}
-      >
+      <OnyxNavButton href="#1" label="Item 1" onClick={(href) => clickEvents.push(href)} />
+      <OnyxNavButton href="#2" label="Item 2" onClick={(href) => clickEvents.push(href)}>
         Item 2
         <OnyxBadge color="warning" dot />
-      </OnyxNavItem>
-      <OnyxNavItem
+        <template v-slot:children>
+          <OnyxNavItem
+            label="Nested item 1"
+            href="#2-1"
+            onClick={(href) => clickEvents.push(href)}
+          />
+          <OnyxNavItem label="" href="#2-2" active onClick={(href) => clickEvents.push(href)} />
+          <OnyxNavItem label="" href="#2-3" onClick={(href) => clickEvents.push(href)} />
+        </template>
+      </OnyxNavButton>
+      <OnyxNavButton
         href="https://onyx.schwarz"
         label="Item 3"
         onClick={(href) => clickEvents.push(href)}
@@ -103,15 +95,15 @@ test("Screenshot tests (mobile)", async ({ mount, page }) => {
         <OnyxTag icon={mockPlaywrightIcon} color="warning" label="QA stage" />
 
         <OnyxUserMenu username="John Doe" description="Company name">
-          <OnyxListItem>
+          <OnyxMenuItem>
             <OnyxIcon icon={mockPlaywrightIcon} />
             Settings
-          </OnyxListItem>
+          </OnyxMenuItem>
 
-          <OnyxListItem color="danger">
+          <OnyxMenuItem color="danger">
             <OnyxIcon icon={mockPlaywrightIcon} />
             Logout
-          </OnyxListItem>
+          </OnyxMenuItem>
 
           <template v-slot:footer>
             App version
@@ -127,40 +119,41 @@ test("Screenshot tests (mobile)", async ({ mount, page }) => {
 
   // ASSERT
   await expect(page).toHaveScreenshot("burger.png");
+  return;
 
   // ACT
   await component.getByLabel("Item 1").click();
-  expect(clickEvents).toStrictEqual(["/1"]);
+  expect(clickEvents).toStrictEqual(["#1"]);
 
   // ACT
   await component.getByLabel("Item 2").click();
 
   // ASSERT
   await expect(page).toHaveScreenshot("burger-children.png");
-  expect(clickEvents).toStrictEqual(["/1"]);
+  expect(clickEvents).toStrictEqual(["#1"]);
 
   // ACT
   await component.getByLabel("Item 2", { exact: true }).click();
 
   // ASSERT
-  expect(clickEvents).toStrictEqual(["/1", "/2"]);
+  expect(clickEvents).toStrictEqual(["#1", "#2"]);
 
   // ACT
-  await component.getByLabel("Nested item 1").click();
+  await component.getByText("Nested item 1").click();
 
   // ASSERT
-  expect(clickEvents).toStrictEqual(["/1", "/2", "/2/1"]);
+  expect(clickEvents).toStrictEqual(["#1", "#2", "#2-1"]);
 
   // ACT
   await component.getByRole("button", { name: "Back" }).click();
 
   // ASSERT
-  await expect(component.getByLabel("Nested item 1")).toBeHidden();
-  await expect(component.getByLabel("Nested item 2")).toBeHidden();
-  await expect(component.getByLabel("Nested item 3")).toBeHidden();
-  await expect(component.getByLabel("Item 1")).toBeVisible();
-  await expect(component.getByLabel("Item 2")).toBeVisible();
-  await expect(component.getByLabel("Item 3")).toBeVisible();
+  await expect(component.getByText("Nested item 1")).toBeHidden();
+  await expect(component.getByText("Nested item 2")).toBeHidden();
+  await expect(component.getByText("Nested item 3")).toBeHidden();
+  await expect(component.getByText("Item 1")).toBeVisible();
+  await expect(component.getByText("Item 2")).toBeVisible();
+  await expect(component.getByText("Item 3")).toBeVisible();
 
   // ACT
   await component.getByLabel("Toggle context menu").click();
@@ -219,8 +212,8 @@ Object.entries(ONYX_BREAKPOINTS).forEach(([breakpoint, width]) => {
     await mount(
       <OnyxAppLayout>
         <OnyxNavBar appName="App name" logoUrl={MOCK_PLAYWRIGHT_LOGO_URL}>
-          <OnyxNavItem label="Item" active />
-          <OnyxNavItem label="Item" />
+          <OnyxNavButton label="Item" active />
+          <OnyxNavButton label="Item" />
 
           <template v-slot:mobileActivePage>Item</template>