Skip to content

Commit

Permalink
fix: fix type of form associated controls (#3242)
Browse files Browse the repository at this point in the history
Closes #3238
  • Loading branch information
jeripeierSBB authored Nov 25, 2024
1 parent edcdec1 commit cbf839c
Show file tree
Hide file tree
Showing 19 changed files with 128 additions and 67 deletions.
27 changes: 14 additions & 13 deletions src/elements/checkbox/checkbox-panel/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,20 @@ If you don't want the label to appear next to the checkbox, you can use `aria-la

## Properties

| Name | Attribute | Privacy | Type | Default | Description |
| --------------- | --------------- | ------- | --------------------------------- | --------- | -------------------------------------------------------------- |
| `borderless` | `borderless` | public | `boolean` | `false` | Whether the unselected panel has a border. |
| `checked` | `checked` | public | `boolean` | `false` | Whether the checkbox is checked. |
| `color` | `color` | public | `'white' \| 'milk'` | `'white'` | The background color of the panel. |
| `disabled` | `disabled` | public | `boolean` | `false` | Whether the component is disabled. |
| `form` | - | public | `HTMLFormElement \| null` | | Returns the form owner of the internals of the target element. |
| `group` | - | public | `SbbCheckboxGroupElement \| null` | `null` | Reference to the connected checkbox group. |
| `indeterminate` | `indeterminate` | public | `boolean` | `false` | Whether the checkbox is indeterminate. |
| `name` | `name` | public | `string` | | Name of the form element. Will be read from name attribute. |
| `required` | `required` | public | `boolean` | `false` | Whether the component is required. |
| `size` | `size` | public | `SbbPanelSize` | `'m'` | Size variant. |
| `value` | `value` | public | `string \| null` | `null` | Value of the form element. |
| Name | Attribute | Privacy | Type | Default | Description |
| --------------- | --------------- | ------- | --------------------------------- | ------------ | -------------------------------------------------------------- |
| `borderless` | `borderless` | public | `boolean` | `false` | Whether the unselected panel has a border. |
| `checked` | `checked` | public | `boolean` | `false` | Whether the checkbox is checked. |
| `color` | `color` | public | `'white' \| 'milk'` | `'white'` | The background color of the panel. |
| `disabled` | `disabled` | public | `boolean` | `false` | Whether the component is disabled. |
| `form` | - | public | `HTMLFormElement \| null` | | Returns the form owner of the internals of the target element. |
| `group` | - | public | `SbbCheckboxGroupElement \| null` | `null` | Reference to the connected checkbox group. |
| `indeterminate` | `indeterminate` | public | `boolean` | `false` | Whether the checkbox is indeterminate. |
| `name` | `name` | public | `string` | | Name of the form element. Will be read from name attribute. |
| `required` | `required` | public | `boolean` | `false` | Whether the component is required. |
| `size` | `size` | public | `SbbPanelSize` | `'m'` | Size variant. |
| `type` | - | public | `string` | `'checkbox'` | Form type of element. |
| `value` | `value` | public | `string \| null` | `null` | Value of the form element. |

## Events

Expand Down
27 changes: 14 additions & 13 deletions src/elements/checkbox/checkbox/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,20 @@ If you don't want the label to appear next to the checkbox, you can use `aria-la

## Properties

| Name | Attribute | Privacy | Type | Default | Description |
| --------------- | ---------------- | ------- | --------------------------------- | ------- | -------------------------------------------------------------------------------------------------------------------------------- |
| `checked` | `checked` | public | `boolean` | `false` | Whether the checkbox is checked. |
| `disabled` | `disabled` | public | `boolean` | `false` | Whether the component is disabled. |
| `form` | - | public | `HTMLFormElement \| null` | | Returns the form owner of the internals of the target element. |
| `group` | - | public | `SbbCheckboxGroupElement \| null` | `null` | Reference to the connected checkbox group. |
| `iconName` | `icon-name` | public | `string` | `''` | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch. |
| `iconPlacement` | `icon-placement` | public | `SbbIconPlacement` | `'end'` | The label position relative to the labelIcon. Defaults to end |
| `indeterminate` | `indeterminate` | public | `boolean` | `false` | Whether the checkbox is indeterminate. |
| `name` | `name` | public | `string` | | Name of the form element. Will be read from name attribute. |
| `required` | `required` | public | `boolean` | `false` | Whether the component is required. |
| `size` | `size` | public | `SbbCheckboxSize` | `'m'` | Size variant. |
| `value` | `value` | public | `string \| null` | `null` | Value of the form element. |
| Name | Attribute | Privacy | Type | Default | Description |
| --------------- | ---------------- | ------- | --------------------------------- | ------------ | -------------------------------------------------------------------------------------------------------------------------------- |
| `checked` | `checked` | public | `boolean` | `false` | Whether the checkbox is checked. |
| `disabled` | `disabled` | public | `boolean` | `false` | Whether the component is disabled. |
| `form` | - | public | `HTMLFormElement \| null` | | Returns the form owner of the internals of the target element. |
| `group` | - | public | `SbbCheckboxGroupElement \| null` | `null` | Reference to the connected checkbox group. |
| `iconName` | `icon-name` | public | `string` | `''` | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch. |
| `iconPlacement` | `icon-placement` | public | `SbbIconPlacement` | `'end'` | The label position relative to the labelIcon. Defaults to end |
| `indeterminate` | `indeterminate` | public | `boolean` | `false` | Whether the checkbox is indeterminate. |
| `name` | `name` | public | `string` | | Name of the form element. Will be read from name attribute. |
| `required` | `required` | public | `boolean` | `false` | Whether the component is required. |
| `size` | `size` | public | `SbbCheckboxSize` | `'m'` | Size variant. |
| `type` | - | public | `string` | `'checkbox'` | Form type of element. |
| `value` | `value` | public | `string \| null` | `null` | Value of the form element. |

## Events

Expand Down
1 change: 1 addition & 0 deletions src/elements/checkbox/common/checkbox-common.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ describe(`checkbox common behaviors`, () => {
})) as unknown as CheckboxAccessibilitySnapshot;

