@lion/button - npm Package Compare versions

Comparing version 0.8.4 to 0.8.5

CHANGELOG.md

 # Change Log
 
+## 0.8.5
+
+### Patch Changes
+
+- 27020f12: Button fixes
+
+  - make click event target predictable (always host)
+  - do not override aria-labelledby from user
+
 ## 0.8.4
@@ -4,0 +13,0 @@

package.json

 {
   "name": "@lion/button",
-  "version": "0.8.4",
+  "version": "0.8.5",
   "description": "A button that is easily styleable and accessible in all contexts",
@@ -5,0 +5,0 @@ "license": "MIT",

src/LionButton.d.ts

@@ -22,3 +22,2 @@ declare const LionButton_base: typeof LitElement & import("@open-wc/dedupe-mixin").Constructor<import("@lion/core/types/SlotMixinTypes").SlotHost> & import("@open-wc/dedupe-mixin").Constructor<import("@lion/core/types/DisabledWithTabIndexMixinTypes").DisabledWithTabIndexHost> & import("@open-wc/dedupe-mixin").Constructor<import("@lion/core/types/DisabledMixinTypes").DisabledHost>;
     get _nativeButtonNode(): HTMLButtonElement;
-    get _form(): HTMLFormElement | null;
     role: string;
@@ -28,9 +27,16 @@ type: string;
     _buttonId: string;
+    /** @type {HTMLButtonElement} */
+    __submitAndResetHelperButton: HTMLButtonElement;
     /**
+     * Prevents that someone who listens outside or on form catches the click event
+     * @param {Event} e
+     */
+    __preventEventLeakage(e: Event): void;
+    /**
      * Delegate click, by flashing a native button as a direct child
      * of the form, and firing click on this button. This will fire the form submit
      * without side effects caused by the click bubbling back up to lion-button.
-     * @param {Event} e
+     * @param {Event} ev
      */
-    __clickDelegationHandler(e: Event): void;
+    __clickDelegationHandler(ev: Event): void;
     __setupDelegationInConstructor(): void;
@@ -48,4 +54,7 @@ __setupEvents(): void;
     __keyupHandler(e: KeyboardEvent): void;
+    __setupSubmitAndResetHelperOnConnected(): void;
+    _form: HTMLFormElement | null | undefined;
+    __teardownSubmitAndResetHelperOnDisconnected(): void;
 }
 import { LitElement } from "@lion/core";
 export {};

src/LionButton.js

@@ -149,6 +149,2 @@ import {
 
-  get _form() {
-    return this._nativeButtonNode.form;
-  }
-
   // @ts-ignore
@@ -177,4 +173,14 @@ get slots() {
     if (browserDetection.isIE11) {
-      this.updateComplete.then(() => this.setAttribute('aria-labelledby', this._buttonId));
+      this.updateComplete.then(() => {
+        if (!this.hasAttribute('aria-labelledby')) {
+          this.setAttribute('aria-labelledby', this._buttonId);
+        }
+      });
     }
+
+    /** @type {HTMLButtonElement} */
+    this.__submitAndResetHelperButton = document.createElement('button');
+
+    /** @type {EventListener} */
+    this.__preventEventLeakage = this.__preventEventLeakage.bind(this);
   }
@@ -185,2 +191,3 @@
     this.__setupEvents();
+    this.__setupSubmitAndResetHelperOnConnected();
   }
@@ -191,2 +198,3 @@
     this.__teardownEvents();
+    this.__teardownSubmitAndResetHelperOnDisconnected();
   }
@@ -214,8 +222,20 @@
    * without side effects caused by the click bubbling back up to lion-button.
-   * @param {Event} e
+   * @param {Event} ev
    */
