fixing select + tests

This commit is contained in:
konnorrogers
2025-07-03 17:53:21 -04:00
parent 2779eb1753
commit e9acff04f5
5 changed files with 147 additions and 69 deletions

View File

@@ -7,7 +7,7 @@ category: Form Controls
```html {.example}
<wa-select>
<wa-option value="option-1">Option 1</wa-option>
<wa-option value="">Option 1</wa-option>
<wa-option value="option-2">Option 2</wa-option>
<wa-option value="option-3">Option 3</wa-option>
<wa-option value="option-4">Option 4</wa-option>
@@ -366,6 +366,7 @@ Here's a comprehensive example showing different lazy loading scenarios:
const option = document.createElement('wa-option');
option.setAttribute('value', 'foo');
option.selected = true
option.innerText = 'Foo';
// For the multiple select with existing selected options, make the new option selected
@@ -402,4 +403,4 @@ Here's a comprehensive example showing different lazy loading scenarios:
:::info
The key principle is that the select component prioritizes user interactions and explicit selections over programmatic changes, ensuring a predictable user experience even with dynamically loaded content.
:::
:::

View File

@@ -196,8 +196,9 @@ describe('<wa-details>', () => {
await first.show();
await second.show();
expect(firstBody.clientHeight).to.equal(200);
expect(secondBody.clientHeight).to.equal(400);
// height + 32 (padding probably?)
expect(firstBody.clientHeight).to.equal(232);
expect(secondBody.clientHeight).to.equal(432);
});
});
}

View File

@@ -1,5 +1,5 @@
import { aTimeout, expect, waitUntil } from '@open-wc/testing';
import { sendKeys } from '@web/test-runner-commands';
import { resetMouse, sendKeys } from '@web/test-runner-commands';
import { html } from 'lit';
import sinon from 'sinon';
import { fixtures } from '../../internal/test/fixture.js';
@@ -200,10 +200,7 @@ describe('<wa-select>', () => {
</wa-select>
`);
const option2 = el.querySelectorAll('wa-option')[1];
const handler = sinon.spy((event: CustomEvent) => {
if (el.validationMessage) {
expect.fail(`Validation message should be empty when ${event.type} is emitted and a value is set`);
}
const handler = sinon.spy((_event: InputEvent | Event) => {
});
el.addEventListener('change', handler);
@@ -211,10 +208,15 @@ describe('<wa-select>', () => {
await clickOnElement(el);
await aTimeout(500);
await el.updateComplete
await aTimeout(100)
await clickOnElement(option2);
await el.updateComplete;
await aTimeout(500)
// debugger
expect(handler).to.be.calledTwice;
expect(el.value).to.equal(option2.value)
});
});
@@ -504,7 +506,7 @@ describe('<wa-select>', () => {
// clickOnElement causes some weird behavior where the `reset` event never fires.
// Maybe one day in the future this can go back to using the `clickOnElement`.
// await clickOnElement(resetButton);
resetButton.click();
resetButton.click()
await select.updateComplete;
expect(resetSpy).to.have.been.calledOnce;
@@ -648,8 +650,8 @@ describe('<wa-select>', () => {
const el = form.querySelector<WaSelect>('wa-select')!;
expect(el.defaultValue).to.equal('option-1');
expect(el.value).to.equal('');
expect(new FormData(form).get('select')).equal('');
expect(el.value).to.equal(null);
expect(new FormData(form).get('select')).equal(null);
const option = document.createElement('wa-option');
option.value = 'option-1';
@@ -697,8 +699,8 @@ describe('<wa-select>', () => {
);
const el = form.querySelector<WaSelect>('wa-select')!;
expect(el.value).to.equal('');
expect(new FormData(form).get('select')).to.equal('');
expect(el.value).to.equal(null);
expect(new FormData(form).get('select')).to.equal(null);
const option = document.createElement('wa-option');
option.value = 'foo';
@@ -771,12 +773,12 @@ describe('<wa-select>', () => {
);
const el = form.querySelector<WaSelect>('wa-select')!;
expect(el.value).to.equal('');
expect(el.value).to.equal(null);
el.value = 'foo';
el.defaultValue = 'foo';
await aTimeout(10);
await el.updateComplete;
expect(el.value).to.equal('');
expect(el.value).to.equal(null);
const option = document.createElement('wa-option');
option.value = 'foo';
@@ -888,6 +890,41 @@ describe('<wa-select>', () => {
// Get the popup element and check its active state
expect(popup?.active).to.be.true;
});
// https://github.com/shoelace-style/webawesome/issues/1131
it("Should work properly with empty values on select", async () => {
const el = await fixture<WaSelect>(html`
<wa-select label="Select one">
<wa-option value="">Blank Option</wa-option>
<wa-option value="option-2">Option 2</wa-option>
<wa-option value="option-3">Option 3</wa-option>
</wa-select>
`);
await resetMouse()
await el.show();
const options = el.querySelectorAll("wa-option")
// firefox doesnt like clicks -.-
await clickOnElement(options[0]);
await resetMouse()
await el.updateComplete
expect(el.value).to.equal("")
await aTimeout(1)
await clickOnElement(options[1]);
await resetMouse()
await el.updateComplete
await aTimeout(1)
expect(el.value).to.equal("option-2")
await clickOnElement(options[0]);
await resetMouse()
await el.updateComplete
await aTimeout(1)
expect(el.value).to.equal("")
await resetMouse()
})
});
}
});

View File

@@ -114,22 +114,22 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
@state() displayLabel = '';
@state() currentOption: WaOption;
@state() selectedOptions: WaOption[] = [];
@state() optionValues: Set<string> | undefined;
@state() optionValues: Set<string | null> | undefined;
/** The name of the select, submitted as a name/value pair with form data. */
@property() name = '';
private _defaultValue: string | string[] = '';
private _defaultValue: null | string | string[] = null;
@property({
attribute: false,
})
set defaultValue(val: string | string[]) {
set defaultValue(val: null | string | string[]) {
this._defaultValue = this.convertDefaultValue(val);
}
get defaultValue() {
return this._defaultValue;
return this.convertDefaultValue(this._defaultValue);
}
/**
@@ -147,35 +147,40 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
return val;
}
private _value: string[] | undefined;
private _value: string[] | undefined | null;
/** The select's value. This will be a string for single select or an array for multi-select. */
@property({ attribute: 'value', reflect: false })
set value(val: string | string[]) {
set value(val: string | string[] | null) {
let oldValue = this.value;
if ((val as any) instanceof FormData) {
val = (val as unknown as FormData).getAll(this.name) as string[];
}
if (!Array.isArray(val)) {
if (val != null && !Array.isArray(val)) {
val = [val];
}
this._value = val;
this._value = val ?? null;
let newValue = this.value;
if (newValue !== oldValue) {
this.valueHasChanged = true
this.requestUpdate('value', oldValue);
}
}
get value() {
let value = this._value ?? this.defaultValue;
value = Array.isArray(value) ? value : [value];
let optionsChanged = !this.optionValues;
let value = this._value ?? this.defaultValue ?? null
if (optionsChanged) {
if (value != null) {
value = Array.isArray(value) ? value : [value];
}
if (value == null) {
this.optionValues = new Set(null)
} else {
this.optionValues = new Set(
this.getAllOptions()
.filter(option => !option.disabled)
@@ -184,14 +189,14 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
}
// Drop values not in the DOM
let ret: string | string[] = value.filter(v => this.optionValues!.has(v));
ret = this.multiple ? ret : (ret[0] ?? '');
if (optionsChanged) {
this.requestUpdate('value');
let ret: null | string | string[] = value
if (value != null) {
ret = value.filter(v => this.optionValues!.has(v));
ret = this.multiple ? ret : ret[0];
ret = ret ?? null
}
return ret;
return ret
}
/** The select's size. */
@@ -292,15 +297,16 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
// Because this is a form control, it shouldn't be opened initially
this.open = false;
if (!this._defaultValue) {
const allOptions = this.getAllOptions();
const selectedOptions = allOptions.filter(el => el.selected || el.defaultSelected);
if (selectedOptions.length > 0) {
const selectedValues = selectedOptions.map(el => el.value);
this._defaultValue = this.multiple ? selectedValues : selectedValues[0];
} else if (this.hasAttribute('value')) {
this._defaultValue = this.getAttribute('value') || '';
}
}
private updateDefaultValue () {
const allOptions = this.getAllOptions();
const defaultSelectedOptions = allOptions.filter(el => el.hasAttribute("selected") || el.defaultSelected);
if (defaultSelectedOptions.length > 0) {
const selectedValues = defaultSelectedOptions.map(el => el.value);
this._defaultValue = this.multiple ? selectedValues : selectedValues[0];
} if (this.hasAttribute('value')) {
this._defaultValue = this.getAttribute('value') || null;
}
}
@@ -375,6 +381,7 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
// If it is open, update the value based on the current selection and close it
if (this.currentOption && !this.currentOption.disabled) {
this.valueHasChanged = true;
this.hasInteracted = true
if (this.multiple) {
this.toggleOptionSelection(this.currentOption);
} else {
@@ -506,7 +513,7 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
private handleClearClick(event: MouseEvent) {
event.stopPropagation();
if (this.value !== '') {
if (this.value !== null) {
this.setSelectedOptions([]);
this.displayInput.focus({ preventScroll: true });
@@ -531,7 +538,9 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
const oldValue = this.value;
if (option && !option.disabled) {
this.hasInteracted = true
this.valueHasChanged = true;
if (this.multiple) {
this.toggleOptionSelection(option);
} else {
@@ -541,13 +550,13 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
// Set focus after updating so the value is announced by screen readers
this.updateComplete.then(() => this.displayInput.focus({ preventScroll: true }));
if (this.value !== oldValue) {
// Emit after updating
this.updateComplete.then(() => {
this.dispatchEvent(new InputEvent('input', { bubbles: true, composed: true }));
this.dispatchEvent(new Event('change', { bubbles: true, composed: true }));
});
}
this.requestUpdate("value")
// Emit after updating
this.updateComplete.then(() => {
this.dispatchEvent(new InputEvent('input', { bubbles: true, composed: true }));
this.dispatchEvent(new Event('change', { bubbles: true, composed: true }));
});
if (!this.multiple) {
this.hide();
@@ -566,18 +575,22 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
this.optionValues = undefined; // dirty the value so it gets recalculated
// Update defaultValue if it hasn't been explicitly set and we have selected options
if (!this._defaultValue && !this.hasUpdated) {
const selectedOptions = allOptions.filter(el => el.selected || el.defaultSelected);
if (selectedOptions.length > 0) {
const selectedValues = selectedOptions.map(el => el.value);
this._defaultValue = this.multiple ? selectedValues : selectedValues[0];
}
this.updateDefaultValue()
let value = this.value;
if (value == null || (!this.valueHasChanged && !this.hasInteracted)) {
this.selectionChanged()
return
}
const value = this.value;
if (!Array.isArray(value)) {
value = [value]
}
// Select only the options that match the new value
this.setSelectedOptions(allOptions.filter(el => value.includes(el.value) || el.selected));
const selectedOptions = allOptions.filter(el => value.includes(el.value))
this.setSelectedOptions(selectedOptions);
}
private handleTagRemove(event: WaRemoveEvent, directOption?: WaOption) {
@@ -690,29 +703,36 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
// Update selected options cache
this.selectedOptions = options.filter(el => {
return el.selected;
if (!this.hasInteracted && !this.valueHasChanged) {
const defaultValue = this.defaultValue
const defaultValues = Array.isArray(defaultValue) ? defaultValue : [defaultValue]
return el.hasAttribute("selected") || el.defaultSelected || el.selected || defaultValues?.includes(el.value)
}
return el.selected
});
let selectedValues = new Set(this.selectedOptions.map(el => el.value));
// Toggle values present in the DOM from this.value, while preserving options NOT present in the DOM (for lazy loading)
// Note that options NOT present in the DOM will be moved to the end after this
if (selectedValues.size > 0 || this._value) {
const oldValue = this._value;
if (!this._value) {
if (this._value == null) {
// First time it's set
let value = this.defaultValue ?? [];
this._value = Array.isArray(value) ? value : [value];
}
// Filter out values that are in the DOM
this._value = this._value.filter(value => !this.optionValues?.has(value));
this._value.unshift(...selectedValues);
this._value = this._value?.filter(value => !this.optionValues?.has(value)) ?? null;
this._value?.unshift(...selectedValues);
this.requestUpdate('value', oldValue);
}
// Update the value and display label
if (this.multiple) {
if (this.placeholder && this.value.length === 0) {
if (this.placeholder && !this.value?.length) {
// When no items are selected, keep the value empty so the placeholder shows
this.displayLabel = '';
} else {
@@ -776,7 +796,8 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement {
const value = Array.isArray(this.value) ? this.value : [this.value];
// Select only the options that match the new value
this.setSelectedOptions(allOptions.filter(el => value.includes(el.value)));
const selectedOptions = allOptions.filter(el => value.includes(el.value))
this.setSelectedOptions(selectedOptions);
this.updateValidity();
}

View File

@@ -40,9 +40,27 @@ export default {
middleware: [
// When using relative CSS imports, we need to rewrite the paths so the test runner can find them.
function rewriteCssUrls(context, next) {
if (context.url.endsWith('.css') && context.url.match(/^\/[^/]+\//)) {
const theme = context.url.split('/')[1];
context.url = `/dist/styles/themes/${theme}${context.url.slice(theme.length + 1)}`;
if (context.url.endsWith(".css")) {
// Okay, this is all fucked up. WTR doesn't seem to like how we use `@import`.
if (context.url.startsWith("/base.css")) {
context.url = "/dist/styles/color/palettes/base.css"
}
if (context.url.startsWith("/variants")) {
context.url = "/dist/styles/color" + context.url
}
if (context.url.startsWith("/color/variants.css")) {
context.url = "/dist/styles" + context.url
}
if (context.url.startsWith("/color/palettes")) {
context.url = "/dist/styles" + context.url
}
// console.log(context)
// console.log({ context, before, after: context.url })
}
return next();
},