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.

Sign up for GitHub

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

fix(sbb-dialog, sbb-menu, sbb-navigation, sbb-tooltip): fix reading order for VoiceOver with Safari #2106

Merged
merged 21 commits into from
Oct 26, 2023
Merged

fix(sbb-dialog, sbb-menu, sbb-navigation, sbb-tooltip): fix reading order for VoiceOver with Safari #2106

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
7dda525
refactor(sbb-navigation): simplify implementation, remove HTML dialog
jeripeierSBB Oct 24, 2023
d9e64c4
fix: review
jeripeierSBB Oct 24, 2023
79e6f78
fix: remove display contents as it interferes with VoiceOver
jeripeierSBB Oct 24, 2023
a30cef4
fix: aria-label
jeripeierSBB Oct 24, 2023
6c5b06f
fix: convert dialog
jeripeierSBB Oct 24, 2023
6ccebbf
fix: default spacing
jeripeierSBB Oct 24, 2023
277276e
fix: fix tests
jeripeierSBB Oct 24, 2023
7fa5f2a
fix: convert menu
dauriamarco Oct 24, 2023
9a3201d
fix: wording
jeripeierSBB Oct 25, 2023
2b4ec92
fix: integrity
jeripeierSBB Oct 25, 2023
25d329b
refactor: tooltip
jeripeierSBB Oct 25, 2023
24ddf14
fix: chromatic
jeripeierSBB Oct 25, 2023
d92e03f
fix: tooltip stories
jeripeierSBB Oct 25, 2023
ef7f8bf
fix: accent color and tooltip description
jeripeierSBB Oct 25, 2023
d245b16
fix: menu role
jeripeierSBB Oct 26, 2023
d928ed0
refactor: remove role group from dialog
jeripeierSBB Oct 26, 2023
dfb265c
refactor: naming of dialog
jeripeierSBB Oct 26, 2023
33c6fd9
refactor: add tests
jeripeierSBB Oct 26, 2023
3c729c6
fix: review
jeripeierSBB Oct 26, 2023
562aafc
fix: review
jeripeierSBB Oct 26, 2023
5c3862f
fix: review
jeripeierSBB Oct 26, 2023
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
20 changes: 6 additions & 14 deletions src/components.d.ts
View file
Open in desktop
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 @@ -1150,10 +1150,6 @@ export namespace Components {
* This will be forwarded as aria-label to the close button element.
*/
"accessibilityCloseLabel": string | undefined;
/**
* This will be forwarded as aria-label to the dialog and is read as a title of the navigation.
*/
"accessibilityLabel": string | undefined;
/**
* Closes the navigation.
*/
Expand Down Expand Up @@ -1226,7 +1222,7 @@ export namespace Components {
*/
"accessibilityBackLabel": string | undefined;
/**
* This will be forwarded as aria-label to the dialog and is read as a title of the navigation-section.
* This will be forwarded as aria-label to the nav element and is read as a title of the navigation-section.
*/
"accessibilityLabel": string | undefined;
/**
Expand Down Expand Up @@ -1968,7 +1964,7 @@ export namespace Components {
*/
"openDelay"?: number;
/**
* The element that will trigger the tooltip dialog. Accepts both a string (id of an element) or an HTML element.
* The element that will trigger the tooltip overlay. Accepts both a string (id of an element) or an HTML element.
*/
"trigger": string | HTMLElement;
}
Expand Down Expand Up @@ -3885,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 Expand Up @@ -3950,10 +3946,6 @@ declare namespace LocalJSX {
* This will be forwarded as aria-label to the close button element.
*/
"accessibilityCloseLabel"?: string | undefined;
/**
* This will be forwarded as aria-label to the dialog and is read as a title of the navigation.
*/
"accessibilityLabel"?: string | undefined;
/**
* Whether the animation is enabled.
*/
Expand Down Expand Up @@ -4032,7 +4024,7 @@ declare namespace LocalJSX {
*/
"accessibilityBackLabel"?: string | undefined;
/**
* This will be forwarded as aria-label to the dialog and is read as a title of the navigation-section.
* This will be forwarded as aria-label to the nav element and is read as a title of the navigation-section.
*/
"accessibilityLabel"?: string | undefined;
/**
Expand Down Expand Up @@ -4863,7 +4855,7 @@ declare namespace LocalJSX {
*/
"openDelay"?: number;
/**
* The element that will trigger the tooltip dialog. Accepts both a string (id of an element) or an HTML element.
* The element that will trigger the tooltip overlay. Accepts both a string (id of an element) or an HTML element.
*/
"trigger"?: string | HTMLElement;
}
Expand Down
55 changes: 18 additions & 37 deletions src/components/sbb-dialog/sbb-dialog.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ describe('sbb-dialog', () => {
});

it('opens the dialog', async () => {
const dialog = await page.find('sbb-dialog >>> dialog');
const willOpen = await page.spyOnEvent(events.willOpen);
const didOpen = await page.spyOnEvent(events.didOpen);

Expand All @@ -39,15 +38,13 @@ describe('sbb-dialog', () => {
await page.waitForChanges();

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

it('closes the dialog', async () => {
const willOpen = await page.spyOnEvent(events.willOpen);
const didOpen = await page.spyOnEvent(events.didOpen);
const willClose = await page.spyOnEvent(events.willClose);
const didClose = await page.spyOnEvent(events.didClose);
const dialog = await page.find('sbb-dialog >>> dialog');

await element.callMethod('open');
await page.waitForChanges();
Expand All @@ -60,7 +57,7 @@ describe('sbb-dialog', () => {
expect(didOpen).toHaveReceivedEventTimes(1);
await page.waitForChanges();

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

await element.callMethod('close');
await page.waitForChanges();
Expand All @@ -74,15 +71,13 @@ describe('sbb-dialog', () => {
await page.waitForChanges();

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

it('closes the dialog on backdrop click', async () => {
const willOpen = await page.spyOnEvent(events.willOpen);
const didOpen = await page.spyOnEvent(events.didOpen);
const willClose = await page.spyOnEvent(events.willClose);
const didClose = await page.spyOnEvent(events.didClose);
const dialog = await page.find('sbb-dialog >>> dialog');

await element.callMethod('open');
await page.waitForChanges();
Expand All @@ -95,7 +90,7 @@ describe('sbb-dialog', () => {
expect(didOpen).toHaveReceivedEventTimes(1);
await page.waitForChanges();

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

// Simulate backdrop click
await page.mouse.click(1, 1);
Expand All @@ -110,15 +105,13 @@ describe('sbb-dialog', () => {
await page.waitForChanges();

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

it('does not close the dialog on backdrop click', async () => {
const willOpen = await page.spyOnEvent(events.willOpen);
const didOpen = await page.spyOnEvent(events.didOpen);
const willClose = await page.spyOnEvent(events.willClose);
const didClose = await page.spyOnEvent(events.didClose);
const dialog = await page.find('sbb-dialog >>> dialog');

await element.setProperty('backdropAction', 'none');
await page.waitForChanges();
Expand All @@ -134,7 +127,7 @@ describe('sbb-dialog', () => {
expect(didOpen).toHaveReceivedEventTimes(1);
await page.waitForChanges();

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

// Simulate backdrop click
await page.mouse.click(1, 1);
Expand All @@ -147,11 +140,9 @@ describe('sbb-dialog', () => {
await page.waitForChanges();

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

it('closes the dialog on close button click', async () => {
const dialog = await page.find('sbb-dialog >>> dialog');
const closeButton = await page.find('sbb-dialog >>> [sbb-dialog-close]');
const willOpen = await page.spyOnEvent(events.willOpen);
const didOpen = await page.spyOnEvent(events.didOpen);
Expand All @@ -169,7 +160,7 @@ describe('sbb-dialog', () => {
expect(didOpen).toHaveReceivedEventTimes(1);
await page.waitForChanges();

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

closeButton.triggerEvent('click');
await page.waitForChanges();
Expand All @@ -182,11 +173,10 @@ describe('sbb-dialog', () => {
expect(didClose).toHaveReceivedEventTimes(1);
await page.waitForChanges();

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

it('closes the dialog on Esc key press', async () => {
const dialog = await page.find('sbb-dialog >>> dialog');
const willOpen = await page.spyOnEvent(events.willOpen);
const didOpen = await page.spyOnEvent(events.didOpen);
const willClose = await page.spyOnEvent(events.willClose);
Expand All @@ -203,7 +193,7 @@ describe('sbb-dialog', () => {
expect(didOpen).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 @@ -219,11 +209,10 @@ describe('sbb-dialog', () => {
expect(didClose).toHaveReceivedEventTimes(1);
await page.waitForChanges();

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

it('does not have the fullscreen attribute', async () => {
const dialog = await page.find('sbb-dialog >>> dialog');
const willOpen = await page.spyOnEvent(events.willOpen);
const didOpen = await page.spyOnEvent(events.didOpen);

Expand All @@ -238,7 +227,7 @@ describe('sbb-dialog', () => {
expect(didOpen).toHaveReceivedEventTimes(1);
await page.waitForChanges();

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

await page.waitForChanges();
expect(element).not.toHaveAttribute('data-fullscreen');
Expand All @@ -254,7 +243,6 @@ describe('sbb-dialog', () => {
`);
element = await page.find('sbb-dialog');

const dialog = await page.find('sbb-dialog >>> dialog');
const willOpen = await page.spyOnEvent(events.willOpen);
const didOpen = await page.spyOnEvent(events.didOpen);

Expand All @@ -269,7 +257,7 @@ describe('sbb-dialog', () => {
expect(didOpen).toHaveReceivedEventTimes(1);
await page.waitForChanges();

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

await page.waitForChanges();
expect(element).toHaveAttribute('data-fullscreen');
Expand All @@ -280,7 +268,7 @@ describe('sbb-dialog', () => {
await page.setContent(`
<sbb-dialog id="my-dialog" title-content="Title" title-back-button="true" disable-animation>
Dialog content.
<div slot="action-group">Action group</div>
<div slot="action-group">Action group</div>
</sbb-dialog>

<sbb-dialog id="stacked-dialog" disable-animation>
Expand All @@ -289,7 +277,6 @@ describe('sbb-dialog', () => {
`);
element = await page.find('sbb-dialog');

const dialog = await page.find('#my-dialog >>> dialog');
const willOpen = await page.spyOnEvent(events.willOpen);
const didOpen = await page.spyOnEvent(events.didOpen);

Expand All @@ -304,34 +291,33 @@ describe('sbb-dialog', () => {
expect(didOpen).toHaveReceivedEventTimes(1);
await page.waitForChanges();

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

const stackedDialog = await page.find('#stacked-dialog');
const stackedDialogElement = await page.find('#stacked-dialog >>> dialog');

await stackedDialog.callMethod('open');
await page.waitForChanges();

expect(stackedDialogElement).toHaveAttribute('open');
expect(stackedDialog).toEqualAttribute('data-state', 'opened');

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

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

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

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

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

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

it('does not close the dialog on other overlay click', async () => {
Expand All @@ -352,9 +338,7 @@ describe('sbb-dialog', () => {
const didOpen = await page.spyOnEvent(events.didOpen);
const willClose = await page.spyOnEvent(events.willClose);
const didClose = await page.spyOnEvent(events.didClose);
const outerDialog = await page.find('sbb-dialog >>> dialog');
const innerElement = await page.find('sbb-dialog > sbb-dialog');
const innerDialog = await page.find('sbb-dialog > sbb-dialog >>> dialog');

await element.callMethod('open');
await page.waitForChanges();
Expand All @@ -367,7 +351,7 @@ describe('sbb-dialog', () => {
expect(didOpen).toHaveReceivedEventTimes(1);
await page.waitForChanges();

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

await innerElement.callMethod('open');
await page.waitForChanges();
Expand All @@ -380,7 +364,7 @@ describe('sbb-dialog', () => {
expect(didOpen).toHaveReceivedEventTimes(2);
await page.waitForChanges();

expect(innerDialog).toHaveAttribute('open');
expect(innerElement).toEqualAttribute('data-state', 'opened');

// Simulate a click on the inner dialog's backdrop
await page.mouse.click(1, 1);
Expand All @@ -395,9 +379,6 @@ describe('sbb-dialog', () => {
await page.waitForChanges();

expect(innerElement).toEqualAttribute('data-state', 'closed');
expect(innerDialog).not.toHaveAttribute('open');

expect(element).toEqualAttribute('data-state', 'opened');
expect(outerDialog).toHaveAttribute('open');
});
});
Loading