-  __clickDelegationHandler(e) {
-    if ((this.type === 'submit' || this.type === 'reset') && e.target === this && this._form) {
-      e.stopImmediatePropagation();
-      this._nativeButtonNode.click();
+  __clickDelegationHandler(ev) {
+    if ((this.type === 'submit' || this.type === 'reset') && ev.target === this && this._form) {
+      /**
+       * Here, we make sure our button is compatible with a native form, by firing a click
+       * from a native button that our form responds to. The native button we spawn will be a direct
+       * child of the form, plus the click event that will be sent will be prevented from
+       * propagating outside of the form. This will keep the amount of 'noise' (click events
+       * from 'ghost elements' that can be intercepted by listeners in the bubble chain) to an
+       * absolute minimum.
+       */
+      this.__submitAndResetHelperButton.type = this.type;
+
+      this._form.appendChild(this.__submitAndResetHelperButton);
+      // Form submission or reset will happen
+      this.__submitAndResetHelperButton.click();
+      this._form.removeChild(this.__submitAndResetHelperButton);
     }
@@ -295,2 +315,26 @@ }
   }
+
+  /**
+   * Prevents that someone who listens outside or on form catches the click event
+   * @param {Event} e
+   */
+  __preventEventLeakage(e) {
+    if (e.target === this.__submitAndResetHelperButton) {
+      e.stopImmediatePropagation();
+    }
+  }
+
+  __setupSubmitAndResetHelperOnConnected() {
+    this._form = this._nativeButtonNode.form;
+
+    if (this._form) {
+      this._form.addEventListener('click', this.__preventEventLeakage);
+    }
+  }
+
+  __teardownSubmitAndResetHelperOnDisconnected() {
+    if (this._form) {
+      this._form.removeEventListener('click', this.__preventEventLeakage);
+    }
+  }
 }

test/lion-button.test.js

@@ -214,2 +214,11 @@ import { browserDetection } from '@lion/core';
 
