Skip to content

Commit

Permalink
fix: review
Browse files Browse the repository at this point in the history
  • Loading branch information
jeripeierSBB committed Dec 16, 2024
1 parent 2747a54 commit 396272b
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ class SbbAutocompleteGridOptionElement extends SbbOptionBaseElement {
public static readonly events = {
selectionChange: 'autocompleteOptionSelectionChange',
optionSelected: 'autocompleteOptionSelected',
optionLabelChanged: 'optionLabelChanged',
} as const;

protected optionId = autocompleteGridOptionId;
Expand All @@ -46,15 +45,6 @@ class SbbAutocompleteGridOptionElement extends SbbOptionBaseElement {
SbbAutocompleteGridOptionElement.events.optionSelected,
);

/**
* @internal
* Emits when the label changed.
*/
protected optionLabelChanged: EventEmitter = new EventEmitter(
this,
SbbAutocompleteGridOptionElement.events.optionLabelChanged,
);

protected override onOptionAttributesChange(mutationsList: MutationRecord[]): void {
super.onOptionAttributesChange(mutationsList);
this.closest?.('sbb-autocomplete-grid-row')?.toggleAttribute(
Expand Down
5 changes: 1 addition & 4 deletions src/elements/option/option/option-base-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ abstract class SbbOptionBaseElement extends SbbDisabledMixin(
/** Emits when an option was selected by user. */
protected abstract optionSelected: EventEmitter;

/** Emits when the label changes. */
protected abstract optionLabelChanged: EventEmitter;

/** Whether to apply the negative styling */
@state() protected accessor negative = false;

Expand Down Expand Up @@ -266,7 +263,7 @@ abstract class SbbOptionBaseElement extends SbbDisabledMixin(

private _handleSlotChange(): void {
this.handleHighlightState();
this.optionLabelChanged.emit();
this.dispatchEvent(new Event('optionLabelChanged', { bubbles: true }));
}

protected override render(): TemplateResult {
Expand Down
10 changes: 0 additions & 10 deletions src/elements/option/option/option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class SbbOptionElement extends SbbOptionBaseElement {
public static readonly events = {
selectionChange: 'optionSelectionChange',
optionSelected: 'optionSelected',
optionLabelChanged: 'optionLabelChanged',
} as const;

protected optionId = `sbb-option`;
Expand All @@ -48,15 +47,6 @@ class SbbOptionElement extends SbbOptionBaseElement {
SbbOptionElement.events.optionSelected,
);

/**
* @internal
* Emits when the label changed.
*/
protected optionLabelChanged: EventEmitter = new EventEmitter(
this,
SbbOptionElement.events.optionLabelChanged,
);

private set _variant(state: SbbOptionVariant) {
if (state) {
this.setAttribute('data-variant', state);
Expand Down
46 changes: 22 additions & 24 deletions src/elements/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,23 +268,23 @@ class SbbSelectElement extends SbbUpdateSchedulerMixin(
/** Listens to option changes. */
private _onOptionLabelChanged(event: Event): void {
const target = event.target as SbbOptionElement;
const selectedOption = this._getSelectedOption();
const selected = this._getSelected();

if (
(!Array.isArray(selectedOption) && target !== selectedOption) ||
(Array.isArray(selectedOption) && !selectedOption.includes(target))
(!Array.isArray(selected) && target !== selected) ||
(Array.isArray(selected) && !selected.includes(target))
) {
return;
}

this._updateDisplayValue(selectedOption);
this._updateDisplayValue(selected);
}

private _updateDisplayValue(selectedOption: SbbOptionElement | SbbOptionElement[] | null): void {
if (Array.isArray(selectedOption)) {
this._displayValue = selectedOption.map((o) => o.textContent).join(', ') || null;
} else if (selectedOption) {
this._displayValue = selectedOption?.textContent || null;
private _updateDisplayValue(selected: SbbOptionElement | SbbOptionElement[] | null): void {
if (Array.isArray(selected)) {
this._displayValue = selected.map((o) => o.textContent).join(', ') || null;
} else if (selected) {
this._displayValue = selected?.textContent || null;
} else {
this._displayValue = null;
}
Expand All @@ -307,11 +307,11 @@ class SbbSelectElement extends SbbUpdateSchedulerMixin(
options
.filter((o) => !newValue.includes(o.value ?? o.getAttribute('value')))
.forEach((e) => (e.selected = false));
const selectedOptionElements = options.filter((o) =>
const selectedElements = options.filter((o) =>
newValue.includes(o.value ?? o.getAttribute('value')),
);
selectedOptionElements.forEach((o) => (o.selected = true));
this._updateDisplayValue(selectedOptionElements);
selectedElements.forEach((o) => (o.selected = true));
this._updateDisplayValue(selectedElements);
}
this._stateChange.emit({ type: 'value', value: newValue });
}
Expand Down Expand Up @@ -792,26 +792,24 @@ class SbbSelectElement extends SbbUpdateSchedulerMixin(
}
};

private _setValueFromSelectedOption(): void {
const selectedOption = this._getSelectedOption();
private _setValueFromSelected(): void {
const selected = this._getSelected();

if (Array.isArray(selectedOption)) {
if (selectedOption && selectedOption.length > 0) {
if (Array.isArray(selected)) {
if (selected && selected.length > 0) {
const value: string[] = [];
for (const option of selectedOption) {
for (const option of selected) {
value.push(option.value!);
}
this.value = value;
}
} else if (selectedOption) {
this._activeItemIndex = this._filteredOptions.findIndex(
(option) => option === selectedOption,
);
this.value = selectedOption.value;
} else if (selected) {
this._activeItemIndex = this._filteredOptions.findIndex((option) => option === selected);
this.value = selected.value;
}
}

private _getSelectedOption(): SbbOptionElement | SbbOptionElement[] | null {
private _getSelected(): SbbOptionElement | SbbOptionElement[] | null {
if (this.multiple) {
return this._filteredOptions.filter((option) => option.selected);
} else {
Expand Down Expand Up @@ -897,7 +895,7 @@ class SbbSelectElement extends SbbUpdateSchedulerMixin(
?aria-multiselectable=${this.multiple}
${ref((containerRef) => (this._optionContainer = containerRef as HTMLElement))}
>
<slot @slotchange=${this._setValueFromSelectedOption}></slot>
<slot @slotchange=${this._setValueFromSelected}></slot>
</div>
</div>
</div>
Expand Down

0 comments on commit 396272b

Please sign in to comment.