diff --git a/packages/webawesome/docs/docs/components/select.md b/packages/webawesome/docs/docs/components/select.md index c98c59b44..6b90b7344 100644 --- a/packages/webawesome/docs/docs/components/select.md +++ b/packages/webawesome/docs/docs/components/select.md @@ -7,7 +7,7 @@ category: Form Controls ```html {.example} - Option 1 + Option 1 Option 2 Option 3 Option 4 @@ -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. -::: +::: \ No newline at end of file diff --git a/packages/webawesome/docs/docs/resources/changelog.md b/packages/webawesome/docs/docs/resources/changelog.md index 5c013afe8..b6b5bb5d8 100644 --- a/packages/webawesome/docs/docs/resources/changelog.md +++ b/packages/webawesome/docs/docs/resources/changelog.md @@ -10,6 +10,13 @@ Components with the Experimental badge sh ## 3.0.0-beta.2 +### New Features {data-no-outline} + +- Added `.wa-hover-rows` to native styles to opt-in to highlighting table rows on hover. + +### Bug Fixes and Improvements {data-no-outline} + +- Fixed a bug in `` with options that had blank string values. [pr:1136] - Added `.wa-hover-rows` to native styles to opt-in to highlighting table rows on hover [pr:1111] - Added missing changelog entries for beta.1 [pr:1117] - Fixed a bug in `` that prevented the menu from flipping/shifting to keep the menu in the viewport [pr:1122] @@ -367,4 +374,4 @@ Many of these changes and improvements were the direct result of feedback from u -Did we miss something? [Let us know!](https://github.com/shoelace-style/webawesome-alpha/discussions) +Did we miss something? [Let us know!](https://github.com/shoelace-style/webawesome-alpha/discussions) \ No newline at end of file diff --git a/packages/webawesome/src/components/details/details.test.ts b/packages/webawesome/src/components/details/details.test.ts index 949de2492..75bba1a17 100644 --- a/packages/webawesome/src/components/details/details.test.ts +++ b/packages/webawesome/src/components/details/details.test.ts @@ -196,8 +196,9 @@ describe('', () => { 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); }); }); } diff --git a/packages/webawesome/src/components/select/select.test.ts b/packages/webawesome/src/components/select/select.test.ts index 375c42b45..45e5c03cf 100644 --- a/packages/webawesome/src/components/select/select.test.ts +++ b/packages/webawesome/src/components/select/select.test.ts @@ -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,21 +200,22 @@ describe('', () => { `); 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); el.addEventListener('input', handler); 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); }); }); @@ -648,8 +649,8 @@ describe('', () => { const el = form.querySelector('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 +698,8 @@ describe('', () => { ); const el = form.querySelector('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 +772,12 @@ describe('', () => { ); const el = form.querySelector('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 +889,43 @@ describe('', () => { // Get the popup element and check its active state expect(popup?.active).to.be.true; }); + + // https://github.com/shoelace-style/webawesome/issues/1131 + // new test, failing only in CI + it.skip('Should work properly with empty values on select', async () => { + const el = await fixture(html` + + Blank Option + Option 2 + Option 3 + + `); + + await resetMouse(); + + await el.show(); + const options = el.querySelectorAll('wa-option'); + await aTimeout(100); + // firefox doesnt like clicks -.- + await clickOnElement(options[0]); + await resetMouse(); + await el.updateComplete; + expect(el.value).to.equal(''); + + await aTimeout(100); + await clickOnElement(options[1]); + await resetMouse(); + await el.updateComplete; + await aTimeout(100); + expect(el.value).to.equal('option-2'); + + await clickOnElement(options[0]); + await resetMouse(); + await el.updateComplete; + await aTimeout(100); + expect(el.value).to.equal(''); + await resetMouse(); + }); }); } }); diff --git a/packages/webawesome/src/components/select/select.ts b/packages/webawesome/src/components/select/select.ts index fe067a686..fdb62e214 100644 --- a/packages/webawesome/src/components/select/select.ts +++ b/packages/webawesome/src/components/select/select.ts @@ -114,22 +114,22 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement { @state() displayLabel = ''; @state() currentOption: WaOption; @state() selectedOptions: WaOption[] = []; - @state() optionValues: Set | undefined; + @state() optionValues: Set | 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,11 +189,11 @@ 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; @@ -291,16 +296,17 @@ 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 }); @@ -528,10 +535,11 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement { private handleOptionClick(event: MouseEvent) { const target = event.target as HTMLElement; const option = target.closest('wa-option'); - const oldValue = this.value; if (option && !option.disabled) { + this.hasInteracted = true; this.valueHasChanged = true; + if (this.multiple) { this.toggleOptionSelection(option); } else { @@ -541,13 +549,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 +574,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 +702,36 @@ export default class WaSelect extends WebAwesomeFormAssociatedElement { // Update selected options cache this.selectedOptions = options.filter(el => { + 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 +795,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(); } diff --git a/packages/webawesome/web-test-runner.config.js b/packages/webawesome/web-test-runner.config.js index 38956df73..d2f444d9c 100644 --- a/packages/webawesome/web-test-runner.config.js +++ b/packages/webawesome/web-test-runner.config.js @@ -40,9 +40,26 @@ 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(); },