Merge pull request #1477 from ing-bank/amount_reset_improvements

Amount reset improvements

.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.

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');
-          const el = document.createElement('span');
+        // The data-label attribute will make sure that FormControl adds this to
-          // The data-label attribute will make sure that FormControl adds this to
+        // input[aria-labelledby]
-          // input[aria-labelledby]
+        el.setAttribute('data-label', '');
-          el.setAttribute('data-label', '');
+        el.textContent = this.__currencyLabel;
-
+        return el;
-          el.textContent = this.__currencyLabel;
-          return el;
-        }
-        return undefined;
       },
     };
   }
 
-  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) {
+    if (changedProperties.has('currency')) {
-      this._onCurrencyChanged({ currency: this.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) {
+    if (!this.__currencyDisplayNode) {
-      this._currencyDisplayNode.textContent = this.__currencyLabel;
+      return;
     }
-    this.formatOptions.currency = currency;
+
-    this._calculateValues({ source: null });
+    this.formatOptions.currency = currency || undefined;
-    this.__setCurrencyDisplayLabel();
+    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) {
+    if (this.__currencyDisplayNode) {
-      this._currencyDisplayNode.setAttribute('aria-label', getCurrencyName(this.currency, {}));
+      this.__currencyDisplayNode.setAttribute(
+        'aria-label',
+        this.currency ? getCurrencyName(this.currency, {}) : '',
+      );
     }
   }
 

packages/input-amount/test/lion-input-amount.test.js

@@ -197,6 +197,60 @@ describe('<lion-input-amount>', () => {
     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(`<lion-input-amount currency="EUR"></lion-input-amount>`)
+    );
+    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(`<lion-input-amount></lion-input-amount>`)
+    );
+    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(`
+        <lion-input-amount>
+          <span slot="after" id="123">Currency, please</span>
+        </lion-input-amount>`)
+    );
+    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('<lion-input-amount>', () => {
       const el = /** @type {LionInputAmount} */ (
         await fixture(`<lion-input-amount currency="EUR"></lion-input-amount>`)
       );
-      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(`<lion-input-amount currency="EUR"></lion-input-amount>`)
       );
-      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.