Merge pull request #281 from ing-bank/fix/button

Fix button implicit submission on enter keypress

packages/button/src/LionButton.js

@@ -71,7 +71,12 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
 
         :host .btn ::slotted(button) {
           position: absolute;
-          visibility: hidden;
+          clip: rect(0 0 0 0);
+          clip-path: inset(50%);
+          overflow: hidden;
+          white-space: nowrap;
+          height: 1px;
+          width: 1px;
         }
 
         .click-area {
@@ -120,7 +125,6 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
       _button: () => {
         if (!this.constructor._button) {
           this.constructor._button = document.createElement('button');
-          this.constructor._button.setAttribute('slot', '_button');
           this.constructor._button.setAttribute('tabindex', '-1');
         }
         return this.constructor._button.cloneNode();
@@ -167,31 +171,13 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
     }
   }
 
-  _redispatchClickEvent(oldEvent) {
-    // replacing `MouseEvent` with `oldEvent.constructor` breaks IE
-    const newEvent = new MouseEvent(oldEvent.type, oldEvent);
-    newEvent.__isRedispatchedOnNativeButton = true;
-    this.__enforceHostEventTarget(newEvent);
-    this._nativeButtonNode.dispatchEvent(newEvent);
-  }
-
   /**
-   * Prevent normal click and redispatch click on the native button unless already redispatched.
+   * Dispatch submit event and invoke submit on the native form when clicked
    */
-  __clickDelegationHandler(e) {
-    if (!e.__isRedispatchedOnNativeButton) {
-      e.stopImmediatePropagation();
-      this._redispatchClickEvent(e);
-    }
-  }
-
-  __enforceHostEventTarget(event) {
-    try {
-      // this is for IE11 (and works in others), because `Object.defineProperty` does not give any effect there
-      event.__defineGetter__('target', () => this); // eslint-disable-line no-restricted-properties
-    } catch (error) {
-      // in case `__defineGetter__` is removed from the platform
-      Object.defineProperty(event, 'target', { writable: false, value: this });
+  __clickDelegationHandler() {
+    if (this.type === 'submit' && this._nativeButtonNode.form) {
+      this._nativeButtonNode.form.dispatchEvent(new Event('submit'));
+      this._nativeButtonNode.form.submit();
     }
   }
 
@@ -239,7 +225,7 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
   __keyupHandler(e) {
     if (this.__isKeyboardClickEvent(e)) {
       // redispatch click
-      this.shadowRoot.querySelector('.click-area').click();
+      this.click();
     }
   }
 

packages/button/stories/index.stories.js

@@ -35,15 +35,14 @@ storiesOf('Buttons|Button', module)
   .add(
     'Within a form',
     () => html`
-      <lion-form id="form"
-        ><form>
-          <lion-input name="foo" label="Foo" .modelValue=${'bar'}></lion-input>
-          <lion-button
-            type="submit"
-            @click=${() => console.log(document.querySelector('#form').serializeGroup())}
-            >Submit</lion-button
-          >
-        </form></lion-form
-      >
+      <form @submit=${() => console.log('native form submitted')}>
+        <input name="foo" label="Foo" .modelValue=${'bar'} />
+        <input name="foo2" label="Foo2" .modelValue=${'bar'} />
+        <lion-button
+          type="submit"
+          @click=${() => console.log(document.querySelector('#form').serializeGroup())}
+          >Submit</lion-button
+        >
+      </form>
     `,
   );

packages/button/test/lion-button.test.js

@@ -57,7 +57,8 @@ describe('lion-button', () => {
   it('hides the native button in the UI', async () => {
     const el = await fixture(`<lion-button>foo</lion-button>`);
     expect(el._nativeButtonNode.getAttribute('tabindex')).to.equal('-1');
-    expect(window.getComputedStyle(el._nativeButtonNode).visibility).to.equal('hidden');
+    // TODO: If we abstract to an srOnlyMixin, we should test that the styling equals that of the srOnlyMixin output
+    expect(window.getComputedStyle(el._nativeButtonNode).clip).to.equal('rect(0px, 0px, 0px, 0px)');
   });
 
   it('can be disabled imperatively', async () => {
@@ -231,6 +232,8 @@ describe('lion-button', () => {
           <lion-button type="submit">foo</lion-button>
         </form>
       `);
+      // Prevent page refresh
+      form.submit = () => {};
 
       const button = form.querySelector('lion-button');
       getTopElement(button).click();
@@ -245,6 +248,8 @@ describe('lion-button', () => {
           <lion-button type="submit">foo</lion-button>
         </form>
       `);
+      // Prevent page refresh
+      form.submit = () => {};
 
       pressSpace(form.querySelector('lion-button'));
       await aTimeout();
@@ -260,6 +265,8 @@ describe('lion-button', () => {
           <lion-button type="submit">foo</lion-button>
         </form>
       `);
+      // Prevent page refresh
+      form.submit = () => {};
 
       pressEnter(form.querySelector('lion-button'));
       await aTimeout();
@@ -267,6 +274,26 @@ describe('lion-button', () => {
 
       expect(formSubmitSpy.called).to.be.true;
     });
+
+    // input "enter" keypress mock doesn't seem to work right now, but should be tested in the future (maybe with Selenium)
+    it.skip('works with implicit form submission on-enter inside an input', async () => {
+      const formSubmitSpy = sinon.spy(e => e.preventDefault());
+      const form = await fixture(html`
+        <form @submit="${formSubmitSpy}">
+          <input name="foo" />
+          <input name="foo2" />
+          <lion-button type="submit">foo</lion-button>
+        </form>
+      `);
+      // Prevent page refresh
+      form.submit = () => {};
+
+      pressEnter(form.querySelector('input[name="foo2"]'));
+      await aTimeout();
+      await aTimeout();
+
+      expect(formSubmitSpy.called).to.be.true;
+    });
   });
 
   describe('click event', () => {