Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(OnyxNavItem): Implement OnyxNavItem #1367

Merged
merged 29 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e709131
Implement OnyxNavItem
MajaZarkova Jun 20, 2024
9d54e97
docs(changeset): Implement new OnyxNavItem component
MajaZarkova Jun 20, 2024
3a401e5
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 20, 2024
784ae5e
Update screenshot tests
MajaZarkova Jun 20, 2024
21b03bb
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 20, 2024
dc45d03
Merge branch 'main' into feat/883-Implement-OnyxNavItem
BoppLi Jun 21, 2024
df220f8
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 21, 2024
468249b
Merge branch 'main' into feat/883-Implement-OnyxNavItem
BoppLi Jun 21, 2024
96f20ad
Update from main
MajaZarkova Jun 24, 2024
417987f
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 24, 2024
c4adc91
Mob session work
MajaZarkova Jun 24, 2024
f858247
add href fallback
JoCa96 Jun 24, 2024
63690ee
added more keyboard handlers
JoCa96 Jun 24, 2024
4f5c908
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 25, 2024
be99d78
Fix composable test
MajaZarkova Jun 25, 2024
eb01d8f
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 25, 2024
2b3d325
Fix styles
MajaZarkova Jun 25, 2024
fc5f8c5
Fix test
MajaZarkova Jun 25, 2024
435f8db
Add composable tests
MajaZarkova Jun 25, 2024
d499800
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 26, 2024
9594868
Apply code suggestions
MajaZarkova Jun 26, 2024
08de3e0
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 26, 2024
e70736e
Resolve conflicts
MajaZarkova Jun 26, 2024
64859e3
Apply code suggestions
MajaZarkova Jun 26, 2024
44d6f89
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 26, 2024
e2cdfe9
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 27, 2024
01931b5
Merge branch 'main' into feat/883-Implement-OnyxNavItem
MajaZarkova Jun 27, 2024
1365f46
Add JSDocs
MajaZarkova Jun 27, 2024
7a13014
Merge branch 'main' into feat/883-Implement-OnyxNavItem
BoppLi Jun 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/little-clouds-share.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"sit-onyx": minor
---

Implement new OnyxNavItem component
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,15 @@ const activeItem = ref<string>();
const {
elements: { button, menu, menuItem, listItem, flyout },
state: { isExpanded },
} = createMenuButton({
onSelect: (value) => {
activeItem.value = value;
},
});
} = createMenuButton({});
</script>