expect(snapshot.role).to.equal('checkbox');
expect(element.type).to.be.equal('checkbox');

expect(snapshot.checked, `ariaChecked in ${JSON.stringify(snapshot)}`).to.be.equal(
isFirefox && assertions.ariaChecked === false ? undefined : assertions.ariaChecked,
Expand Down
8 changes: 8 additions & 0 deletions src/elements/core/mixins/form-associated-checkbox-mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ export const SbbFormAssociatedCheckboxMixin = <T extends Constructor<LitElement>
}
private _checked: boolean = false;

/**
* Form type of element.
* @default 'checkbox'
*/
public override get type(): string {
return 'checkbox';
}

protected constructor() {
super();
/** @internal */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ export const SbbFormAssociatedRadioButtonMixin = <T extends Constructor<LitEleme
@property({ type: Boolean })
public accessor checked: boolean = false;

/**
* Form type of element.
* @default 'radio'
*/
public override get type(): string {
return 'radio';
}

protected abort = new SbbConnectedAbortController(this);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ describe('sbb-file-selector common', () => {
expect(e.lastModified, `file - lastModified - ${i}`).to.be.equal(
Array.from(input.files!)[i].lastModified,
);

expect(element.role, 'compare to native role').to.be.equal(input.role);
expect(element.type, 'compare to native type').to.be.equal(input.type);
});

// Compare formData
Expand Down
8 changes: 8 additions & 0 deletions src/elements/file-selector/common/file-selector-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,14 @@ export const SbbFileSelectorCommonElementMixin = <T extends Constructor<LitEleme
}
private _files: Readonly<File>[] = [];

/**
* Form type of element.
* @default 'file'
*/
public override get type(): string {
return 'file';
}

