From b499c26aed3e3a703c7cbde0d40d64bb06174f36 Mon Sep 17 00:00:00 2001 From: nicholasrice Date: Thu, 30 Jul 2020 15:00:35 -0700 Subject: [PATCH 1/5] add button submit and reset fix --- .../src/button/fixtures/button.html | 10 +++ .../fast-foundation/docs/api-report.md | 5 +- .../fast-foundation/src/button/button.ts | 62 ++++++++++++++----- .../src/form-associated/form-associated.ts | 39 ++++++++++++ 4 files changed, 100 insertions(+), 16 deletions(-) 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..5048e427514 100644 --- a/packages/web-components/fast-foundation/docs/api-report.md +++ b/packages/web-components/fast-foundation/docs/api-report.md @@ -132,7 +132,10 @@ export class Button extends FormAssociated { formmethod: string; formnovalidate: boolean; formtarget: "_self" | "_blank" | "_parent" | "_top"; - name: string; + // @internal + handleFormReset: () => void; + // @internal + handleSubmission: () => void; // (undocumented) protected proxy: HTMLInputElement; type: "submit" | "reset" | "button"; diff --git a/packages/web-components/fast-foundation/src/button/button.ts b/packages/web-components/fast-foundation/src/button/button.ts index a8145533b5a..87375208c4f 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,44 @@ 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 + * @internal + */ + public handleSubmission = () => { + debugger; + 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 + * @internal + */ + public handleFormReset = () => { + debugger; + 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..44f4354ab07 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 @@ -283,6 +283,7 @@ export abstract class FormAssociated< if (typeof this.name === "string") { this.proxy.name = this.name; } + if (typeof this.value === "string") { this.proxy.value = this.value; } @@ -344,6 +345,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 From a538672631380f833ebd8d0488a0f28f540feeca Mon Sep 17 00:00:00 2001 From: nicholasrice Date: Thu, 30 Jul 2020 15:32:05 -0700 Subject: [PATCH 2/5] adding tests --- .../fast-foundation/src/button/button.spec.ts | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 packages/web-components/fast-foundation/src/button/button.spec.ts 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); + }); + }); +}); From d72d58e8a9cce6fe6dc9806c6f17355a7e4a452b Mon Sep 17 00:00:00 2001 From: nicholasrice Date: Thu, 30 Jul 2020 15:38:16 -0700 Subject: [PATCH 3/5] remove debugger statements --- packages/web-components/fast-foundation/src/button/button.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/web-components/fast-foundation/src/button/button.ts b/packages/web-components/fast-foundation/src/button/button.ts index 87375208c4f..577764a793e 100644 --- a/packages/web-components/fast-foundation/src/button/button.ts +++ b/packages/web-components/fast-foundation/src/button/button.ts @@ -144,7 +144,6 @@ export class Button extends FormAssociated { * @internal */ public handleSubmission = () => { - debugger; if (!this.form) { return; } @@ -171,7 +170,6 @@ export class Button extends FormAssociated { * @internal */ public handleFormReset = () => { - debugger; this.form?.reset(); }; } From 624fb3777771c5736cccbfd07a68ae08823169d2 Mon Sep 17 00:00:00 2001 From: nicholasrice Date: Thu, 30 Jul 2020 15:41:22 -0700 Subject: [PATCH 4/5] remove duplicate proxy attachment code --- .../src/form-associated/form-associated.ts | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) 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 44f4354ab07..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,26 +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(); } } From bef33d3c1e294b35500d80d4968bb22dda234964 Mon Sep 17 00:00:00 2001 From: nicholasrice Date: Thu, 30 Jul 2020 15:50:33 -0700 Subject: [PATCH 5/5] make methods private --- packages/web-components/fast-foundation/docs/api-report.md | 4 ---- .../web-components/fast-foundation/src/button/button.ts | 6 ++---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/web-components/fast-foundation/docs/api-report.md b/packages/web-components/fast-foundation/docs/api-report.md index 5048e427514..06072c6ccff 100644 --- a/packages/web-components/fast-foundation/docs/api-report.md +++ b/packages/web-components/fast-foundation/docs/api-report.md @@ -132,10 +132,6 @@ export class Button extends FormAssociated { formmethod: string; formnovalidate: boolean; formtarget: "_self" | "_blank" | "_parent" | "_top"; - // @internal - handleFormReset: () => void; - // @internal - handleSubmission: () => void; // (undocumented) protected proxy: HTMLInputElement; type: "submit" | "reset" | "button"; diff --git a/packages/web-components/fast-foundation/src/button/button.ts b/packages/web-components/fast-foundation/src/button/button.ts index 577764a793e..e059272809b 100644 --- a/packages/web-components/fast-foundation/src/button/button.ts +++ b/packages/web-components/fast-foundation/src/button/button.ts @@ -141,9 +141,8 @@ export class Button extends FormAssociated { /** * Submits the parent form - * @internal */ - public handleSubmission = () => { + private handleSubmission = () => { if (!this.form) { return; } @@ -167,9 +166,8 @@ export class Button extends FormAssociated { /** * Resets the parent form - * @internal */ - public handleFormReset = () => { + private handleFormReset = () => { this.form?.reset(); }; }