Skip to content

Commit

Permalink
feat(core/menu): provide keyboard navigation (#685)
Browse files Browse the repository at this point in the history
  • Loading branch information
danielleroux authored Aug 14, 2023
1 parent 2f3b488 commit 4327a96
Show file tree
Hide file tree
Showing 13 changed files with 35 additions and 25 deletions.
10 changes: 10 additions & 0 deletions packages/core/src/components/menu-avatar/menu-avatar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,17 @@
@include ix-component;
display: block;
position: relative;
margin-bottom: 0.5rem;
margin-right: 0.75rem;

.avatar {
all: unset;
box-sizing: border-box;

display: flex;
align-items: center;
height: 2.5rem;
width: 100%;
max-height: 2.5rem;
padding-left: 0.25rem;
margin-left: 0.41rem;
Expand Down Expand Up @@ -80,4 +86,8 @@
color: var(--theme-avatar-btn--color--active);
}
}

.avatar:focus-visible {
outline: 1px solid var(--theme-color-focus-bdr);
}
}
7 changes: 4 additions & 3 deletions packages/core/src/components/menu-avatar/menu-avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,11 @@ export class MenuAvatar {
render() {
return (
<Host slot="ix-menu-avatar">
<li
<button
class="nav-item top-item avatar no-hover"
title={this.top}
id={this.avatarElementId}
tabIndex={0}
>
<ix-avatar image={this.image} initials={this.initials}></ix-avatar>

Expand All @@ -78,12 +79,12 @@ export class MenuAvatar {
{this.bottom}
</span>
</div>
</li>
</button>
<ix-dropdown
trigger={this.hostElement}
placement={'right-start'}
offset={{
mainAxis: 6,
mainAxis: 16,
}}
>
<slot></slot>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ test('should show items as dropdown', async ({ mount, page }) => {
const dropdownHeader = dropdown.locator('ix-dropdown-item');
await expect(dropdownHeader).toHaveText('Category label');

const itemOne = page.getByRole('listitem', { name: 'Test Item 1' });
const itemTwo = page.getByRole('listitem', { name: 'Test Item 2' });
const itemOne = page.locator('ix-menu-item').nth(0);
const itemTwo = page.locator('ix-menu-item').nth(1);

await expect(itemOne).toBeVisible();
await expect(itemTwo).toBeVisible();
Expand Down
14 changes: 8 additions & 6 deletions packages/core/src/components/menu-item/menu-item.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
@import 'common-variables';
@import 'mixins/hover';
@import 'mixins/text-truncation';

$menuItemHeight: 3rem;
$menuItemPadding: 0.875rem;

:host {
position: relative;
Expand All @@ -22,12 +24,15 @@ $menuItemHeight: 3rem;
max-height: var(--ix-menu-item-height, $menuItemHeight);

.tab {
all: unset;
box-sizing: border-box;
display: flex;
position: relative;
align-items: center;
height: var(--ix-menu-item-height, $menuItemHeight);
width: calc(100%);
z-index: 500;
padding-left: 0.875rem;
padding-left: $menuItemPadding;

@include ghost-hover-pressed;
}
Expand All @@ -38,11 +43,8 @@ $menuItemHeight: 3rem;
}

.tab:focus-visible {
outline: none;
}

&:focus-visible {
outline: none;
outline: 1px solid var(--theme-color-focus-bdr);
outline-offset: -1px;
}

.tab:not(:last-child) {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/components/menu-item/menu-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class MenuItem {
}}
{...extendedAttributes}
>
<li class="tab" title={this.title}>
<button class="tab" title={this.title} tabIndex={0} role="listitem">
<ix-icon name={this.icon ?? this.tabIcon}></ix-icon>
<div class="notification">
{this.notifications ? (
Expand All @@ -119,7 +119,7 @@ export class MenuItem {
<span class="tab-text text-default">
<slot></slot>
</span>
</li>
</button>
</Host>
);
}
Expand Down
12 changes: 6 additions & 6 deletions packages/core/src/components/menu-item/test/menu-item.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ describe('ix-menu-item', () => {

const item = page.doc.querySelector('ix-menu-item');

const li = item.shadowRoot.querySelector('li');
const avatarElement = item.shadowRoot.querySelector('button');

expect(li.title).toBe('Example Title');
expect(avatarElement.title).toBe('Example Title');
});

it('should have a title from innerText', async () => {
Expand All @@ -44,9 +44,9 @@ describe('ix-menu-item', () => {

const item = page.doc.querySelector('ix-menu-item');

const li = item.shadowRoot.querySelector('li');
const avatarElement = item.shadowRoot.querySelector('button');

expect(li.title).toBe('Example Title');
expect(avatarElement.title).toBe('Example Title');
});

it('should change title after content change', async () => {
Expand All @@ -58,7 +58,7 @@ describe('ix-menu-item', () => {
await page.waitForChanges();
const item = page.doc.querySelector('ix-menu-item');
item.innerText = 'Updated Title';
const li = item.shadowRoot.querySelector('li');
expect(li.title).toStrictEqual('Example Title');
const avatarElement = item.shadowRoot.querySelector('button');
expect(avatarElement.title).toStrictEqual('Example Title');
});
});
9 changes: 3 additions & 6 deletions packages/core/src/components/menu/test/menu.ct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ test('should stay close after menu click when NOT pinned', async ({
await menuButton.click();

await expect(menu).toHaveClass(/expanded/);
await page.getByRole('listitem').click();
await page.locator('ix-menu-item').click();
await expect(menu).not.toHaveClass(/expanded/);
});

Expand All @@ -62,7 +62,7 @@ test('should stay open after menu click when pinned', async ({

await expect(menu).not.toHaveClass(/expanded/);

await page.getByRole('listitem').click();
await page.locator('ix-menu-item').click();

await expect(menu).not.toHaveClass(/expanded/);
});
Expand Down Expand Up @@ -172,10 +172,7 @@ test('should close about by item click', async ({ mount, page }) => {
await clickAboutButton(element, page);
let about = page.locator('ix-menu-about');
let settings = page.locator('ix-menu-settings');
const menuItem = page
.locator('ix-menu-item')
.filter({ hasText: 'Random' })
.getByRole('listitem');
const menuItem = page.locator('ix-menu-item').filter({ hasText: 'Random' });

await menuItem.click();
await expect(about).not.toBeVisible();
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 4327a96

Please sign in to comment.