diff --git a/.changeset/nervous-lions-trade.md b/.changeset/nervous-lions-trade.md new file mode 100644 index 000000000..22cfd6a0a --- /dev/null +++ b/.changeset/nervous-lions-trade.md @@ -0,0 +1,5 @@ +--- +'@lion/input-amount': patch +--- + +Empty or invalid currency now removes the currency label node, this is restored when the currency is valid again. diff --git a/packages/input-amount/src/LionInputAmount.js b/packages/input-amount/src/LionInputAmount.js index d09fd3da7..93ebcff85 100644 --- a/packages/input-amount/src/LionInputAmount.js +++ b/packages/input-amount/src/LionInputAmount.js @@ -34,24 +34,16 @@ export class LionInputAmount extends LocalizeMixin(LionInput) { return { ...super.slots, after: () => { - if (this.currency) { - const el = document.createElement('span'); - // The data-label attribute will make sure that FormControl adds this to - // input[aria-labelledby] - el.setAttribute('data-label', ''); - - el.textContent = this.__currencyLabel; - return el; - } - return undefined; + const el = document.createElement('span'); + // The data-label attribute will make sure that FormControl adds this to + // input[aria-labelledby] + el.setAttribute('data-label', ''); + el.textContent = this.__currencyLabel; + return el; }, }; } - get _currencyDisplayNode() { - return Array.from(this.children).find(child => child.slot === 'after'); - } - static get styles() { return [ ...super.styles, @@ -71,6 +63,7 @@ export class LionInputAmount extends LocalizeMixin(LionInput) { this.currency = undefined; /** @type {string | undefined} */ this.locale = undefined; + this.__currencyDisplayNodeIsConnected = true; this.defaultValidators.push(new IsNumber()); } @@ -88,8 +81,8 @@ export class LionInputAmount extends LocalizeMixin(LionInput) { /** @param {import('@lion/core').PropertyValues } changedProperties */ updated(changedProperties) { super.updated(changedProperties); - if (changedProperties.has('currency') && this.currency) { - this._onCurrencyChanged({ currency: this.currency }); + if (changedProperties.has('currency')) { + this._onCurrencyChanged({ currency: this.currency || null }); } if (changedProperties.has('locale') && this.locale !== changedProperties.get('locale')) { @@ -102,6 +95,20 @@ export class LionInputAmount extends LocalizeMixin(LionInput) { } } + /** + * Upon connecting slot mixin, we should check if + * the after slot was created by the slot mixin, + * and if so, we should execute the currency changed flow + * which evaluates whether the slot node should be + * removed for invalid currencies + */ + _connectSlotMixin() { + super._connectSlotMixin(); + if (this._isPrivateSlot('after')) { + this._onCurrencyChanged({ currency: this.currency || null }); + } + } + /** * @param {string} newLocale * @param {string} oldLocale @@ -126,16 +133,51 @@ export class LionInputAmount extends LocalizeMixin(LionInput) { /** * @param {Object} opts - * @param {string} opts.currency + * @param {string?} opts.currency * @protected */ _onCurrencyChanged({ currency }) { - if (this._isPrivateSlot('after') && this._currencyDisplayNode) { - this._currencyDisplayNode.textContent = this.__currencyLabel; + if (!this.__currencyDisplayNode) { + return; } - this.formatOptions.currency = currency; - this._calculateValues({ source: null }); - this.__setCurrencyDisplayLabel(); + + this.formatOptions.currency = currency || undefined; + if (currency) { + if (!this.__currencyDisplayNodeIsConnected) { + this.appendChild(this.__currencyDisplayNode); + this.__currencyDisplayNodeIsConnected = true; + } + this.__currencyDisplayNode.textContent = this.__currencyLabel; + + try { + this._calculateValues({ source: null }); + } catch (e) { + // In case Intl.NumberFormat gives error for invalid currency + // we should catch, remove the node, and rethrow (since it's still a user error) + if (e instanceof RangeError) { + this.__currencyDisplayNode?.remove(); + this.__currencyDisplayNodeIsConnected = false; + } + throw e; + } + this.__setCurrencyDisplayLabel(); + } else { + this.__currencyDisplayNode?.remove(); + this.__currencyDisplayNodeIsConnected = false; + } + } + + /** + * @returns the current currency display node + * @private + */ + get __currencyDisplayNode() { + const node = Array.from(this.children).find(child => child.slot === 'after'); + if (node) { + this.__storedCurrencyDisplayNode = node; + } + + return node || this.__storedCurrencyDisplayNode; } /** @private */ @@ -143,8 +185,11 @@ export class LionInputAmount extends LocalizeMixin(LionInput) { // TODO: (@erikkroes) for optimal a11y, abbreviations should be part of aria-label // example, for a language switch with text 'en', an aria-label of 'english' is not // sufficient, it should also contain the abbreviation. - if (this.currency && this._currencyDisplayNode) { - this._currencyDisplayNode.setAttribute('aria-label', getCurrencyName(this.currency, {})); + if (this.__currencyDisplayNode) { + this.__currencyDisplayNode.setAttribute( + 'aria-label', + this.currency ? getCurrencyName(this.currency, {}) : '', + ); } } diff --git a/packages/input-amount/test/lion-input-amount.test.js b/packages/input-amount/test/lion-input-amount.test.js index 14cb160b4..d3867f510 100644 --- a/packages/input-amount/test/lion-input-amount.test.js +++ b/packages/input-amount/test/lion-input-amount.test.js @@ -197,6 +197,60 @@ describe('', () => { expect(el.formattedValue).to.equal('123,45'); }); + it('removes the currency label when currency switches from EUR to undefined', async () => { + const el = /** @type {LionInputAmount} */ ( + await fixture(``) + ); + expect( + /** @type {HTMLElement[]} */ (Array.from(el.children)).find(child => child.slot === 'after') + ?.innerText, + ).to.equal('EUR'); + el.currency = undefined; + await el.updateComplete; + expect( + /** @type {HTMLElement[]} */ (Array.from(el.children)).find(child => child.slot === 'after'), + ).to.be.undefined; + }); + + it('adds the currency label when currency switches from undefined to EUR', async () => { + const el = /** @type {LionInputAmount} */ ( + await fixture(``) + ); + expect( + /** @type {HTMLElement[]} */ (Array.from(el.children)).find(child => child.slot === 'after'), + ).to.be.undefined; + + el.currency = 'EUR'; + await el.updateComplete; + const currLabel = /** @type {HTMLElement[]} */ (Array.from(el.children)).find( + child => child.slot === 'after', + ); + expect(currLabel?.innerText).to.equal('EUR'); + expect(currLabel?.getAttribute('aria-label')).to.equal('euros'); + }); + + it('sets currency label on the after element', async () => { + const el = /** @type {LionInputAmount} */ ( + await fixture(` + + Currency, please + `) + ); + const mySlotLabel = /** @type {HTMLElement[]} */ (Array.from(el.children)).find( + child => child.slot === 'after', + ); + expect(mySlotLabel?.id).to.equal('123'); + + el.currency = 'EUR'; + await el.updateComplete; + const currLabel = /** @type {HTMLElement[]} */ (Array.from(el.children)).find( + child => child.slot === 'after', + ); + expect(currLabel).to.equal(mySlotLabel); + expect(currLabel?.id).to.equal('123'); + expect(currLabel?.innerText).to.equal('EUR'); + }); + describe('Accessibility', () => { it('is accessible', async () => { const el = await fixture( @@ -223,19 +277,25 @@ describe('', () => { const el = /** @type {LionInputAmount} */ ( await fixture(``) ); - expect(el._currencyDisplayNode?.getAttribute('data-label')).to.be.not.null; + const label = /** @type {HTMLElement[]} */ (Array.from(el.children)).find( + child => child.slot === 'after', + ); + expect(label?.getAttribute('data-label')).to.be.not.null; const { _inputNode } = getInputMembers(/** @type {* & LionInput} */ (el)); - expect(_inputNode.getAttribute('aria-labelledby')).to.contain(el._currencyDisplayNode?.id); + expect(_inputNode.getAttribute('aria-labelledby')).to.contain(label?.id); }); it('adds an aria-label to currency slot', async () => { const el = /** @type {LionInputAmount} */ ( await fixture(``) ); - expect(el._currencyDisplayNode?.getAttribute('aria-label')).to.equal('euros'); + const label = /** @type {HTMLElement[]} */ (Array.from(el.children)).find( + child => child.slot === 'after', + ); + expect(label?.getAttribute('aria-label')).to.equal('euros'); el.currency = 'USD'; await el.updateComplete; - expect(el._currencyDisplayNode?.getAttribute('aria-label')).to.equal('US dollars'); + expect(label?.getAttribute('aria-label')).to.equal('US dollars'); el.currency = 'PHP'; await el.updateComplete; // TODO: Chrome Intl now thinks this should be pesos instead of pisos. They're probably right.