@lion/button - npm Package Compare versions

Comparing version 0.3.26 to 0.3.27

CHANGELOG.md

@@ -6,2 +6,13 @@ # Change Log
 
+## [0.3.27](https://github.com/ing-bank/lion/compare/@lion/button@0.3.26...@lion/button@0.3.27) (2019-10-21)
+
+
+### Bug Fixes
+
+* **button:** fix form integration, fires only once, submit preventable ([d19ca7c](https://github.com/ing-bank/lion/commit/d19ca7c))
+
+
+
+
+
 ## [0.3.26](https://github.com/ing-bank/lion/compare/@lion/button@0.3.25...@lion/button@0.3.26) (2019-10-14)
@@ -8,0 +19,0 @@

package.json

 {
   "name": "@lion/button",
-  "version": "0.3.26",
+  "version": "0.3.27",
   "description": "A button that is easily styleable and accessible in all contexts",
@@ -46,3 +46,3 @@ "author": "ing-bank",
   },
-  "gitHead": "5e3099ae9df5f0b307931342059910fb3510ab86"
+  "gitHead": "0ca59c82cc6fd274b2fead269bd485b62648d813"
 }

README.md

@@ -39,10 +39,7 @@ # Button
 
-- Don't use a button when you want a user to navigate. Use a link instead.
-- Not all color and font size combinations are available because some do not meet accessibility contrast requirements
-
 ## Considerations
 
-### Why a webcomponent?
+### Why a Web Component?
 
-There are multiple reasons why we used a web component as opposed to a CSS component.
+There are multiple reasons why we used a Web Component as opposed to a CSS component.
 
@@ -52,1 +49,27 @@ - **Target size**: The minimum target size is 40 pixels, which makes even the small buttons easy to activate. A container element was needed to make this size possible.
 - **Advanced styling**: There are advanced styling options regarding icons in buttons, where it is a lot more maintainable to handle icons in our button using slots. An example is that a sticky icon-only buttons may looks different from buttons which have both icons and text.
+
+### Event target
+
+We want to ensure that the event target returned to the user is `lion-button`, not `button`. Therefore, simply delegating the click to the native button immediately, is not desired. Instead, we catch the click event in the `lion-button`, and ensure delegation inside of there.
+
+### Flashing a native button click as a direct child of form
+
+By delegating the `click()` to the native button, it will bubble back up to `lion-button` which would cause duplicate actions. We have to simulate the full `.click()` however, otherwise form submission is not triggered. So this bubbling cannot be prevented.
+Therefore, on click, we flash a `<button>` to the form as a direct child and fire the click on that button. We then immediately remove that button. This is a fully synchronous process; users or developers will not notice this, it should not cause problems.
+
+### Native button & implicit form submission
+
+Flashing the button in the way we do solves almost all issues except for one.
+One of the specs of W3C is that when you have a form with multiple inputs, pressing enter while inside one of the inputs only triggers a form submit if that form has a button of type submit.
+
+To get this particular implicit form submission to work, having a native button in our `lion-button` is a hard requirement. Therefore, not only do we flash a native button on the form to delegate `lion-button` trigger to `button` and thereby trigger form submission, we **also** add a native `button` inside the `lion-button` which `type` property is synchronized with the type of the `lion-button`.
+
+### Preventing full page reloads
+
+To prevent form submission full page reloads, add a **submit handler on the form** like so:
+
+```html
+<form @submit=${ev => ev.preventDefault()} >
+```
+
+Putting this on the `@click` of the `lion-button` is not enough.

src/LionButton.js

 import { css, html, SlotMixin, DisabledWithTabIndexMixin, LitElement } from '@lion/core';
 
+// eslint-disable-next-line class-methods-use-this
+const isKeyboardClickEvent = e => e.keyCode === 32 /* space */ || e.keyCode === 13; /* enter */
+
 export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement)) {
@@ -122,2 +125,10 @@ static get properties() {
 
+  get _nativeButtonNode() {
+    return this.querySelector('[slot=_button]');
+  }
+
+  get _form() {
+    return this._nativeButtonNode.form;
+  }
+
   get slots() {
@@ -136,6 +147,2 @@ return {
 
-  get _nativeButtonNode() {
-    return this.querySelector('[slot=_button]');
-  }
-
   constructor() {
@@ -177,9 +184,20 @@ super();
   /**
-   * Dispatch submit event and invoke submit on the native form when clicked
+   * 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.
    */
-  __clickDelegationHandler() {
-    if (this.type === 'submit' && this._nativeButtonNode && this._nativeButtonNode.form) {
-      this._nativeButtonNode.form.dispatchEvent(new Event('submit'));
-      this._nativeButtonNode.form.submit();
+  __clickDelegationHandler(e) {
+    if (this.constructor.__isIE11()) {
+      e.stopPropagation();
     }
+
+    if ((this.type === 'submit' || this.type === 'reset') && e.target === this) {
+      if (this._form) {
+        const nativeButton = document.createElement('button');
+        nativeButton.type = this.type;
+        this._form.appendChild(nativeButton);
+        nativeButton.click();
+        this._form.removeChild(nativeButton);
+      }
+    }
   }
@@ -215,8 +233,9 @@
   __keydownHandler(e) {
-    if (this.active || !this.__isKeyboardClickEvent(e)) {
+    if (this.active || !isKeyboardClickEvent(e)) {
       return;
     }
+    // FIXME: In Edge & IE11, this toggling the active state to prevent bounce, does not work.
     this.active = true;
     const keyupHandler = keyupEvent => {
-      if (this.__isKeyboardClickEvent(keyupEvent)) {
+      if (isKeyboardClickEvent(keyupEvent)) {
         this.active = false;
@@ -230,4 +249,8 @@ document.removeEventListener('keyup', keyupHandler, true);
   __keyupHandler(e) {
-    if (this.__isKeyboardClickEvent(e)) {
-      // redispatch click
+    if (isKeyboardClickEvent(e)) {
+      // Fixes IE11 double submit/click. Enter keypress somehow triggers the __keyUpHandler on the native <button>
+      if (e.srcElement && e.srcElement !== this) {
+        return;
+      }
+      // dispatch click
       this.click();
@@ -237,7 +260,2 @@ }
 
-  // eslint-disable-next-line class-methods-use-this
-  __isKeyboardClickEvent(e) {
-    return e.keyCode === 32 /* space */ || e.keyCode === 13 /* enter */;
-  }
-
   static __isIE11() {
@@ -244,0 +262,0 @@ const ua = window.navigator.userAgent;

stories/index.stories.js

@@ -36,14 +36,57 @@ import { storiesOf, html } from '@open-wc/demoing-storybook';
   .add(
-    'Within a form',
+    'Within a native form',
     () => html`
-      <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
+        @submit=${ev => {
+          ev.preventDefault();
+          console.log('submit handler');
+        }}
+      >
+        <label>First name</label>
+        <input name="firstName" />
+        <label>Last name</label>
+        <input name="lastName" />
+        <lion-button @click=${() => console.log('click handler')}>Submit</lion-button>
       </form>
+      <p>
+        Supports the following use cases:
+      </p>
+      <ul>
+        <li>
+          Submit on button click
+        </li>
+        <li>
+          Reset native form fields when using type="reset"
+        </li>
+        <li>
+          Submit on button enter or space keypress
+        </li>
+        <li>
+          Submit on enter keypress inside an input
+        </li>
+      </ul>
+      <p>Important notes:</p>
+      <ul>
+        <li>
+          A (lion)-button of type submit is mandatory for the last use case, if you have multiple
+          inputs. This is native behavior.
+        </li>
+        <li>
+          <span style="background-color: azure">
+            <code>@click</code> on <code>lion-button</code>
+          </span>
+          and
+          <span style="background-color: seashell">
+            <code>@submit</code> on <code>form</code>
+          </span>
+          are triggered by these use cases. We strongly encourage you to listen to the submit
+          handler if your goal is to do something on form-submit
+        </li>
+        <li>
+          To prevent form submission full page reloads, add a <b>submit handler on the form</b>
+          <code>@submit</code> with <code>event.preventDefault()</code>. Adding it on the
+          <code>lion-button</code> is not enough.
+        </li>
+      </ul>
     `,
   );

test/lion-button.test.js

@@ -228,70 +228,151 @@ import { expect, fixture, html, aTimeout, oneEvent } from '@open-wc/testing';
   describe('form integration', () => {
-    it('behaves like native `button` when clicked', async () => {
-      const formSubmitSpy = sinon.spy(e => e.preventDefault());
-      const form = await fixture(html`
-        <form @submit="${formSubmitSpy}">
-          <lion-button type="submit">foo</lion-button>
-        </form>
-      `);
-      // Prevent page refresh
-      form.submit = () => {};
+    describe('with submit event', () => {
+      it('behaves like native `button` when clicked', async () => {
+        const formSubmitSpy = sinon.spy(e => e.preventDefault());
+        const form = await fixture(html`
+          <form @submit="${formSubmitSpy}">
+            <lion-button type="submit">foo</lion-button>
+          </form>
+        `);
 
-      const button = form.querySelector('lion-button');
-      getTopElement(button).click();
+        const button = form.querySelector('lion-button');
+        getTopElement(button).click();
 
-      expect(formSubmitSpy.called).to.be.true;
-    });
+        expect(formSubmitSpy.callCount).to.equal(1);
+      });
 
-    it('behaves like native `button` when interected with keyboard space', async () => {
-      const formSubmitSpy = sinon.spy(e => e.preventDefault());
-      const form = await fixture(html`
-        <form @submit="${formSubmitSpy}">
-          <lion-button type="submit">foo</lion-button>
-        </form>
-      `);
-      // Prevent page refresh
-      form.submit = () => {};
+      it('behaves like native `button` when interacted with keyboard space', async () => {
+        const formSubmitSpy = sinon.spy(e => e.preventDefault());
+        const form = await fixture(html`
+          <form @submit="${formSubmitSpy}">
+            <lion-button type="submit">foo</lion-button>
+          </form>
+        `);
 
-      pressSpace(form.querySelector('lion-button'));
-      await aTimeout();
-      await aTimeout();
+        pressSpace(form.querySelector('lion-button'));
+        await aTimeout();
+        await aTimeout();
 
-      expect(formSubmitSpy.called).to.be.true;
-    });
+        expect(formSubmitSpy.callCount).to.equal(1);
+      });
 
-    it('behaves like native `button` when interected with keyboard enter', async () => {
-      const formSubmitSpy = sinon.spy(e => e.preventDefault());
-      const form = await fixture(html`
-        <form @submit="${formSubmitSpy}">
-          <lion-button type="submit">foo</lion-button>
-        </form>
-      `);
-      // Prevent page refresh
-      form.submit = () => {};
+      it('behaves like native `button` when interacted with keyboard enter', async () => {
+        const formSubmitSpy = sinon.spy(e => e.preventDefault());
+        const form = await fixture(html`
+          <form @submit="${formSubmitSpy}">
+            <lion-button type="submit">foo</lion-button>
+          </form>
+        `);
 
-      pressEnter(form.querySelector('lion-button'));
-      await aTimeout();
-      await aTimeout();
+        pressEnter(form.querySelector('lion-button'));
+        await aTimeout();
+        await aTimeout();
 
-      expect(formSubmitSpy.called).to.be.true;
+        expect(formSubmitSpy.callCount).to.equal(1);
+      });
+
+      it('supports resetting form inputs in a native form', async () => {
+        const form = await fixture(html`
+          <form>
+            <input name="firstName" />
+            <input name="lastName" />
+            <lion-button type="reset">reset</lion-button>
+          </form>
+        `);
+        const btn = form.querySelector('lion-button');
+        const firstName = form.querySelector('input[name=firstName]');
+        const lastName = form.querySelector('input[name=lastName]');
+        firstName.value = 'Foo';
+        lastName.value = 'Bar';
+
+        expect(firstName.value).to.equal('Foo');
+        expect(lastName.value).to.equal('Bar');
+
+        btn.click();
+
+        expect(firstName.value).to.be.empty;
+        expect(lastName.value).to.be.empty;
+      });
+
+      // 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>
+        `);
+
+        pressEnter(form.querySelector('input[name="foo2"]'));
+        await aTimeout();
+        await aTimeout();
+
+        expect(formSubmitSpy.callCount).to.equal(1);
+      });
     });
 
-    // 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 = () => {};
+    describe('with click event', () => {
+      it('behaves like native `button` when clicked', async () => {
+        const formButtonClickedSpy = sinon.spy();
+        const form = await fixture(html`
+          <form @submit=${ev => ev.preventDefault()}>
+            <lion-button @click="${formButtonClickedSpy}" type="submit">foo</lion-button>
+          </form>
+        `);
 
-      pressEnter(form.querySelector('input[name="foo2"]'));
-      await aTimeout();
-      await aTimeout();
+        const button = form.querySelector('lion-button');
+        getTopElement(button).click();
 
-      expect(formSubmitSpy.called).to.be.true;
+        expect(formButtonClickedSpy.callCount).to.equal(1);
+      });
+
+      it('behaves like native `button` when interacted with keyboard space', async () => {
+        const formButtonClickedSpy = sinon.spy();
+        const form = await fixture(html`
+          <form @submit=${ev => ev.preventDefault()}>
+            <lion-button @click="${formButtonClickedSpy}" type="submit">foo</lion-button>
+          </form>
+        `);
+
+        pressSpace(form.querySelector('lion-button'));
+        await aTimeout();
+        await aTimeout();
+
+        expect(formButtonClickedSpy.callCount).to.equal(1);
+      });
+
+      it('behaves like native `button` when interacted with keyboard enter', async () => {
+        const formButtonClickedSpy = sinon.spy();
+        const form = await fixture(html`
+          <form @submit=${ev => ev.preventDefault()}>
+            <lion-button @click="${formButtonClickedSpy}" type="submit">foo</lion-button>
+          </form>
+        `);
+
+        pressEnter(form.querySelector('lion-button'));
+        await aTimeout();
+        await aTimeout();
+
+        expect(formButtonClickedSpy.callCount).to.equal(1);
+      });
+
+      // 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 formButtonClickedSpy = sinon.spy();
+        const form = await fixture(html`
+          <form @submit=${ev => ev.preventDefault()}>
+            <input name="foo" />
+            <input name="foo2" />
+            <lion-button @click="${formButtonClickedSpy}" type="submit">foo</lion-button>
+          </form>
+        `);
+
+        pressEnter(form.querySelector('input[name="foo2"]'));
+        await aTimeout();
+        await aTimeout();
+
+        expect(formButtonClickedSpy.callCount).to.equal(1);
+      });
     });
@@ -298,0 +379,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

46680

increased by17.53%

Lines of code

684

increased by22.14%

Number of lines in readme file

74

increased by45.1%

No dependency changes