/** An event which is emitted each time the file list changes. */
private _fileChangedEvent: EventEmitter<Readonly<File>[]> = new EventEmitter(
this,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ It's suggested to have a different value for each variant, e.g.:
| `name` | `name` | public | `string` | | Name of the form element. Will be read from name attribute. |
| `size` | `size` | public | `'s' \| 'm'` | `'m'` | Size variant, either s or m. |
| `titleContent` | `title-content` | public | `string` | `''` | The title displayed in `dropzone` variant. |
| `type` | - | public | `string` | `'file'` | Form type of element. |
| `value` | `value` | public | `string \| null` | `null` | The path of the first selected file. Empty string ('') if no file is selected |

## Methods
Expand Down
1 change: 1 addition & 0 deletions src/elements/file-selector/file-selector/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ It's suggested to have a different value for each variant, e.g.:
| `multipleMode` | `multiple-mode` | public | `'default' \| 'persistent'` | `'default'` | Whether the newly added files should override the previously added ones. |
| `name` | `name` | public | `string` | | Name of the form element. Will be read from name attribute. |
| `size` | `size` | public | `'s' \| 'm'` | `'m'` | Size variant, either s or m. |
| `type` | - | public | `string` | `'file'` | Form type of element. |
| `value` | `value` | public | `string \| null` | `null` | The path of the first selected file. Empty string ('') if no file is selected |

## Methods
Expand Down
16 changes: 10 additions & 6 deletions src/elements/radio-button/common/radio-button-common.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { SbbRadioButtonElement } from '../radio-button.js';
import '../radio-button.js';
import '../radio-button-panel.js';

interface CheckboxAccessibilitySnapshot {
interface RadioButtonAccessibilitySnapshot {
checked: boolean;
role: string;
disabled: boolean;
Expand Down Expand Up @@ -81,7 +81,7 @@ describe(`radio-button common behaviors`, () => {
it('should reflect aria-required false', async () => {
const snapshot = (await a11ySnapshot({
selector: selector,
})) as unknown as CheckboxAccessibilitySnapshot;
})) as unknown as RadioButtonAccessibilitySnapshot;

expect(snapshot.required).to.be.undefined;
});
Expand All @@ -95,7 +95,7 @@ describe(`radio-button common behaviors`, () => {

const snapshot = (await a11ySnapshot({
selector: selector,
})) as unknown as CheckboxAccessibilitySnapshot;
})) as unknown as RadioButtonAccessibilitySnapshot;

// TODO: Recheck if it is working in Chromium
if (!isChromium) {
Expand All @@ -115,7 +115,7 @@ describe(`radio-button common behaviors`, () => {

const snapshot = (await a11ySnapshot({
selector: selector,
})) as unknown as CheckboxAccessibilitySnapshot;
})) as unknown as RadioButtonAccessibilitySnapshot;

expect(snapshot.required).not.to.be.ok;
});
Expand All @@ -129,7 +129,7 @@ describe(`radio-button common behaviors`, () => {

const snapshot = (await a11ySnapshot({
selector: selector,
})) as unknown as CheckboxAccessibilitySnapshot;
})) as unknown as RadioButtonAccessibilitySnapshot;

// TODO: Recheck if it is working in Chromium
if (!isChromium) {
Expand All @@ -149,7 +149,7 @@ describe(`radio-button common behaviors`, () => {

const snapshot = (await a11ySnapshot({
selector: selector,
})) as unknown as CheckboxAccessibilitySnapshot;
})) as unknown as RadioButtonAccessibilitySnapshot;

expect(snapshot.required).not.to.be.ok;
});
Expand Down Expand Up @@ -202,6 +202,10 @@ describe(`radio-button common behaviors`, () => {
);
});

// General form configuration
expect(elements[0].type, 'radio type').to.be.equal(nativeElements[0].type);
expect(elements[0].role, 'radio role').to.be.equal(nativeElements[0].role);

// Events
expect(inputSpy.count, `'input' event`).to.be.equal(nativeInputSpy.count);
expect(changeSpy.count, `'change' event`).to.be.equal(nativeChangeSpy.count);
Expand Down
1 change: 1 addition & 0 deletions src/elements/radio-button/radio-button-panel/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ The component's label can be displayed in bold using the `sbb-text--bold` class
| `name` | `name` | public | `string` | | Name of the form element. Will be read from name attribute. |
| `required` | `required` | public | `boolean` | `false` | Whether the component is required. |
| `size` | `size` | public | `SbbPanelSize` | `'m'` | Size variant. |
| `type` | - | public | `string` | `'radio'` | Form type of element. |
| `value` | `value` | public | `string \| null` | `null` | Value of the form element. |

## Methods
Expand Down
Loading

0 comments on commit cbf839c

Please sign in to comment.