From f6463436a001d07a7cfbd5d87460c76cf8584e65 Mon Sep 17 00:00:00 2001 From: Nicholas Rice <3213292+nicholasrice@users.noreply.github.com> Date: Thu, 30 Jul 2020 16:33:24 -0700 Subject: [PATCH] fix: add form submission and reset behavior to Button (#3603) * add button submit and reset fix * adding tests * remove debugger statements * remove duplicate proxy attachment code * make methods private Co-authored-by: nicholasrice --- .../src/button/fixtures/button.html | 10 ++++ .../fast-foundation/docs/api-report.md | 1 - .../fast-foundation/src/button/button.spec.ts | 54 +++++++++++++++++ .../fast-foundation/src/button/button.ts | 58 ++++++++++++++----- .../src/form-associated/form-associated.ts | 58 +++++++++++++------ 5 files changed, 146 insertions(+), 35 deletions(-) create mode 100644 packages/web-components/fast-foundation/src/button/button.spec.ts diff --git a/packages/web-components/fast-components/src/button/fixtures/button.html b/packages/web-components/fast-components/src/button/fixtures/button.html index 134d28033cd..17f5c83c5b7 100644 --- a/packages/web-components/fast-components/src/button/fixtures/button.html +++ b/packages/web-components/fast-components/src/button/fixtures/button.html @@ -86,4 +86,14 @@

Icon in default slot

With aria-label

Button + +
+ + Submit +
+ +
+ + Reset +
diff --git a/packages/web-components/fast-foundation/docs/api-report.md b/packages/web-components/fast-foundation/docs/api-report.md index a98fac0b45e..06072c6ccff 100644 --- a/packages/web-components/fast-foundation/docs/api-report.md +++ b/packages/web-components/fast-foundation/docs/api-report.md @@ -132,7 +132,6 @@ export class Button extends FormAssociated { formmethod: string; formnovalidate: boolean; formtarget: "_self" | "_blank" | "_parent" | "_top"; - name: string; // (undocumented) protected proxy: HTMLInputElement; type: "submit" | "reset" | "button"; diff --git a/packages/web-components/fast-foundation/src/button/button.spec.ts b/packages/web-components/fast-foundation/src/button/button.spec.ts new file mode 100644 index 00000000000..8b483a39711 --- /dev/null +++ b/packages/web-components/fast-foundation/src/button/button.spec.ts @@ -0,0 +1,54 @@ +import { expect } from "chai"; +import { customElement, html, DOM } from "@microsoft/fast-element"; +import { classNames } from "@microsoft/fast-web-utilities"; +import { Button } from "./button"; + +@customElement({ + name: "test-button", + template: html` + + `, +}) +class TestElement extends Button {} + +describe("Button", () => { + describe("of type 'submit'", () => { + it("should submit the parent form when clicked", () => { + let wasSumbitted = false; + let event: any; + const form = document.createElement("form"); + const submit = document.createElement("test-button"); + submit.setAttribute("type", "submit"); + form.appendChild(submit); + form.addEventListener("submit", e => { + e.preventDefault(); + wasSumbitted = true; + event = e; + }); + document.body.appendChild(form); + + submit.click(); + + expect(wasSumbitted).to.equal(true); + expect(event.submitter).to.equal(submit["proxy"] as any); + }); + }); + + describe("of type 'reset'", () => { + it("should submit the parent form when clicked", () => { + let wasReset = false; + const form = document.createElement("form"); + const reset = document.createElement("test-button"); + reset.setAttribute("type", "reset"); + form.appendChild(reset); + document.body.appendChild(form); + form.addEventListener("reset", () => { + wasReset = true; + }); + + reset.click(); + + expect(wasReset).to.equal(true); + }); + }); +}); diff --git a/packages/web-components/fast-foundation/src/button/button.ts b/packages/web-components/fast-foundation/src/button/button.ts index a8145533b5a..e059272809b 100644 --- a/packages/web-components/fast-foundation/src/button/button.ts +++ b/packages/web-components/fast-foundation/src/button/button.ts @@ -1,4 +1,4 @@ -import { attr } from "@microsoft/fast-element"; +import { attr, observable } from "@microsoft/fast-element"; import { FormAssociated } from "../form-associated/form-associated"; import { ARIAGlobalStatesAndProperties, StartEnd } from "../patterns/index"; import { applyMixins } from "../utilities/apply-mixins"; @@ -105,16 +105,6 @@ export class Button extends FormAssociated { } } - /** - * The name of the button - * - * @public - * @remarks - * HTML Attribute: name - */ - @attr - public name: string; - /** * The button type. * @@ -124,10 +114,18 @@ export class Button extends FormAssociated { */ @attr public type: "submit" | "reset" | "button"; - private typeChanged(): void { + private typeChanged( + previous: "submit" | "reset" | "button" | void, + next: "submit" | "reset" | "button" + ): void { if (this.proxy instanceof HTMLElement) { this.proxy.type = this.type; } + + next === "submit" && this.addEventListener("click", this.handleSubmission); + previous === "submit" && this.removeEventListener("click", this.handleSubmission); + next === "reset" && this.addEventListener("click", this.handleFormReset); + previous === "reset" && this.removeEventListener("click", this.handleFormReset); } protected proxy: HTMLInputElement = document.createElement("input"); @@ -138,10 +136,40 @@ export class Button extends FormAssociated { public connectedCallback(): void { super.connectedCallback(); - this.proxy.setAttribute("type", `${this.type}`); - - this.setFormValue(this.value, this.value); + this.proxy.setAttribute("type", this.type); } + + /** + * Submits the parent form + */ + private handleSubmission = () => { + if (!this.form) { + return; + } + + const attached = this.proxy.isConnected; + + if (!attached) { + super.attachProxy(); + } + + // Browser support for requestSubmit is not comprehensive + // so click the proxy if it isn't supported + typeof this.form.requestSubmit === "function" + ? this.form.requestSubmit(this.proxy) + : this.proxy.click(); + + if (!attached) { + super.detachProxy(); + } + }; + + /** + * Resets the parent form + */ + private handleFormReset = () => { + this.form?.reset(); + }; } /** diff --git a/packages/web-components/fast-foundation/src/form-associated/form-associated.ts b/packages/web-components/fast-foundation/src/form-associated/form-associated.ts index f8c15492f78..c1803069d7a 100644 --- a/packages/web-components/fast-foundation/src/form-associated/form-associated.ts +++ b/packages/web-components/fast-foundation/src/form-associated/form-associated.ts @@ -267,25 +267,7 @@ export abstract class FormAssociated< this.dirtyValue = false; if (!supportsElementInternals) { - this.proxy.style.display = "none"; - this.appendChild(this.proxy); - - this.proxyEventsToBlock.forEach(name => - this.proxy.addEventListener(name, this.stopPropagation) - ); - - // These are typically mapped to the proxy during - // property change callbacks, but during initialization - // on the initial call of the callback, the proxy is - // still undefined. We should find a better way to address this. - this.proxy.disabled = this.disabled; - this.proxy.required = this.required; - if (typeof this.name === "string") { - this.proxy.name = this.name; - } - if (typeof this.value === "string") { - this.proxy.value = this.value; - } + this.attachProxy(); } } @@ -344,6 +326,44 @@ export abstract class FormAssociated< this.disabled = disabled; } + private proxyInitialized: boolean = false; + + /** + * Attach the proxy element to the DOM + */ + protected attachProxy() { + if (!this.proxyInitialized) { + this.proxyInitialized = true; + this.proxy.style.display = "none"; + this.proxyEventsToBlock.forEach(name => + this.proxy.addEventListener(name, this.stopPropagation) + ); + + // These are typically mapped to the proxy during + // property change callbacks, but during initialization + // on the initial call of the callback, the proxy is + // still undefined. We should find a better way to address this. + this.proxy.disabled = this.disabled; + this.proxy.required = this.required; + if (typeof this.name === "string") { + this.proxy.name = this.name; + } + + if (typeof this.value === "string") { + this.proxy.value = this.value; + } + } + + this.appendChild(this.proxy); + } + + /** + * Detach the proxy element from the DOM + */ + protected detachProxy() { + this.removeChild(this.proxy); + } + /** * * @param value - The value to set