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

fix(sbb-toggle): fix pill position on value change and initial rendering #3015

Merged
merged 2 commits into from
Aug 22, 2024
Merged
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
1 change: 1 addition & 0 deletions src/elements/toggle/toggle-option/toggle-option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export class SbbToggleOptionElement extends SbbIconNameMixin(LitElement) {

private _uncheckOtherOptions(): void {
this._toggle?.options.filter((o) => o !== this).forEach((o) => (o.checked = false));
this._toggle?.updatePillPosition(false);
}

private _handleDisabledChange(): void {
Expand Down
5 changes: 3 additions & 2 deletions src/elements/toggle/toggle/toggle.snapshot.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { expect } from '@open-wc/testing';
import { html } from 'lit/static-html.js';

import { fixture, testA11yTreeSnapshot } from '../../core/testing/private.js';
import type { SbbToggleElement } from '../toggle.js';

import type { SbbToggleElement } from './toggle.js';

import './toggle.js';
import '../toggle-option.js';
Expand All @@ -21,7 +22,7 @@ describe(`sbb-toggle`, () => {
});

it('DOM', async () => {
await expect(element).dom.to.be.equalSnapshot();
await expect(element).dom.to.be.equalSnapshot({ ignoreAttributes: ['style'] });
});

it('Shadow DOM', async () => {
Expand Down
35 changes: 35 additions & 0 deletions src/elements/toggle/toggle/toggle.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,24 @@ describe(`sbb-toggle`, () => {
firstOption: SbbToggleOptionElement,
secondOption: SbbToggleOptionElement;

function assertPillLeft(): void {
expect(
parseInt(window.getComputedStyle(element).getPropertyValue('--sbb-toggle-option-left'), 10),
).to.be.equal(0);
expect(
parseInt(window.getComputedStyle(element).getPropertyValue('--sbb-toggle-option-right'), 10),
).to.be.greaterThan(0);
}

function assertPillRight(): void {
expect(
parseInt(window.getComputedStyle(element).getPropertyValue('--sbb-toggle-option-left'), 10),
).to.be.greaterThan(0);
expect(
parseInt(window.getComputedStyle(element).getPropertyValue('--sbb-toggle-option-right'), 10),
).to.be.equal(0);
}

describe('basics', () => {
beforeEach(async () => {
element = await fixture(html`
Expand Down Expand Up @@ -45,20 +63,25 @@ describe(`sbb-toggle`, () => {
`);
secondOption = element.querySelectorAll('sbb-toggle-option')[1];
expect(secondOption).to.have.attribute('checked');

assertPillRight();
});

it('should select the correct option by setting value programmatically', async () => {
element.value = 'Value two';
await waitForLitRender(element);

expect(firstOption).not.to.have.attribute('checked');
expect(secondOption).to.have.attribute('checked');
assertPillRight();
});

it('should update the value of the sbb-toggle when the value of a checked option changes', async () => {
firstOption.value = 'Changed';
await waitForLitRender(element);

expect(element.value).to.be.equal(firstOption.value);
assertPillLeft();
});

it('should not update the value of the sbb-toggle when the value of a unchecked option changes', async () => {
Expand All @@ -72,17 +95,21 @@ describe(`sbb-toggle`, () => {
describe('checked', () => {
it('should initially have the checked attributes on the first option by default', async () => {
expect(firstOption).to.have.attribute('checked');
assertPillLeft();
});

it('should initially have the checked property set to true on the first option by default', async () => {
expect(firstOption.checked).to.be.equal(true);
assertPillLeft();
});

it('should select the correct option by setting checked on the option', async () => {
secondOption.checked = true;
await waitForLitRender(secondOption);

expect(secondOption).to.have.attribute('checked');
expect(firstOption).not.to.have.attribute('checked');
assertPillRight();
});

it('should select the correct option by setting the checked attribute on the option', async () => {
Expand All @@ -95,13 +122,15 @@ describe(`sbb-toggle`, () => {
secondOption = element.querySelectorAll('sbb-toggle-option')[1];

expect(secondOption.checked).to.be.equal(true);
assertPillRight();
});

it('should check first option when unchecking all options', async () => {
firstOption.setAttribute('checked', 'false');
await waitForLitRender(firstOption);

expect(firstOption.checked).to.be.equal(true);
assertPillLeft();
});
});

Expand Down Expand Up @@ -160,32 +189,38 @@ describe(`sbb-toggle`, () => {

it('selects option on click', async () => {
expect(firstOption).to.have.attribute('checked');
assertPillLeft();

secondOption.click();
await waitForLitRender(secondOption);

expect(secondOption).to.have.attribute('checked');
expect(firstOption).not.to.have.attribute('checked');
assertPillRight();
});

it('selects option on checked attribute change', async () => {
expect(firstOption).to.have.attribute('checked');
assertPillLeft();

secondOption.toggleAttribute('checked', true);
await waitForLitRender(element);

expect(secondOption).to.have.attribute('checked');
expect(firstOption).not.to.have.attribute('checked');
assertPillRight();
});

it('selects option on checked property change', async () => {
expect(firstOption.checked).to.equal(true);
assertPillLeft();

secondOption.checked = true;
await waitForLitRender(element);

expect(firstOption.checked).to.equal(false);
expect(secondOption.checked).to.equal(true);
assertPillRight();
});

it('dispatches event on option change', async () => {
Expand Down
12 changes: 6 additions & 6 deletions src/elements/toggle/toggle/toggle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,7 @@ export class SbbToggleElement extends LitElement {
}

private _loaded: boolean = false;
private _toggleResizeObserver = new AgnosticResizeObserver(() =>
this._setCheckedPillPosition(true),
);
private _toggleResizeObserver = new AgnosticResizeObserver(() => this.updatePillPosition(true));

private _valueChanged(value: any | undefined): void {
const options = this.options;
Expand All @@ -104,7 +102,7 @@ export class SbbToggleElement extends LitElement {
if (!selectedOption.checked) {
selectedOption.checked = true;
}
this._setCheckedPillPosition(false);
this.updatePillPosition(false);
}

private _updateDisabled(): void {
Expand All @@ -130,7 +128,8 @@ export class SbbToggleElement extends LitElement {

private _abort = new SbbConnectedAbortController(this);

private _setCheckedPillPosition(resizing: boolean): void {
/** @internal */
public updatePillPosition(resizing: boolean): void {
if (!this._loaded) {
return;
}
Expand Down Expand Up @@ -173,6 +172,7 @@ export class SbbToggleElement extends LitElement {

await this.updateComplete;
this._loaded = true;
this.updatePillPosition(false);
}

public override disconnectedCallback(): void {
Expand All @@ -186,7 +186,7 @@ export class SbbToggleElement extends LitElement {
}

private _handleInput(): void {
this._setCheckedPillPosition(false);
this.updatePillPosition(false);
this._change.emit();
this._didChange.emit();
}
Expand Down
Loading