Skip to content

Commit

Permalink
fix: add form submission and reset behavior to Button (#3603)
Browse files Browse the repository at this point in the history
* add button submit and reset fix

* adding tests

* remove debugger statements

* remove duplicate proxy attachment code

* make methods private

Co-authored-by: nicholasrice <nicholasrice@users.noreply.github.com>
  • Loading branch information
nicholasrice and nicholasrice authored Jul 30, 2020
1 parent c145e8e commit f646343
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,14 @@ <h4>Icon in default slot</h4>

<h4>With aria-label</h4>
<fast-button aria-label="Button with aria-label">Button</fast-button>

<form>
<input type="text" name="test" />
<fast-button type="submit">Submit</fast-button>
</form>

<form>
<input type="text" name="test-2" />
<fast-button type="reset">Reset</fast-button>
</form>
</fast-design-system-provider>
1 change: 0 additions & 1 deletion packages/web-components/fast-foundation/docs/api-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ export class Button extends FormAssociated<HTMLInputElement> {
formmethod: string;
formnovalidate: boolean;
formtarget: "_self" | "_blank" | "_parent" | "_top";
name: string;
// (undocumented)
protected proxy: HTMLInputElement;
type: "submit" | "reset" | "button";
Expand Down
54 changes: 54 additions & 0 deletions packages/web-components/fast-foundation/src/button/button.spec.ts
Original file line number Diff line number Diff line change
@@ -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`
<slot></slot>
`,
})
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);
});
});
});
58 changes: 43 additions & 15 deletions packages/web-components/fast-foundation/src/button/button.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -105,16 +105,6 @@ export class Button extends FormAssociated<HTMLInputElement> {
}
}

/**
* The name of the button
*
* @public
* @remarks
* HTML Attribute: name
*/
@attr
public name: string;

/**
* The button type.
*
Expand All @@ -124,10 +114,18 @@ export class Button extends FormAssociated<HTMLInputElement> {
*/
@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");
Expand All @@ -138,10 +136,40 @@ export class Button extends FormAssociated<HTMLInputElement> {
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();
};
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit f646343

Please sign in to comment.