Skip to content

Commit

Permalink
fix: convert menu
Browse files Browse the repository at this point in the history
  • Loading branch information
dauriamarco committed Oct 24, 2023
1 parent f946b4e commit a9c5884
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 118 deletions.
4 changes: 2 additions & 2 deletions src/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1085,7 +1085,7 @@ export namespace Components {
*/
"open": () => Promise<void>;
/**
* The element that will trigger the menu dialog. Accepts both a string (id of an element) or an HTML element.
* The element that will trigger the menu overlay. Accepts both a string (id of an element) or an HTML element.
*/
"trigger": string | HTMLElement;
}
Expand Down Expand Up @@ -3881,7 +3881,7 @@ declare namespace LocalJSX {
*/
"onWill-open"?: (event: SbbMenuCustomEvent<void>) => void;
/**
* The element that will trigger the menu dialog. Accepts both a string (id of an element) or an HTML element.
* The element that will trigger the menu overlay. Accepts both a string (id of an element) or an HTML element.
*/
"trigger"?: string | HTMLElement;
}
Expand Down
8 changes: 4 additions & 4 deletions src/components/sbb-menu/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ to identify which actions are active and which are not.
| ------------------------ | -------------------------- | --------------------------------------------------------------------------------------------------------------------------------- | ----------------------- | ----------- |
| `disableAnimation` | `disable-animation` | Whether the animation is enabled. | `boolean` | `false` |
| `listAccessibilityLabel` | `list-accessibility-label` | This will be forwarded as aria-label to the inner list. Used only if the menu automatically renders the actions inside as a list. | `string` | `undefined` |
| `trigger` | `trigger` | The element that will trigger the menu dialog. Accepts both a string (id of an element) or an HTML element. | `HTMLElement \| string` | `undefined` |
| `trigger` | `trigger` | The element that will trigger the menu overlay. Accepts both a string (id of an element) or an HTML element. | `HTMLElement \| string` | `undefined` |


## Events
Expand Down Expand Up @@ -107,9 +107,9 @@ Type: `Promise<void>`

## Slots

| Slot | Description |
| ----------- | ------------------------------------------------------- |
| `"unnamed"` | Use this slot to project any content inside the dialog. |
| Slot | Description |
| ----------- | ----------------------------------------------------- |
| `"unnamed"` | Use this slot to project any content inside the menu. |


----------------------------------------------
Expand Down
76 changes: 23 additions & 53 deletions src/components/sbb-menu/sbb-menu.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ describe('sbb-menu', () => {
});

