From 717de40ba1ed0ada767d16c4b380ac2ce6c6909c Mon Sep 17 00:00:00 2001 From: bendera Date: Sun, 19 Jan 2025 03:42:10 +0100 Subject: [PATCH 1/8] Fix the disabled hover style --- src/includes/vscode-select/styles.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/includes/vscode-select/styles.ts b/src/includes/vscode-select/styles.ts index 842867a54..ab219738d 100644 --- a/src/includes/vscode-select/styles.ts +++ b/src/includes/vscode-select/styles.ts @@ -207,7 +207,7 @@ export default [ color: var(--vscode-list-focusHighlightForeground); } - .option:hover { + .option:not(.disabled):hover { background-color: var(--vscode-list-hoverBackground); color: var(--vscode-list-hoverForeground); } From d0d48edce8a3625a461fd62c97c357a017cc8460 Mon Sep 17 00:00:00 2001 From: bendera Date: Sun, 19 Jan 2025 03:45:46 +0100 Subject: [PATCH 2/8] Synchronize option changes with the parent select --- dev/vscode-single-select/sync-options.html | 55 +++++++++++++++++++ .../vscode-select/vscode-select-base.ts | 31 ++++++++++- src/vscode-option/vscode-option.ts | 43 +++++++++++++-- .../vscode-single-select.test.ts | 26 +++++++++ 4 files changed, 149 insertions(+), 6 deletions(-) create mode 100644 dev/vscode-single-select/sync-options.html diff --git a/dev/vscode-single-select/sync-options.html b/dev/vscode-single-select/sync-options.html new file mode 100644 index 000000000..3cc439ef1 --- /dev/null +++ b/dev/vscode-single-select/sync-options.html @@ -0,0 +1,55 @@ + + + + + + VSCode Elements + + + + + + + +

Sync options

+
+ + + + Lorem + Ipsum + Dolor + + + + +
+ + diff --git a/src/includes/vscode-select/vscode-select-base.ts b/src/includes/vscode-select/vscode-select-base.ts index fd49d2bb8..736812644 100644 --- a/src/includes/vscode-select/vscode-select-base.ts +++ b/src/includes/vscode-select/vscode-select-base.ts @@ -141,6 +141,31 @@ export class VscodeSelectBase extends VscElement { }) private _assignedOptions!: VscodeOption[]; + constructor() { + super(); + this.addEventListener('vsc-option-state-change', (ev) => { + ev.stopPropagation(); + const stat = this._addOptionsFromSlottedElements(); + + if (stat.selectedIndexes.length > 0) { + this._selectedIndex = stat.selectedIndexes[0]; + this._selectedIndexes = stat.selectedIndexes; + this._value = stat.values[0]; + this._values = stat.values; + } + + if ( + !this._multiple && + !this.combobox && + stat.selectedIndexes.length === 0 + ) { + this._selectedIndex = this._options.length > 0 ? 0 : -1; + } + + this.requestUpdate(); + }); + } + connectedCallback(): void { super.connectedCallback(); this.addEventListener('keydown', this._onComponentKeyDown); @@ -575,6 +600,10 @@ export class VscodeSelectBase extends VscElement { return html`${nothing}`; } + private _onPropChange = () => { + console.log('ONPROPCHANGE'); + }; + private _renderDropdown() { const classes = classMap({ dropdown: true, @@ -582,7 +611,7 @@ export class VscodeSelectBase extends VscElement { }); return html` -
+
${this.position === 'above' ? this._renderDescription() : nothing} ${this._renderOptions()} ${this._renderDropdownControls()} ${this.position === 'below' ? this._renderDescription() : nothing} diff --git a/src/vscode-option/vscode-option.ts b/src/vscode-option/vscode-option.ts index a3d15253c..249e70880 100644 --- a/src/vscode-option/vscode-option.ts +++ b/src/vscode-option/vscode-option.ts @@ -1,4 +1,4 @@ -import {html, TemplateResult} from 'lit'; +import {html, PropertyValues, TemplateResult} from 'lit'; import {customElement, property} from 'lit/decorators.js'; import {VscElement} from '../includes/VscElement.js'; import styles from './vscode-option.styles.js'; @@ -13,12 +13,45 @@ export class VscodeOption extends VscElement { @property({type: String}) value?: string | undefined; - @property({type: String}) description = ''; - @property({type: Boolean, reflect: true}) selected = false; - @property({type: Boolean, reflect: true}) disabled = false; + @property({type: String}) + description = ''; + + @property({type: Boolean, reflect: true}) + selected = false; + + @property({type: Boolean, reflect: true}) + disabled = false; + + private _initialized = false; + + connectedCallback(): void { + super.connectedCallback(); + + this.updateComplete.then(() => { + this._initialized = true; + }); + } + + protected willUpdate(changedProperties: PropertyValues): void { + if ( + this._initialized && + (changedProperties.has('description') || + changedProperties.has('value') || + changedProperties.has('selected') || + changedProperties.has('disabled')) + ) { + /** @internal */ + this.dispatchEvent(new Event('vsc-option-state-change', {bubbles: true})); + } + } + + private _handleSlotChange = () => { + /** @internal */ + this.dispatchEvent(new Event('vsc-option-state-change', {bubbles: true})); + }; render(): TemplateResult { - return html``; + return html``; } } diff --git a/src/vscode-single-select/vscode-single-select.test.ts b/src/vscode-single-select/vscode-single-select.test.ts index af96c827e..0f8aec624 100644 --- a/src/vscode-single-select/vscode-single-select.test.ts +++ b/src/vscode-single-select/vscode-single-select.test.ts @@ -1,3 +1,5 @@ +import {clickOnElement, moveMouseOnElement} from '../includes/test-helpers.js'; +import type {VscodeOption} from '../vscode-option/vscode-option.js'; import {VscodeSingleSelect} from './index.js'; import {aTimeout, expect, fixture, html} from '@open-wc/testing'; import sinon from 'sinon'; @@ -925,4 +927,28 @@ describe('vscode-single-select', () => { expect(sl.shadowRoot?.querySelector('ul.options')).to.be.ok; }); + + it('changes the description of an option in an existing select', async () => { + const el = await fixture(html` + + Lorem + Ipsum + Dolor + + `); + const secondOption = el.querySelectorAll('vscode-option')[1]; + + secondOption.description = 'Test description'; + await el.updateComplete; + + await clickOnElement(el); + await el.updateComplete; + + await moveMouseOnElement(el.shadowRoot!.querySelectorAll('li')[1]); + await el.updateComplete; + + const desc = el.shadowRoot?.querySelector('.description'); + + expect(/Test description/.exec(desc!.innerHTML)).to.be.ok; + }); }); From 673c9f4d8eeee4e4286b60bb92386f9e26068a55 Mon Sep 17 00:00:00 2001 From: bendera Date: Sun, 19 Jan 2025 19:51:26 +0100 Subject: [PATCH 3/8] Fix missing cleanup after tests --- .../vscode-single-select.test.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/vscode-single-select/vscode-single-select.test.ts b/src/vscode-single-select/vscode-single-select.test.ts index 0f8aec624..362c1bc57 100644 --- a/src/vscode-single-select/vscode-single-select.test.ts +++ b/src/vscode-single-select/vscode-single-select.test.ts @@ -71,13 +71,9 @@ describe('vscode-single-select', () => { }); it('should return the validation message', async () => { - const el = document.createElement( - 'vscode-single-select' - ) as VscodeSingleSelect; - el.required = true; - document.body.appendChild(el); - - await aTimeout(0); + const el = await fixture( + html`` + ); expect(el.validationMessage).to.eql('Please select an item in the list.'); }); @@ -845,10 +841,11 @@ describe('vscode-single-select', () => { expect(el.tabIndex).to.eq(2); }); - it('should not throw error when selectedIndex points to a non-existent option', () => { - const el = document.createElement('vscode-single-select'); + it('should not throw error when selectedIndex points to a non-existent option', async () => { + const el = await fixture( + html`` + ); el.selectedIndex = 2; - document.body.appendChild(el); expect(() => { // trigger a slot change event From 7789356ad64c1a1cec3a4abe601601d3c74f55c1 Mon Sep 17 00:00:00 2001 From: bendera Date: Sun, 19 Jan 2025 20:00:04 +0100 Subject: [PATCH 4/8] Simplify calculating the label text --- .../vscode-single-select.ts | 26 +++---------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/src/vscode-single-select/vscode-single-select.ts b/src/vscode-single-select/vscode-single-select.ts index 40087cefd..35f8cb9d7 100644 --- a/src/vscode-single-select/vscode-single-select.ts +++ b/src/vscode-single-select/vscode-single-select.ts @@ -83,9 +83,6 @@ export class VscodeSingleSelect this._value = this._options[this._selectedIndex] ? this._options[this._selectedIndex].value : ''; - this._labelText = this._options[this._selectedIndex] - ? this._options[this._selectedIndex].label - : ''; } get selectedIndex(): number { return this._selectedIndex; @@ -101,11 +98,9 @@ export class VscodeSingleSelect if (this._selectedIndex > -1) { this._options[this._selectedIndex].selected = true; - this._labelText = this._options[this._selectedIndex].label; this._value = val; this._requestedValueToSetLater = ''; } else { - this._labelText = ''; this._value = ''; this._requestedValueToSetLater = val; } @@ -141,9 +136,6 @@ export class VscodeSingleSelect return this._internals.reportValidity(); } - @state() - private _labelText = ''; - @query('.face') private _face!: HTMLDivElement; @@ -221,10 +213,6 @@ export class VscodeSingleSelect } } - if (this._selectedIndex > -1) { - this._labelText = this._options[this._selectedIndex]?.label ?? ''; - } - if (this._selectedIndex > -1 && this._options.length > 0) { this._internals.setFormValue(this._options[this._selectedIndex].value); } else { @@ -242,7 +230,6 @@ export class VscodeSingleSelect this._filterPattern = ''; this._selectedIndex -= 1; this._activeIndex = this._selectedIndex; - this._labelText = this._options[this._selectedIndex].label; this._value = this._options[this._selectedIndex].value; this._internals.setFormValue(this._value); this._manageRequired(); @@ -259,7 +246,6 @@ export class VscodeSingleSelect this._filterPattern = ''; this._selectedIndex += 1; this._activeIndex = this._selectedIndex; - this._labelText = this._options[this._selectedIndex].label; this._value = this._options[this._selectedIndex].value; this._internals.setFormValue(this._value); this._manageRequired(); @@ -269,10 +255,6 @@ export class VscodeSingleSelect protected _onEnterKeyDown(): void { super._onEnterKeyDown(); - if (this._selectedIndex > -1) { - this._labelText = this._options[this._selectedIndex].label; - } - this.updateInputValue(); this._internals.setFormValue(this._value); this._manageRequired(); @@ -291,10 +273,6 @@ export class VscodeSingleSelect this._selectedIndex = Number((optEl as HTMLElement).dataset.index); this._value = this._options[this._selectedIndex].value; - if (this._selectedIndex > -1) { - this._labelText = this._options[this._selectedIndex].label; - } - this._toggleDropdown(false); this._internals.setFormValue(this._value); this._manageRequired(); @@ -317,13 +295,15 @@ export class VscodeSingleSelect } protected _renderSelectFace(): TemplateResult { + const label = this._options[this._selectedIndex]?.label ?? ''; + return html`
-1 ? 0 : -1} > - ${this._labelText} ${chevronDownIcon} + ${label} ${chevronDownIcon}
`; } From de9b67e2e7f1fe30e73d4a5ff7c451eec47a5205 Mon Sep 17 00:00:00 2001 From: bendera Date: Sun, 19 Jan 2025 20:02:40 +0100 Subject: [PATCH 5/8] Add tests --- .../vscode-single-select.test.ts | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/src/vscode-single-select/vscode-single-select.test.ts b/src/vscode-single-select/vscode-single-select.test.ts index 362c1bc57..460662a64 100644 --- a/src/vscode-single-select/vscode-single-select.test.ts +++ b/src/vscode-single-select/vscode-single-select.test.ts @@ -944,8 +944,50 @@ describe('vscode-single-select', () => { await moveMouseOnElement(el.shadowRoot!.querySelectorAll('li')[1]); await el.updateComplete; - const desc = el.shadowRoot?.querySelector('.description'); + const desc = el.shadowRoot!.querySelector('.description'); - expect(/Test description/.exec(desc!.innerHTML)).to.be.ok; + expect(desc).lightDom.to.eq('Test description'); + }); + + it('changes the label of an option in an existing select', async () => { + const el = await fixture(html` + + Lorem + Ipsum + Dolor + + `); + const secondOption = el.querySelectorAll('vscode-option')[1]; + + secondOption.innerHTML = 'Test label'; + await el.updateComplete; + + await clickOnElement(el); + await el.updateComplete; + + const li = el.shadowRoot!.querySelectorAll('li')[1]; + + expect(li).lightDom.to.eq('Test label'); + }); + + it('changes the disabled state of an option in an existing select', async () => { + const el = await fixture(html` + + Lorem + Ipsum + Dolor + + `); + const secondOption = el.querySelectorAll('vscode-option')[1]; + + secondOption.disabled = true; + await el.updateComplete; + + await clickOnElement(el); + await el.updateComplete; + + const li = el.shadowRoot!.querySelectorAll('li')[1]; + + expect(li.classList.contains('disabled')).to.be.true; }); }); From 896eb86fec020149cd3c3b1beb0a9cf1624acbbc Mon Sep 17 00:00:00 2001 From: bendera Date: Sun, 19 Jan 2025 20:12:54 +0100 Subject: [PATCH 6/8] Fix test Fix the test case where the selected value should be visible, when options are added later. --- src/vscode-option/vscode-option.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/vscode-option/vscode-option.ts b/src/vscode-option/vscode-option.ts index 249e70880..a6bc56a1e 100644 --- a/src/vscode-option/vscode-option.ts +++ b/src/vscode-option/vscode-option.ts @@ -46,8 +46,10 @@ export class VscodeOption extends VscElement { } private _handleSlotChange = () => { - /** @internal */ - this.dispatchEvent(new Event('vsc-option-state-change', {bubbles: true})); + if (this._initialized) { + /** @internal */ + this.dispatchEvent(new Event('vsc-option-state-change', {bubbles: true})); + } }; render(): TemplateResult { From b7c7c43c535606858a4091be156a90e6896f479b Mon Sep 17 00:00:00 2001 From: bendera Date: Sun, 19 Jan 2025 20:38:38 +0100 Subject: [PATCH 7/8] Refactor --- .../vscode-select/vscode-select-base.ts | 63 +++++++------------ 1 file changed, 21 insertions(+), 42 deletions(-) diff --git a/src/includes/vscode-select/vscode-select-base.ts b/src/includes/vscode-select/vscode-select-base.ts index 736812644..ffae67668 100644 --- a/src/includes/vscode-select/vscode-select-base.ts +++ b/src/includes/vscode-select/vscode-select-base.ts @@ -145,23 +145,7 @@ export class VscodeSelectBase extends VscElement { super(); this.addEventListener('vsc-option-state-change', (ev) => { ev.stopPropagation(); - const stat = this._addOptionsFromSlottedElements(); - - if (stat.selectedIndexes.length > 0) { - this._selectedIndex = stat.selectedIndexes[0]; - this._selectedIndexes = stat.selectedIndexes; - this._value = stat.values[0]; - this._values = stat.values; - } - - if ( - !this._multiple && - !this.combobox && - stat.selectedIndexes.length === 0 - ) { - this._selectedIndex = this._options.length > 0 ? 0 : -1; - } - + this._setStateFromSlottedElements(); this.requestUpdate(); }); } @@ -244,14 +228,12 @@ export class VscodeSelectBase extends VscElement { return this.combobox ? this._filteredOptions : this._options; } - protected _addOptionsFromSlottedElements(): OptionListStat { + protected _setStateFromSlottedElements() { const options: InternalOption[] = []; let nextIndex = 0; const optionElements = this._assignedOptions ?? []; - const optionsListStat: OptionListStat = { - selectedIndexes: [], - values: [], - }; + const selectedIndexes: number[] = []; + const values: string[] = []; this._valueOptionIndexMap = {}; optionElements.forEach((el) => { @@ -270,8 +252,8 @@ export class VscodeSelectBase extends VscElement { nextIndex = options.push(op); if (selected) { - optionsListStat.selectedIndexes.push(options.length - 1); - optionsListStat.values.push(value); + selectedIndexes.push(options.length - 1); + values.push(value); } this._valueOptionIndexMap[op.value] = op.index; @@ -279,7 +261,20 @@ export class VscodeSelectBase extends VscElement { this._options = options; - return optionsListStat; + if (selectedIndexes.length > 0) { + this._selectedIndex = selectedIndexes[0]; + this._selectedIndexes = selectedIndexes; + this._value = values[0]; + this._values = values; + } + + if ( + !this._multiple && + !this.combobox && + selectedIndexes.length === 0 + ) { + this._selectedIndex = this._options.length > 0 ? 0 : -1; + } } protected async _toggleDropdown(visible: boolean): Promise { @@ -538,23 +533,7 @@ export class VscodeSelectBase extends VscElement { } protected _onSlotChange(): void { - const stat = this._addOptionsFromSlottedElements(); - - if (stat.selectedIndexes.length > 0) { - this._selectedIndex = stat.selectedIndexes[0]; - this._selectedIndexes = stat.selectedIndexes; - this._value = stat.values[0]; - this._values = stat.values; - } - - if ( - !this._multiple && - !this.combobox && - stat.selectedIndexes.length === 0 - ) { - this._selectedIndex = this._options.length > 0 ? 0 : -1; - } - + this._setStateFromSlottedElements(); this.requestUpdate(); } From 4beaac55fcc94dae79f562179acccf71185b8b28 Mon Sep 17 00:00:00 2001 From: bendera Date: Sun, 19 Jan 2025 20:50:03 +0100 Subject: [PATCH 8/8] Update changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5db3ac9a..26e793c19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) +## [Unreleased] + +### Added + +- **SingleSelect**: Added the ability to update the state of an already initialized component + by modifying its child options. + ## [1.10.0] - 2025-01-11 ### Fixed