Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add form submission and reset behavior to Button #3603

Merged
merged 6 commits into from
Jul 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>
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
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);
});
});
});
chrisdholt marked this conversation as resolved.
Show resolved Hide resolved
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