From 09ee55cbee8ac6058b77ccf4474380a648100d5e Mon Sep 17 00:00:00 2001 From: qa46hx Date: Wed, 21 Jul 2021 09:18:43 +0200 Subject: [PATCH] fix(select): set formattedValue to text of selected option --- .changeset/large-mails-roll.md | 6 +++ .../test/form-integrations.test.js | 2 +- packages/select/src/LionSelect.js | 51 +++++++++++-------- packages/select/test/lion-select.test.js | 31 ++++++++++- 4 files changed, 65 insertions(+), 25 deletions(-) create mode 100644 .changeset/large-mails-roll.md diff --git a/.changeset/large-mails-roll.md b/.changeset/large-mails-roll.md new file mode 100644 index 000000000..be5d03cd9 --- /dev/null +++ b/.changeset/large-mails-roll.md @@ -0,0 +1,6 @@ +--- +'@lion/form-integrations': patch +'@lion/select': patch +--- + +Set formattedValue to the text of the selected option instead of its value diff --git a/packages/form-integrations/test/form-integrations.test.js b/packages/form-integrations/test/form-integrations.test.js index 04abe1680..8737532a4 100644 --- a/packages/form-integrations/test/form-integrations.test.js +++ b/packages/form-integrations/test/form-integrations.test.js @@ -54,7 +54,7 @@ describe('Form Integrations', () => { favoriteFruit: 'Banana', favoriteMovie: 'Rocky', favoriteColor: 'hotpink', - lyrics: '1', + lyrics: 'Fire up that loud', range: 2.3, terms: [], notifications: '', diff --git a/packages/select/src/LionSelect.js b/packages/select/src/LionSelect.js index 8ab932756..2b8dd1b56 100644 --- a/packages/select/src/LionSelect.js +++ b/packages/select/src/LionSelect.js @@ -1,6 +1,9 @@ /* eslint-disable max-classes-per-file */ import { LionField } from '@lion/form-core'; +/** + * @typedef {import('@lion/localize/types/LocalizeMixinTypes').FormatNumberOptions} FormatOptions + */ class LionFieldWithSelect extends LionField { /** @type {any} */ static get properties() { @@ -28,19 +31,19 @@ class LionFieldWithSelect extends LionField { } /** - * LionSelectNative: wraps the native HTML element select + * LionSelect: wraps the native HTML element select * - * + * * * - * + * * * You can preselect an option by setting the property modelValue. * Example: - * + * * * It extends LionField so it inherits required and disabled. * @@ -59,24 +62,6 @@ export class LionSelect extends LionFieldWithSelect { this._inputNode.addEventListener('change', this._proxyChangeEvent); } - // FIXME: For some reason we have to override this FormatMixin getter/setter pair for the tests to pass - get value() { - return (this._inputNode && this._inputNode.value) || this.__value || ''; - } - - // We don't delegate, because we want to preserve caret position via _setValueAndPreserveCaret - /** @type {string} */ - set value(value) { - // if not yet connected to dom can't change the value - if (this._inputNode) { - this._inputNode.value = value; - /** @type {string | undefined} */ - this.__value = undefined; - } else { - this.__value = value; - } - } - /** @param {import('@lion/core').PropertyValues } changedProperties */ updated(changedProperties) { super.updated(changedProperties); @@ -99,6 +84,28 @@ export class LionSelect extends LionFieldWithSelect { this._inputNode.removeEventListener('change', this._proxyChangeEvent); } + /** + * @override FormatMixin - set formattedValue to selected option text + * @param {*} v - modelValue: can be an Object, Number, String depending on the + * input type(date, number, email etc) + * @returns {string} formattedValue + */ + formatter(v) { + // The selectedIndex is not yet updated + const found = Array.from(this._inputNode.options).find(option => option.value === v); + return found ? found.text : ''; + } + + /** + * @override FormatMixin - set value equal to modelValue instead of formattedValue + */ + _reflectBackFormattedValueToUser() { + if (this._reflectBackOn()) { + // Text 'undefined' should not end up in + this.value = typeof this.modelValue !== 'undefined' ? this.modelValue : ''; + } + } + /** @protected */ _proxyChangeEvent() { this.dispatchEvent( diff --git a/packages/select/test/lion-select.test.js b/packages/select/test/lion-select.test.js index 7a760eed1..e1e25c2ad 100644 --- a/packages/select/test/lion-select.test.js +++ b/packages/select/test/lion-select.test.js @@ -1,7 +1,10 @@ -import { expect, fixture } from '@open-wc/testing'; +import '@lion/select/define'; +import { aTimeout, expect, fixture } from '@open-wc/testing'; import { html } from 'lit/static-html.js'; -import '@lion/select/define'; +/** + * @typedef {import('../src/LionSelect').LionSelect} LionSelect + */ describe('lion-select', () => { it('can preselect an option', async () => { @@ -17,6 +20,30 @@ describe('lion-select', () => { expect(lionSelect.querySelector('select')?.value).to.equal('nr2'); }); + it('sets the values correctly based on the option value and view value', async () => { + const lionSelect = /** @type {LionSelect} */ ( + await fixture(html` + + + + `) + ); + expect(lionSelect.serializedValue).to.equal('nr2'); + expect(lionSelect.formattedValue).to.equal('Item 2'); + lionSelect.modelValue = 'nr1'; + await aTimeout; + expect(lionSelect.serializedValue).to.equal('nr1'); + expect(lionSelect.formattedValue).to.equal('Item 1'); + lionSelect.modelValue = 'nr3'; + await aTimeout; + expect(lionSelect.serializedValue).to.equal('nr3'); + expect(lionSelect.formattedValue).to.equal(''); + }); + it('is accessible', async () => { const lionSelect = await fixture(html`