Skip to content

Commit

Permalink
fix(sbb-radio-button): pr feedbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
TomMenga committed Oct 23, 2024
1 parent 56d5ba4 commit d05caca
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 15 deletions.
24 changes: 17 additions & 7 deletions src/elements/core/mixins/form-associated-radio-button-mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,18 @@ export declare class SbbFormAssociatedRadioButtonMixinType
}

/**
* TODO add docs (maybe move to new file)
* A static registry that holds a collection of `radio-buttons`, grouped by `name`.
* It is mainly used to support the standalone groups of radios.
* @internal
*/
export class RadioButtonRegistry {
private static _registry: { [x: string]: SbbFormAssociatedRadioButtonMixinType[] } = {};

private constructor() {}

/**
* Adds @radio to the @groupName group. Checks for duplicates
*/
public static addRadioToGroup(
radio: SbbFormAssociatedRadioButtonMixinType,
groupName: string,
Expand All @@ -60,6 +64,9 @@ export class RadioButtonRegistry {
this._registry[groupName].push(radio);
}

/**
* Removes @radio from the @groupName group.
*/
public static removeRadioFromGroup(
radio: SbbFormAssociatedRadioButtonMixinType,
groupName: string,
Expand All @@ -75,6 +82,9 @@ export class RadioButtonRegistry {
}
}

/**
* Return an array of radios that belong to @groupName
*/
public static getRadios(groupName: string): SbbFormAssociatedRadioButtonMixinType[] {
return this._registry[groupName] ?? [];
}
Expand Down Expand Up @@ -180,7 +190,7 @@ export const SbbFormAssociatedRadioButtonMixin = <T extends Constructor<LitEleme

/**
* Called on `value` change
* If I'm checked, update the value. Otherwise, do nothing.
* If 'checked', update the value. Otherwise, do nothing.
*/
protected override updateFormValue(): void {
if (this.checked) {
Expand All @@ -190,22 +200,22 @@ export const SbbFormAssociatedRadioButtonMixin = <T extends Constructor<LitEleme

/**
* Called on `checked` change
* If I'm checked, set the value. Otherwise, reset it.
* If 'checked', update the value. Otherwise, reset it.
*/
protected updateFormValueOnCheckedChange(): void {
this.internals.setFormValue(this.checked ? this.value : null);
}

/**
* Only a single radio should be focusable in the group. Defined as:
* - the checked radios;
* - the checked radio;
* - the first non-disabled radio in DOM order;
*/
protected updateFocusableRadios(): void {
if (!this._didLoad) {
return;
}
const radios = this._orderedGrouperRadios();
const radios = this._orderedGroupedRadios();
const checkedIndex = radios.findIndex((r) => r.checked && !r.disabled && !r.formDisabled);
const focusableIndex =
checkedIndex !== -1
Expand Down Expand Up @@ -275,7 +285,7 @@ export const SbbFormAssociatedRadioButtonMixin = <T extends Constructor<LitEleme
/**
* Return the grouped radios in DOM order
*/
private _orderedGrouperRadios(groupName = this.name): SbbFormAssociatedRadioButtonElement[] {
private _orderedGroupedRadios(groupName = this.name): SbbFormAssociatedRadioButtonElement[] {
return Array.from(
(this.form ?? document).querySelectorAll<SbbFormAssociatedRadioButtonElement>(
`:is(sbb-radio-button, sbb-radio-button-panel)[name="${groupName}"]`,
Expand All @@ -289,7 +299,7 @@ export const SbbFormAssociatedRadioButtonMixin = <T extends Constructor<LitEleme
}
evt.preventDefault();

const enabledRadios = this._orderedGrouperRadios().filter(
const enabledRadios = this._orderedGroupedRadios().filter(
(r) => !r.disabled && !r.formDisabled,
);
const current: number = enabledRadios.indexOf(this);
Expand Down
4 changes: 2 additions & 2 deletions src/elements/radio-button/common/radio-button-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ export const SbbRadioButtonCommonElementMixin = <T extends Constructor<LitElemen
}

/**
* Set the radio-button as 'checked'. If 'allowEmptySelection', toggle the checked property.
* Emits events
* Set the radio-button as 'checked'; if 'allowEmptySelection', toggle the checked property.
* In both cases it emits the change events.
*/
public select(): void {
if (this.disabled || this.formDisabled) {
Expand Down
6 changes: 3 additions & 3 deletions src/elements/radio-button/radio-button-panel/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ The component's label can be displayed in bold using the `sbb-text--bold` class

## Methods

| Name | Privacy | Description | Parameters | Return | Inherited From |
| -------- | ------- | ------------------------------------------------------------------------------------------------------ | ---------- | ------ | -------------------------------- |
| `select` | public | Set the radio-button as 'checked'. If 'allowEmptySelection', toggle the checked property. Emits events | | `void` | SbbRadioButtonCommonElementMixin |
| Name | Privacy | Description | Parameters | Return | Inherited From |
| -------- | ------- | ----------------------------------------------------------------------------------------------------------------------------------- | ---------- | ------ | -------------------------------- |
| `select` | public | Set the radio-button as 'checked'; if 'allowEmptySelection', toggle the checked property. In both cases it emits the change events. | | `void` | SbbRadioButtonCommonElementMixin |

## Events

Expand Down
6 changes: 3 additions & 3 deletions src/elements/radio-button/radio-button/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ The component's label can be displayed in bold using the `sbb-text--bold` class

## Methods

| Name | Privacy | Description | Parameters | Return | Inherited From |
| -------- | ------- | ------------------------------------------------------------------------------------------------------ | ---------- | ------ | -------------------------------- |
| `select` | public | Set the radio-button as 'checked'. If 'allowEmptySelection', toggle the checked property. Emits events | | `void` | SbbRadioButtonCommonElementMixin |
| Name | Privacy | Description | Parameters | Return | Inherited From |
| -------- | ------- | ----------------------------------------------------------------------------------------------------------------------------------- | ---------- | ------ | -------------------------------- |
| `select` | public | Set the radio-button as 'checked'; if 'allowEmptySelection', toggle the checked property. In both cases it emits the change events. | | `void` | SbbRadioButtonCommonElementMixin |

## Events

Expand Down

0 comments on commit d05caca

Please sign in to comment.