<template>
<button v-bind="button">Toggle nav menu</button>
<div v-bind="flyout">
<ul v-show="isExpanded" v-bind="menu">
<li v-for="item in items" v-bind="listItem" :key="item.value" title="item">
<a v-bind="menuItem({ active: activeItem === item.value, value: item.value })">{{
item.label
}}</a>
<a v-bind="menuItem({ active: activeItem === item.value })" href="#">{{ item.label }}</a>
</li>
</ul>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ export type MenuButtonTestingOptions = {
* Playwright utility for executing accessibility testing for a navigation menu.
* Will check aria attributes and keyboard shortcuts as defined in https://www.w3.org/WAI/ARIA/apg/patterns/menu-button/examples/menu-button-links.
*/
export const menuButtonTesting = async ({ button, menu }: MenuButtonTestingOptions) => {
export const menuButtonTesting = async ({
page,
button,
menu,
menuItems,
}: MenuButtonTestingOptions) => {
const menuId = await menu.getAttribute("id");
expect(menuId).toBeDefined();
await expect(
Expand All @@ -51,4 +56,54 @@ export const menuButtonTesting = async ({ button, menu }: MenuButtonTestingOptio
button,
'flyout menu must have an "aria-expanded" attribute set to true',
).toHaveAttribute("aria-expanded", "true");

const firstItem = menuItems[0].getByRole("menuitem");
const secondItem = menuItems[1].getByRole("menuitem");
const lastItem = menuItems[menuItems.length - 1].getByRole("menuitem");

await page.keyboard.press("Tab");
await expect(button, "Button should be focused when pressing tab key").toBeFocused();

await button.press("ArrowDown");
await expect(
firstItem,
"First item should be focused when pressing arrow down key",
).toBeFocused();

await menu.press("ArrowDown");
await expect(
secondItem,
"Second item should be focused when pressing arrow down key",
).toBeFocused();

await menu.press("ArrowUp");
await expect(firstItem, "First item should be focused when pressing arrow up key").toBeFocused();

await menu.press("ArrowRight");
await expect(
secondItem,
"Second item should be focused when pressing arrow right key",
).toBeFocused();

await menu.press("ArrowLeft");
await expect(
firstItem,
"First item should be focused when pressing arrow left key",
).toBeFocused();

await page.keyboard.press("Tab");
await expect(button, "Button should be focused when pressing tab key").not.toBeFocused();

await page.keyboard.press("Tab");

await menu.press("Home");
await expect(firstItem, "First item should be focused when pressing home key").toBeFocused();

await page.keyboard.press("Tab");
await expect(button, "Button should be focused when pressing tab key").not.toBeFocused();

await page.keyboard.press("Tab");

await menu.press("End");
await expect(lastItem, "Last item should be focused when pressing end key").toBeFocused();
};
89 changes: 72 additions & 17 deletions packages/headless/src/composables/menuButton/createMenuButton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@ import { createBuilder } from "../../utils/builder";
import { createId } from "../../utils/id";
import { debounce } from "../../utils/timer";

export type CreateMenuButtonOptions = {
/**
* Called when a menu item is selected (via mouse or keyboard).
*/
onSelect: (value: string) => void;
};

export const createMenuButton = createBuilder((options: CreateMenuButtonOptions) => {
/**
* Based on https://www.w3.org/WAI/ARIA/apg/patterns/disclosure/examples/disclosure-navigation/
*/
export const createMenuButton = createBuilder(() => {
MajaZarkova marked this conversation as resolved.
Show resolved Hide resolved
const menuId = createId("menu");
const buttonId = createId("menu-button");
const isExpanded = ref<boolean>(false);
const isExpanded = ref(false);

/**
* Debounced expanded state that will only be toggled after a given timeout.
Expand All @@ -32,6 +28,67 @@ export const createMenuButton = createBuilder((options: CreateMenuButtonOptions)
};
});

const focusRelativeItem = (next: "next" | "prev" | "first" | "last") => {
const currentMenuItem = document.activeElement as HTMLElement;

// Either the current focus is on a "menuitem", then we can just get the parent menu.
// Or the current focus is on the button, then we can get the connected menu using the menuId
const currentMenu =
currentMenuItem?.closest('[role="menu"]') || document.getElementById(menuId);
if (!currentMenu) return;

const menuItems = [...currentMenu.querySelectorAll<HTMLElement>('[role="menuitem"]')];
let nextIndex = 0;

if (currentMenuItem) {
const currentIndex = menuItems.indexOf(currentMenuItem);
switch (next) {
case "next":
nextIndex = currentIndex + 1;
break;
case "prev":
nextIndex = currentIndex - 1;
break;
case "first":
nextIndex = 0;
break;
case "last":
nextIndex = menuItems.length - 1;
break;
}
}

const nextMenuItem = menuItems[nextIndex];
nextMenuItem?.focus();
};

const handleKeydown = (event: KeyboardEvent) => {
switch (event.key) {
case "ArrowDown":
case "ArrowRight":
event.preventDefault();
focusRelativeItem("next");
break;
case "ArrowUp":
case "ArrowLeft":
event.preventDefault();
focusRelativeItem("prev");
break;
case "Home":
event.preventDefault();
focusRelativeItem("first");
break;
case "End":
event.preventDefault();
focusRelativeItem("last");
break;
case " ":
event.preventDefault();
(event.target as HTMLElement).click();
break;
}
};

return {
state: { isExpanded },
elements: {
Expand All @@ -43,26 +100,24 @@ export const createMenuButton = createBuilder((options: CreateMenuButtonOptions)
"aria-haspopup": true,
id: buttonId,
...hoverEvents.value,
onKeydown: handleKeydown,
}) as const,
),
listItem: {
role: "none",
},
flyout: {
...hoverEvents.value,
},
menu: {
id: menuId,
role: "menu",
"aria-labelledby": buttonId,
onKeydown: handleKeydown,
},
listItem: {
role: "none",
},
menuItem: (data: { active?: boolean; value: string }) => ({
menuItem: (data: { active?: boolean }) => ({
"aria-current": data.active ? "page" : undefined,
role: "menuitem",
tabindex: -1,
onClick: () => {
options.onSelect(data.value);
},
}),
},
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { defineStorybookActionsAndVModels } from "@sit-onyx/storybook-utils";
import type { Meta, StoryObj } from "@storybook/vue3";
import { h } from "vue";
import OnyxListItem from "../../OnyxListItem/OnyxListItem.vue";
import OnyxNavItem from "../../OnyxNavItem/future/OnyxNavItem.vue";
import OnyxButton from "../../OnyxButton/OnyxButton.vue";
import OnyxFlyoutMenu from "./OnyxFlyoutMenu.vue";

Expand Down Expand Up @@ -50,6 +50,6 @@ const listAnimals = [
export const Default = {
args: {
default: () => h(OnyxButton, { label: "Hover me" }),
options: () => listAnimals.map(({ label }) => h(OnyxListItem, label)),
options: () => listAnimals.map(({ label }) => h(OnyxNavItem, label)),
},
} satisfies Story;
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<script setup lang="ts" generic="TValue extends SelectOptionValue = SelectOptionValue">
import { createMenuButton } from "@sit-onyx/headless";
import type { SelectOptionValue } from "../../../types";
import { computed, ref, type VNode } from "vue";
import { provide, type VNode } from "vue";
import { injectI18n } from "../../../i18n";
import { MENU_BUTTON_ITEM_INJECTION_KEY } from "../../OnyxNavItem/future/types";

const slots = defineSlots<{
/**
Expand All @@ -23,32 +24,12 @@ const slots = defineSlots<{
footer?(): unknown;
}>();

const activeItem = ref<string>();

const {
elements: { button, menu, menuItem, listItem, flyout },
state: { isExpanded },
} = createMenuButton({
onSelect: (value) => {
activeItem.value = value;
},
});

const getSlotComponents = (vnodes: VNode[]): VNode[] => {
// if the slot only contains a v-for, we need to use the children here which are the "actual" slot content
const isVFor = vnodes.length === 1 && vnodes[0].type.toString() === "Symbol(v-fgt)";

const allNodes =
isVFor && Array.isArray(vnodes[0].children)
? (vnodes[0].children as Extract<(typeof vnodes)[number]["children"], []>)
: vnodes;
} = createMenuButton({});

return allNodes;
};

const options = computed(() => {
return getSlotComponents(slots.options?.() ?? []);
});
provide(MENU_BUTTON_ITEM_INJECTION_KEY, { menuItem, listItem });

const { t } = injectI18n();
</script>
Expand All @@ -73,21 +54,7 @@ const { t } = injectI18n();
v-bind="menu"
class="onyx-future-flyout-menu__wrapper onyx-flyout-menu__group"
>
<li
v-for="(item, index) in options"
v-bind="listItem"
:key="index"
class="onyx-future-flyout-menu__option"
>
<component
:is="item"
v-bind="
item.props?.href
? menuItem({ active: activeItem === item.props.href, value: item.props.href })
: undefined
"
/>
</li>
<slot name="options"></slot>
</ul>
<slot name="footer"></slot>
</div>
Expand Down Expand Up @@ -131,10 +98,6 @@ const { t } = injectI18n();
&__wrapper {
padding: 0;
}

&__option {
list-style: none;
}
}
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const { densityClass } = useDensity(props);
cursor: pointer;

&:hover,
&:focus-within,
&.onyx-list-item--active {
background-color: var(--onyx-list-item-background-hover);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/sit-onyx/src/components/OnyxNavBar/OnyxNavBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import OnyxHeadline from "../OnyxHeadline/OnyxHeadline.vue";
import OnyxIconButton from "../OnyxIconButton/OnyxIconButton.vue";
import OnyxMobileNavButton from "../OnyxMobileNavButton/OnyxMobileNavButton.vue";
import OnyxNavAppArea from "../OnyxNavAppArea/OnyxNavAppArea.vue";
import { mobileNavBarInjectionKey, type OnyxNavBarProps } from "./types";
import { MOBILE_NAV_BAR_INJECTION_KEY, type OnyxNavBarProps } from "./types";

const props = withDefaults(defineProps<OnyxNavBarProps>(), {
mobileBreakpoint: "sm",
Expand Down Expand Up @@ -70,7 +70,7 @@ const isMobile = computed(() => {
return width.value !== 0 && width.value < mobileWidth;
});

provide(mobileNavBarInjectionKey, isMobile);
provide(MOBILE_NAV_BAR_INJECTION_KEY, isMobile);
</script>

<template>
Expand Down
2 changes: 1 addition & 1 deletion packages/sit-onyx/src/components/OnyxNavBar/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ export type OnyxNavBarProps = Omit<OnyxNavAppAreaProps, "label"> & {
*
* @returns `true` if mobile, `false` otherwise
*/
export const mobileNavBarInjectionKey = Symbol() as InjectionKey<ComputedRef<boolean>>;
export const MOBILE_NAV_BAR_INJECTION_KEY = Symbol() as InjectionKey<ComputedRef<boolean>>;
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Locator } from "@playwright/test";
import { expect, test } from "../../playwright/a11y";
import { executeMatrixScreenshotTest } from "../../playwright/screenshots";
import OnyxListItem from "../OnyxListItem/OnyxListItem.vue";
import OnyxNavItem from "../OnyxNavItem/future/OnyxNavItem.vue";
import OnyxNavButton from "./OnyxNavButton.vue";

test.describe("Screenshot tests", () => {
Expand Down Expand Up @@ -58,9 +58,9 @@ test.describe("Screenshot tests with nested children", () => {
component: (column) => (
<OnyxNavButton label="Item" href="#" active={column === "active"}>
<template v-slot:children>
<OnyxListItem>Nested Item 1</OnyxListItem>
<OnyxListItem>Nested Item 2</OnyxListItem>
<OnyxListItem>Nested Item 3</OnyxListItem>
<OnyxNavItem>Nested Item 1</OnyxNavItem>
<OnyxNavItem>Nested Item 2</OnyxNavItem>
<OnyxNavItem>Nested Item 3</OnyxNavItem>
</template>
</OnyxNavButton>
),
Expand Down
Loading
Loading