it('opens on trigger click', async () => {
const dialog = await page.find('sbb-menu >>> dialog');
const willOpenEventSpy = await page.spyOnEvent(events.willOpen);
const didOpenEventSpy = await page.spyOnEvent(events.didOpen);

Expand All @@ -41,15 +40,14 @@ describe('sbb-menu', () => {
expect(didOpenEventSpy).toHaveReceivedEventTimes(1);

await page.waitForChanges();
expect(dialog).toHaveAttribute('open');
expect(element).toEqualAttribute('data-state', 'opened');
});

it('closes on Esc keypress', async () => {
const willOpenEventSpy = await page.spyOnEvent(events.willOpen);
const didOpenEventSpy = await page.spyOnEvent(events.didOpen);
const willCloseEventSpy = await page.spyOnEvent(events.willClose);
const didCloseEventSpy = await page.spyOnEvent(events.didClose);
const dialog = await page.find('sbb-menu >>> dialog');

trigger.triggerEvent('click');
await page.waitForChanges();
Expand All @@ -62,7 +60,7 @@ describe('sbb-menu', () => {
expect(didOpenEventSpy).toHaveReceivedEventTimes(1);
await page.waitForChanges();

expect(dialog).toHaveAttribute('open');
expect(element).toEqualAttribute('data-state', 'opened');

await page.keyboard.down('Tab');
await page.waitForChanges();
Expand All @@ -78,15 +76,14 @@ describe('sbb-menu', () => {
expect(didCloseEventSpy).toHaveReceivedEventTimes(1);
await page.waitForChanges();

expect(dialog).not.toHaveAttribute('open');
expect(element).toEqualAttribute('data-state', 'closed');
});

it('closes on menu action click', async () => {
const willOpenEventSpy = await page.spyOnEvent(events.willOpen);
const didOpenEventSpy = await page.spyOnEvent(events.didOpen);
const willCloseEventSpy = await page.spyOnEvent(events.willClose);
const didCloseEventSpy = await page.spyOnEvent(events.didClose);
const dialog = await page.find('sbb-menu >>> dialog');
const menuAction = await page.find('sbb-menu > sbb-menu-action');

trigger.triggerEvent('click');
Expand All @@ -100,7 +97,7 @@ describe('sbb-menu', () => {
expect(didOpenEventSpy).toHaveReceivedEventTimes(1);
await page.waitForChanges();

expect(dialog).toHaveAttribute('open');
expect(element).toEqualAttribute('data-state', 'opened');
expect(menuAction).not.toBeNull();

menuAction.triggerEvent('click');
Expand All @@ -113,15 +110,14 @@ describe('sbb-menu', () => {
expect(didCloseEventSpy).toHaveReceivedEventTimes(1);

await page.waitForChanges();
expect(dialog).not.toHaveAttribute('open');
expect(element).toEqualAttribute('data-state', 'closed');
});

it('closes on interactive element click', async () => {
const willOpenEventSpy = await page.spyOnEvent(events.willOpen);
const didOpenEventSpy = await page.spyOnEvent(events.didOpen);
const willCloseEventSpy = await page.spyOnEvent(events.willClose);
const didCloseEventSpy = await page.spyOnEvent(events.didClose);
const dialog = await page.find('sbb-menu >>> dialog');
const menuLink = await page.find('sbb-menu > sbb-link');

trigger.triggerEvent('click');
Expand All @@ -135,7 +131,7 @@ describe('sbb-menu', () => {
expect(didOpenEventSpy).toHaveReceivedEventTimes(1);
await page.waitForChanges();

expect(dialog).toHaveAttribute('open');
expect(element).toEqualAttribute('data-state', 'opened');
expect(menuLink).not.toBeNull();

menuLink.triggerEvent('click');
Expand All @@ -149,14 +145,13 @@ describe('sbb-menu', () => {
expect(didCloseEventSpy).toHaveReceivedEventTimes(1);
await page.waitForChanges();

expect(dialog).not.toHaveAttribute('open');
expect(element).toEqualAttribute('data-state', 'closed');
});

it('is correctly positioned on desktop', async () => {
const willOpenEventSpy = await page.spyOnEvent(events.willOpen);
const didOpenEventSpy = await page.spyOnEvent(events.didOpen);
await page.setViewport({ width: 1200, height: 800 });
const dialog = await page.find('sbb-menu >>> dialog');

trigger.triggerEvent('click');
await page.waitForChanges();
Expand All @@ -169,7 +164,7 @@ describe('sbb-menu', () => {
expect(didOpenEventSpy).toHaveReceivedEventTimes(1);
await page.waitForChanges();

expect(dialog).toHaveAttribute('open');
expect(element).toEqualAttribute('data-state', 'opened');

const buttonHeight = await page.evaluate(() =>
getComputedStyle(document.documentElement).getPropertyValue(
Expand All @@ -185,15 +180,19 @@ describe('sbb-menu', () => {
expect(await page.evaluate(() => document.querySelector('sbb-button').offsetTop)).toBe(0);
expect(await page.evaluate(() => document.querySelector('sbb-button').offsetLeft)).toBe(0);

// Expect dialog offsetTop to be equal to the trigger height + the dialog offset (8px)
// Expect menu offsetTop to be equal to the trigger height + the menu offset (8px)
expect(
await page.evaluate(
() => document.querySelector('sbb-menu').shadowRoot.querySelector('dialog').offsetTop,
() =>
(document.querySelector('sbb-menu').shadowRoot.querySelector('.sbb-menu') as HTMLElement)
.offsetTop,
),
).toBe(buttonHeightPx + 8);
expect(
await page.evaluate(
() => document.querySelector('sbb-menu').shadowRoot.querySelector('dialog').offsetLeft,
() =>
(document.querySelector('sbb-menu').shadowRoot.querySelector('.sbb-menu') as HTMLElement)
.offsetLeft,
),
).toBe(0);
});
Expand All @@ -203,7 +202,6 @@ describe('sbb-menu', () => {
const didOpenEventSpy = await page.spyOnEvent(events.didOpen);

await page.setViewport({ width: 800, height: 600 });
const dialog = await page.find('sbb-menu >>> dialog');

trigger.triggerEvent('click');
await page.waitForChanges();
Expand All @@ -216,54 +214,26 @@ describe('sbb-menu', () => {
expect(didOpenEventSpy).toHaveReceivedEventTimes(1);
await page.waitForChanges();

expect(dialog).toHaveAttribute('open');
expect(element).toEqualAttribute('data-state', 'opened');

const menuOffsetTop = await page.evaluate(
() => document.querySelector('sbb-menu').shadowRoot.querySelector('dialog').offsetTop,
() =>
(document.querySelector('sbb-menu').shadowRoot.querySelector('.sbb-menu') as HTMLElement)
.offsetTop,
);
const menuHeight = await page.evaluate(
() => document.querySelector('sbb-menu').shadowRoot.querySelector('dialog').offsetHeight,
() =>
(document.querySelector('sbb-menu').shadowRoot.querySelector('.sbb-menu') as HTMLElement)
.offsetHeight,
);
const pageHeight = await page.evaluate(() => window.innerHeight);

expect(menuOffsetTop).toBe(pageHeight - menuHeight);
});

it('sets the focus on the dialog content when the menu is opened by click', async () => {
const willOpenEventSpy = await page.spyOnEvent(events.willOpen);
const didOpenEventSpy = await page.spyOnEvent(events.didOpen);
const dialog = await page.find('sbb-menu >>> dialog');

trigger.triggerEvent('click');
await page.waitForChanges();

await waitForCondition(() => willOpenEventSpy.events.length === 1);
expect(willOpenEventSpy).toHaveReceivedEventTimes(1);
await page.waitForChanges();

await waitForCondition(() => didOpenEventSpy.events.length === 1);
expect(didOpenEventSpy).toHaveReceivedEventTimes(1);
await page.waitForChanges();

expect(dialog).toHaveAttribute('open');

await page.waitForChanges();
expect(
await page.evaluate(
() => document.querySelector('sbb-menu').shadowRoot.activeElement.className,
),
).toBe('sbb-menu__content');

await page.keyboard.down('Tab');
await page.waitForChanges();

expect(await page.evaluate(() => document.activeElement.id)).toBe('menu-link');
});

it('sets the focus to the first focusable element when the menu is opened by keyboard', async () => {
const willOpenEventSpy = await page.spyOnEvent(events.willOpen);
const didOpenEventSpy = await page.spyOnEvent(events.didOpen);
const dialog = await page.find('sbb-menu >>> dialog');

await page.keyboard.down('Tab');
await page.waitForChanges();
Expand All @@ -279,7 +249,7 @@ describe('sbb-menu', () => {
expect(didOpenEventSpy).toHaveReceivedEventTimes(1);
await page.waitForChanges();

expect(dialog).toHaveAttribute('open');
expect(element).toEqualAttribute('data-state', 'opened');

await page.waitForChanges();
expect(await page.evaluate(() => document.activeElement.id)).toBe('menu-link');
Expand Down
15 changes: 4 additions & 11 deletions src/components/sbb-menu/sbb-menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
}

.sbb-menu {
display: none;
opacity: 0;
pointer-events: none;
max-width: var(--sbb-menu-max-width);
Expand All @@ -110,23 +111,15 @@
padding: 0;
overflow: hidden;

:host([data-is-safari]) & {
:host(:not([data-state='closed'])) & {
display: block;
visibility: hidden;
}

&[open] {
opacity: 1;
pointer-events: all;
animation: {
name: open;
duration: var(--sbb-menu-animation-duration);
timing-function: var(--sbb-menu-animation-easing);
}

:host([data-is-safari]) & {
visibility: visible;
}
}

:host([data-state='closing']) & {
Expand All @@ -146,7 +139,7 @@
max-height: fit-content;
border-radius: var(--sbb-menu-border-radius);

&[open] {
:host(:not([data-state='closed'])) & {
top: var(--sbb-menu-position-y);
left: var(--sbb-menu-position-x);
max-height: var(--sbb-menu-max-height);
Expand All @@ -173,7 +166,7 @@
@include sbb.mq($from: medium) {
max-height: fit-content;

.sbb-menu[open] & {
:host(:not([data-state='closed'])) & {
max-height: var(--sbb-menu-max-height);
min-height: var(--sbb-menu-min-height);
}
Expand Down
8 changes: 4 additions & 4 deletions src/components/sbb-menu/sbb-menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ describe('sbb-menu', () => {
<sbb-menu trigger="menu-trigger" data-state="closed" id="sbb-menu-1">
<mock:shadow-root>
<div class="sbb-menu__container">
<dialog class="sbb-menu" role="presentation">
<div class="sbb-menu" role="presentation">
<div class="sbb-menu__content">
<slot></slot>
</div>
</dialog>
</div>
</div>
</mock:shadow-root>
<sbb-link href="https://www.sbb.ch/en" variant="block">
Expand Down Expand Up @@ -66,7 +66,7 @@ describe('sbb-menu', () => {
<sbb-menu data-state="closed" id="sbb-menu-2" trigger="menu-trigger">
<mock:shadow-root>
<div class="sbb-menu__container">
<dialog class="sbb-menu" role="presentation">
<div class="sbb-menu" role="presentation">
<div class="sbb-menu__content">
<ul class="sbb-menu-list">
<li>
Expand All @@ -86,7 +86,7 @@ describe('sbb-menu', () => {
<slot></slot>
</span>
</div>
</dialog>
</div>
</div>
</mock:shadow-root>
<sbb-menu-action icon="tick-small" slot="action-0">
Expand Down
Loading

0 comments on commit a9c5884

Please sign in to comment.