Skip to content

Commit

Permalink
fix select regressions
Browse files Browse the repository at this point in the history
  • Loading branch information
KonnorRogers committed Nov 4, 2024
1 parent 9919e43 commit 2363f1c
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 8 deletions.
45 changes: 37 additions & 8 deletions src/components/select/select.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,21 +102,42 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
/** The name of the select, submitted as a name/value pair with form data. */
@property() name = '';

private _value: string | string[] = ""

get value () {
return this._value
}

/**
* The current value of the select, submitted as a name/value pair with form data. When `multiple` is enabled, the
* value attribute will be a space-delimited list of values based on the options selected, and the value property will
* be an array. **For this reason, values must not contain spaces.**
*/
@property({
converter: {
fromAttribute: (value: string) => value.split(' '),
toAttribute: (value: string[]) => value.join(' ')
}
@state({
// attribute: false,
// converter: {
// fromAttribute: (value: string) => value.split(' '),
// toAttribute: (value: string[]) => value.join(' ')
// }
})
value: string | string[] = '';
set value (val: string | string[]) {
if (this.multiple) {
val = Array.isArray(val) ? val : val.split(" ")
} else {
val = Array.isArray(val) ? val.join(" ") : val
}

if (this._value === val) {
return
}

this.valueHasChanged = true

this._value = val
}

/** The default value of the form control. Primarily used for resetting the form control. */
@defaultValue() defaultValue: string | string[] = '';
@property({ attribute: "value" }) defaultValue: string | string[] = '';

/** The select's size. */
@property({ reflect: true }) size: 'small' | 'medium' | 'large' = 'medium';
Expand Down Expand Up @@ -598,6 +619,7 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
// Update selected options cache
this.selectedOptions = options.filter(el => el.selected);

const cachedValueHasChanged = this.valueHasChanged
// Update the value and display label
if (this.multiple) {
this.value = this.selectedOptions.map(el => el.value);
Expand All @@ -613,12 +635,14 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
this.value = selectedOption?.value ?? '';
this.displayLabel = selectedOption?.getTextLabel?.() ?? '';
}
this.valueHasChanged = cachedValueHasChanged

// Update validity
this.updateComplete.then(() => {
this.formControlController.updateValidity();
});
}

protected get tags() {
return this.selectedOptions.map((option, index) => {
if (index < this.maxOptionsVisible || this.maxOptionsVisible <= 0) {
Expand Down Expand Up @@ -649,8 +673,13 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
}
}

@watch('value', { waitUntilFirstUpdate: true })
@watch(['defaultValue', 'value'], { waitUntilFirstUpdate: true })
handleValueChange() {
if (!this.valueHasChanged) {
this.value = this.defaultValue
this.valueHasChanged = false
}

const allOptions = this.getAllOptions();
const value = Array.isArray(this.value) ? this.value : [this.value];

Expand Down
19 changes: 19 additions & 0 deletions src/components/select/select.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,25 @@ describe('<sl-select>', () => {
expect(new FormData(form).getAll('select')).to.have.members(['foo', 'bar', 'baz']);
});
});

/**
* @see {https://github.com/shoelace-style/shoelace/issues/2254}
*/
it('Should account for if `value` changed before connecting', async () => {
const select = await fixture<SlSelect>(html`
<sl-select label="Search By" multiple clearable .value=${["foo", "bar"]}>
<sl-option value="foo">Foo</sl-option>
<sl-option value="bar">Bar</sl-option>
</sl-select>
`)

// just for safe measure.
await aTimeout(10)

expect(select.value).to.deep.equal(["foo", "bar"])

})

});

runFormControlBaseTests('sl-select');
Expand Down

0 comments on commit 2363f1c

Please sign in to comment.