+    it('does not override aria-labelledby when provided by user', async () => {
+      const browserDetectionStub = sinon.stub(browserDetection, 'isIE11').value(true);
+      const el = /** @type {LionButton} */ (await fixture(
+        `<lion-button aria-labelledby="some-id another-id">foo</lion-button>`,
+      ));
+      expect(el.getAttribute('aria-labelledby')).to.equal('some-id another-id');
+      browserDetectionStub.restore();
+    });
+
     it('has a native button node with aria-hidden set to true', async () => {
@@ -243,5 +252,5 @@ const el = /** @type {LionButton} */ (await fixture('<lion-button></lion-button>'));
         `);
-        const button = /** @type {LionButton} */ (
-          /** @type {LionButton} */ (form.querySelector('lion-button'))
-        );
+        const button /** @type {LionButton} */ = /** @type {LionButton} */ (form.querySelector(
+          'lion-button',
+        ));
         button.click();
@@ -258,5 +267,5 @@ expect(formSubmitSpy).to.have.been.calledOnce;
         `);
-        const button = /** @type {LionButton} */ (
-          /** @type {LionButton} */ (form.querySelector('lion-button'))
-        );
+        const button /** @type {LionButton} */ = /** @type {LionButton} */ (form.querySelector(
+          'lion-button',
+        ));
         button.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' }));
@@ -292,5 +301,5 @@ await aTimeout(0);
         `);
-        const btn = /** @type {LionButton} */ (
-          /** @type {LionButton} */ (form.querySelector('lion-button'))
-        );
+        const btn /** @type {LionButton} */ = /** @type {LionButton} */ (form.querySelector(
+          'lion-button',
+        ));
         const firstName = /** @type {HTMLInputElement} */ (form.querySelector(
@@ -357,5 +366,4 @@ 'input[name=firstName]',
 
-        /** @type {LionButton} */ (form.querySelector('lion-button')).dispatchEvent(
-          new KeyboardEvent('keyup', { key: ' ' }),
-        );
+        const lionButton = /** @type {LionButton} */ (form.querySelector('lion-button'));
+        lionButton.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' }));
         await aTimeout(0);
@@ -418,3 +426,3 @@ await aTimeout(0);
       const el = /** @type {LionButton} */ (await fixture(
-        html`<lion-button @click="${clickSpy}">foo</lion-button>`,
+        html` <lion-button @click="${clickSpy}">foo</lion-button> `,
       ));
@@ -431,13 +439,69 @@
 
-    it('is fired one inside a form', async () => {
-      const formClickSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
-      const el = /** @type {HTMLFormElement} */ (await fixture(
-        html`<form @click="${formClickSpy}">
-          <lion-button>foo</lion-button>
-        </form>`,
+    it('is fired once outside and inside the form', async () => {
+      const outsideSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
+      const insideSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
+      const formSpyEarly = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
+      const formSpyLater = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
+
+      const el = /** @type {HTMLDivElement} */ (await fixture(
+        html`
+          <div @click="${outsideSpy}">
+            <form @click="${formSpyEarly}">
+              <div @click="${insideSpy}">
+                <lion-button>foo</lion-button>
+              </div>
+            </form>
+          </div>
+        `,
       ));
+      const lionButton = /** @type {LionButton} */ (el.querySelector('lion-button'));
+      const form = /** @type {HTMLFormElement} */ (el.querySelector('form'));
+      form.addEventListener('click', formSpyLater);
 
-      // @ts-ignore
-      el.querySelector('lion-button').click();
+      lionButton.click();
+      // trying to wait for other possible redispatched events
+      await aTimeout(0);
+      await aTimeout(0);
 
+      expect(insideSpy).to.have.been.calledOnce;
+      expect(outsideSpy).to.have.been.calledOnce;
+      // A small sacrifice for event listeners registered early: we get the native button evt.
+      expect(formSpyEarly).to.have.been.calledTwice;
+      expect(formSpyLater).to.have.been.calledOnce;
+    });
+
+    it('works when connected to different form', async () => {
+      const form1El = /** @type {HTMLFormElement} */ (await fixture(
+        html`
+          <form>
+            <lion-button>foo</lion-button>
+          </form>
+        `,
+      ));
+      const lionButton = /** @type {LionButton} */ (form1El.querySelector('lion-button'));
+
+      expect(lionButton._form).to.equal(form1El);
+
+      // Now we add the lionButton to a different form.
+      // We disconnect and connect and check if everything still works as expected
+      const outsideSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
+      const insideSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
+      const formSpyEarly = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
+      const formSpyLater = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
+
+      const form2El = /** @type {HTMLFormElement} */ (await fixture(
+        html`
+          <div @click="${outsideSpy}">
+            <form @click="${formSpyEarly}">
+              <div @click="${insideSpy}">${lionButton}</div>
+            </form>
+          </div>
+        `,
+      ));
+      const form2Node = /** @type {HTMLFormElement} */ (form2El.querySelector('form'));
+
+      expect(lionButton._form).to.equal(form2Node);
+
+      form2Node.addEventListener('click', formSpyLater);
+      lionButton.click();
       // trying to wait for other possible redispatched events
@@ -447,3 +511,7 @@ await aTimeout(0);
 
-      expect(formClickSpy).to.have.been.calledOnce;
+      expect(insideSpy).to.have.been.calledOnce;
+      expect(outsideSpy).to.have.been.calledOnce;
+      // A small sacrifice for event listeners registered early: we get the native button evt.
+      expect(formSpyEarly).to.have.been.calledTwice;
+      expect(formSpyLater).to.have.been.calledOnce;
     });
@@ -490,13 +558,13 @@
       const useCases = [
-        { container: 'div', type: 'submit', targetHost: true },
-        { container: 'div', type: 'reset', targetHost: true },
-        { container: 'div', type: 'button', targetHost: true },
-        { container: 'form', type: 'submit', targetHost: false },
-        { container: 'form', type: 'reset', targetHost: false },
-        { container: 'form', type: 'button', targetHost: true },
+        { container: 'div', type: 'submit' },
+        { container: 'div', type: 'reset' },
+        { container: 'div', type: 'button' },
+        { container: 'form', type: 'submit' },
+        { container: 'form', type: 'reset' },
+        { container: 'form', type: 'button' },
       ];
 
       useCases.forEach(useCase => {
-        const { container, type, targetHost } = useCase;
-        const targetName = targetHost ? 'host' : 'native button';
+        const { container, type } = useCase;
+        const targetName = 'host';
         it(`is ${targetName} with type ${type} and it is inside a ${container}`, async () => {
@@ -511,7 +579,3 @@ const clickSpy = /** @type {EventListener} */ (sinon.spy(e => e.preventDefault()));
 
-          if (targetHost) {
-            expect(event.target).to.equal(el);
-          } else {
-            expect(event.target).to.equal(el._nativeButtonNode);
-          }
+          expect(event.target).to.equal(el);
         });
@@ -518,0 +582,0 @@ });

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

74795

increased by7.31%

Lines of code

922

increased by12.58%

No dependency changes