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

Switch: Completely remove aria-checked from switch, set id on input to be the same as label, replace brittle unit test with more precise e2e test #2455

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,12 @@ describe('modus-switch', () => {

await page.setContent('<modus-switch></modus-switch>');
const component = await page.find('modus-switch');
const input = await page.find('modus-switch >>> input');
expect(await input.getProperty('checked')).toBeFalsy();

component.setProperty('checked', 'true');
await page.waitForChanges();

const input = await page.find('modus-switch >>> input');
expect(await input.getProperty('checked')).toBeTruthy();
});

Expand Down Expand Up @@ -94,14 +95,12 @@ describe('modus-switch', () => {
const input = await page.find('modus-switch >>> input');
expect(await modusSwitch.getProperty('checked')).toBeTruthy();
expect(await input.getProperty('checked')).toBeTruthy();
expect(await input.getAttribute('aria-checked').toLowerCase()).toEqual('true');

await element.click();
await page.waitForChanges();

expect(await modusSwitch.getProperty('checked')).toBeFalsy();
expect(await input.getProperty('checked')).toBeFalsy();
expect(await input.getAttribute('aria-checked').toLowerCase()).toEqual('false');
});
it('renders with medium size', async () => {
const page = await newE2EPage();
Expand All @@ -119,6 +118,28 @@ describe('modus-switch', () => {
expect(element).toHaveClass('small');
});

it('sets tabindex to -1 when disabled', async () => {
const page = await newE2EPage();
await page.setContent('<modus-switch disabled></modus-switch>');

const element = await page.find('modus-switch >>> .modus-switch');
expect(element).toHaveAttribute('tabindex');
expect(element.getAttribute('tabindex')).toEqual('-1');
});

it('renders with "for" on label equal to "id" on input', async () => {
const page = await newE2EPage();
await page.setContent('<modus-switch label="test label"></modus-switch>');

const input = await page.find('modus-switch >>> input');
const id = await input.getAttribute('id');

const label = await page.find('modus-switch >>> label');
const forAttr = await label.getAttribute('for');

expect(id).toEqual(forAttr);
});

it('renders aria-label on input when set', async () => {
const page = await newE2EPage();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,7 @@ describe('modus-switch', () => {
<div class="switch">
<span class="slider"></span>
</div>
<input aria-checked="false" role="switch" type="checkbox">
</div>
</mock:shadow-root>
</modus-switch>
`);
});

it('sets tabindex to -1 when disabled', async () => {
const page = await newSpecPage({
components: [ModusSwitch],
html: '<modus-switch disabled></modus-switch>',
});
expect(page.root).toEqualHtml(`
<modus-switch disabled>
<mock:shadow-root>
<div class="disabled medium modus-switch" tabindex="-1">
<div class="switch">
<span class="slider"></span>
</div>
<input aria-checked="false" role="switch" aria-disabled="true" disabled type="checkbox">
<input id="mwc_id_0_switch_input" role="switch" type="checkbox">
</div>
</mock:shadow-root>
</modus-switch>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// eslint-disable-next-line
import { generateElementId } from '../../utils/utils';
// eslint-disable-next-line
import { Component, Prop, h, Event, EventEmitter, Listen } from '@stencil/core';
austinoneil marked this conversation as resolved.
Show resolved Hide resolved

@Component({
Expand All @@ -25,6 +27,7 @@ export class ModusSwitch {
/** An event that fires on switch click. */
@Event() switchClick: EventEmitter<boolean>;

private generatedId = generateElementId() + '_switch_input';
checkboxInput: HTMLInputElement;

@Listen('keydown')
Expand Down Expand Up @@ -69,15 +72,15 @@ export class ModusSwitch {
<span class={`slider`}></span>
</div>
<input
aria-checked={String(this.checked)}
aria-disabled={this.disabled ? 'true' : undefined}
aria-label={this.ariaLabel || undefined}
checked={this.checked}
disabled={this.disabled}
id={this.generatedId}
ref={(el) => (this.checkboxInput = el as HTMLInputElement)}
role="switch"
type="checkbox"></input>
{this.label ? <label>{this.label}</label> : null}
{this.label ? <label htmlFor={this.generatedId}>{this.label}</label> : null}
</div>
);
}
Expand Down