Skip to content

Commit

Permalink
fix(sbb-checkbox, sbb-toggle-check): enable attribute mutation after …
Browse files Browse the repository at this point in the history 
…form reset (#2505)
  • Loading branch information
@jeripeierSBB
jeripeierSBB committed Mar 19, 2024
1 parent 5226e9b commit 6bd8924
Show file tree
Hide file tree
Showing 4 changed files with 283 additions and 67 deletions.
132 changes: 122 additions & 10 deletions src/components/checkbox/checkbox/checkbox.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('sbb-checkbox', () => {
expect(snapshot.required).to.be.undefined;
});

it('should reflect accessibility tree setting required attribute', async () => {
it('should reflect accessibility tree setting required attribute to true', async () => {
element.toggleAttribute('required', true);
await waitForLitRender(element);

Expand All @@ -100,18 +100,23 @@ describe('sbb-checkbox', () => {
if (!isChromium()) {
expect(snapshot.required).to.be.true;
}
});

it('should reflect accessibility tree setting required attribute to false', async () => {
element.toggleAttribute('required', true);
await waitForLitRender(element);

element.removeAttribute('required');
await waitForLitRender(element);

const snapshotAfterRemoved = (await a11ySnapshot({
const snapshot = (await a11ySnapshot({
selector: 'sbb-checkbox',
})) as unknown as CheckboxAccessibilitySnapshot;

expect(snapshotAfterRemoved.required).not.to.be.ok;
expect(snapshot.required).not.to.be.ok;
});

it('should reflect accessibility tree setting required property', async () => {
it('should reflect accessibility tree setting required property to true', async () => {
element.required = true;
await waitForLitRender(element);

Expand All @@ -123,15 +128,20 @@ describe('sbb-checkbox', () => {
if (!isChromium()) {
expect(snapshot.required).to.be.true;
}
});

it('should reflect accessibility tree setting required property to false', async () => {
element.required = true;
await waitForLitRender(element);

element.required = false;
await waitForLitRender(element);

const snapshotAfterRemoved = (await a11ySnapshot({
const snapshot = (await a11ySnapshot({
selector: 'sbb-checkbox',
})) as unknown as CheckboxAccessibilitySnapshot;

expect(snapshotAfterRemoved.required).not.to.be.ok;
expect(snapshot.required).not.to.be.ok;
});

it('should should restore form state on formStateRestoreCallback()', async () => {
Expand Down Expand Up @@ -270,7 +280,7 @@ describe('sbb-checkbox', () => {
});
});

it('should reflect state after programmatic change ', async () => {
it('should reflect state after programmatic change', async () => {
element.checked = true;
await waitForLitRender(form);

Expand All @@ -296,7 +306,7 @@ describe('sbb-checkbox', () => {
});
});

it('should no longer interpret attribute after programmatic change ', async () => {
it('should no longer interpret attribute after programmatic change', async () => {
element.checked = true;
await waitForLitRender(form);

Expand All @@ -316,6 +326,57 @@ describe('sbb-checkbox', () => {
});
});

it('should interpret attribute after programmatic change and reset', async () => {
// Simulate programmatic change (according to previous test, now attribute mutation is blocked)
element.checked = true;
await waitForLitRender(form);

// When resetting the form
formResetButton.click();
await waitForLitRender(form);

// State should be reset
await assertState({
ariaChecked: false,
checkedAttribute: false,
checkedProperty: false,
indeterminateProperty: false,
inputEventCount: 0,
changeEventCount: 0,
});

// When performing
element.toggleAttribute('checked', true);
await waitForLitRender(form);

// Attribute should be considered
await assertState({
ariaChecked: true,
checkedAttribute: true,
checkedProperty: true,
indeterminateProperty: false,
inputEventCount: 0,
changeEventCount: 0,
});

// When we are manipulating again
element.checked = false;
await waitForLitRender(form);

// Attribute mutation should be blocked again
element.toggleAttribute('checked', true);
await waitForLitRender(form);

await assertState({
ariaChecked: false,
checkedAttribute: true,
checkedProperty: false,
indeterminateProperty: false,
inputEventCount: 0,
changeEventCount: 0,
});
});

it('should reflect state after adding attribute', async () => {
element.toggleAttribute('checked', true);
await waitForLitRender(form);
Expand Down Expand Up @@ -699,7 +760,7 @@ describe('sbb-checkbox', () => {
});
});

it('should reflect state after programmatic change ', async () => {
it('should reflect state after programmatic change', async () => {
element.checked = false;
await waitForLitRender(form);

Expand All @@ -725,7 +786,7 @@ describe('sbb-checkbox', () => {
});
});

it('should no longer interpret attribute after programmatic change ', async () => {
it('should no longer interpret attribute after programmatic change', async () => {
element.checked = false;
await waitForLitRender(form);

Expand All @@ -743,6 +804,57 @@ describe('sbb-checkbox', () => {
});
});

it('should interpret attribute after programmatic change and reset', async () => {
// Simulate programmatic change (according to previous test, now attribute mutation is blocked)
element.checked = false;
await waitForLitRender(form);

// When resetting the form
formResetButton.click();
await waitForLitRender(form);

// State should be reset
await assertState({
ariaChecked: true,
checkedAttribute: true,
checkedProperty: true,
indeterminateProperty: false,
inputEventCount: 0,
changeEventCount: 0,
});

// When performing
element.toggleAttribute('checked', false);
await waitForLitRender(form);

// Attribute should be considered
await assertState({
ariaChecked: false,
checkedAttribute: false,
checkedProperty: false,
indeterminateProperty: false,
inputEventCount: 0,
changeEventCount: 0,
});

// When we are manipulating again
element.checked = true;
await waitForLitRender(form);

// Attribute mutation should be blocked again
element.toggleAttribute('checked', false);
await waitForLitRender(form);

await assertState({
ariaChecked: true,
checkedAttribute: false,
checkedProperty: true,
indeterminateProperty: false,
inputEventCount: 0,
changeEventCount: 0,
});
});

it('should reflect state after removing attribute', async () => {
element.removeAttribute('checked');
await waitForLitRender(form);
Expand Down
1 change: 1 addition & 0 deletions src/components/core/common-behaviors/form-associated-checkbox-mixin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export const SbbFormAssociatedCheckboxMixin = <T extends Constructor<LitElement>
*/
public override formResetCallback(): void {
this.checked = this.hasAttribute('checked');
this._attributeMutationBlocked = false;
}

/**
Expand Down
90 changes: 44 additions & 46 deletions src/components/core/common-behaviors/form-associated-mixin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { property, state } from 'lit/decorators.js';
import type { Constructor } from './constructor';

export declare abstract class SbbFormAssociatedMixinType {
protected readonly internals: ElementInternals;

public get form(): HTMLFormElement | null;
public get name(): string;
public set name(value: string);
Expand All @@ -17,11 +15,13 @@ export declare abstract class SbbFormAssociatedMixinType {
public get validationMessage(): string;
public get willValidate(): boolean;

protected formDisabled: boolean;
protected readonly internals: ElementInternals;

public checkValidity(): boolean;
public reportValidity(): boolean;

public formAssociatedCallback?(form: HTMLFormElement | null): void;

public formDisabledCallback(disabled: boolean): void;
public abstract formResetCallback(): void;
public abstract formStateRestoreCallback(
Expand All @@ -30,8 +30,6 @@ export declare abstract class SbbFormAssociatedMixinType {
): void;

protected updateFormValue(): void;

protected formDisabled: boolean;
}

/**
Expand All @@ -47,8 +45,37 @@ export const SbbFormAssociatedMixin = <T extends Constructor<LitElement>>(
{
public static formAssociated = true;

/**
* Returns the form owner of internals target element.
*/
public get form(): HTMLFormElement | null {
return this.internals.form;
}

/** Name of the form element. Will be read from name attribute. */
@property()
public set name(name: string) {
this.setAttribute('name', `${name}`);
}
public get name(): string {
return this.getAttribute('name') ?? '';
}

/** @internal */
protected readonly internals: ElementInternals = this.attachInternals();
public get type(): string {
return this.localName;
}

/** Value of the form element. */
@property()
public set value(value: string | null) {
this._value = value;
this.updateFormValue();
}
public get value(): string | null {
return this._value;
}
private _value: string | null = null;

/**
* Returns the ValidityState object for internals target element.
Expand Down Expand Up @@ -79,37 +106,11 @@ export const SbbFormAssociatedMixin = <T extends Constructor<LitElement>>(
return this.internals.willValidate;
}

/**
* Returns the form owner of internals target element.
*/
public get form(): HTMLFormElement | null {
return this.internals.form;
}

/** Name of the form element. Will be read from name attribute. */
@property()
public set name(name: string) {
this.setAttribute('name', `${name}`);
}
public get name(): string {
return this.getAttribute('name') ?? '';
}

/** @internal */
public get type(): string {
return this.localName;
}
protected readonly internals: ElementInternals = this.attachInternals();

/** Value of the form element. */
@property()
public set value(value: string | null) {
this._value = value;
this.updateFormValue();
}
public get value(): string | null {
return this._value;
}
private _value: string | null = null;
/** Whenever a surrounding form or fieldset is changing its disabled state. */
@state() protected formDisabled: boolean = false;

/**
* Returns true if internals target element has no validity problems; false otherwise.
Expand All @@ -132,6 +133,14 @@ export const SbbFormAssociatedMixin = <T extends Constructor<LitElement>>(
return this.internals.reportValidity();
}

/**
* Called when the associated form element changes.
* ElementInternals.form returns the associated from element.
*
* @internal
*/
public formAssociatedCallback?(form: HTMLFormElement | null): void;

/**
* Is called whenever a surrounding form / fieldset changes disabled state.
* @param disabled
Expand Down Expand Up @@ -163,21 +172,10 @@ export const SbbFormAssociatedMixin = <T extends Constructor<LitElement>>(
reason: FormRestoreReason,
): void;

/**
* Called when the associated form element changes.
* ElementInternals.form returns the associated from element.
*
* @internal
*/
public formAssociatedCallback?(form: HTMLFormElement | null): void;

/** Should be called when form value is changed. */
protected updateFormValue(): void {
this.internals.setFormValue(this.value);
}

/** Whenever a surrounding form or fieldset is changing its disabled state. */
@state() protected formDisabled: boolean = false;
}
return SbbFormAssociatedElement as unknown as Constructor<SbbFormAssociatedMixinType> & T;
};
Expand Down

0 comments on commit 6bd8924

Please sign in to comment.