From b78af7598f19ca5e1b440ddc0091a62d86321066 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Fri, 14 Jul 2023 10:24:53 -0700 Subject: [PATCH] fix(button): submit form when pressing enter key (#27790) Issue number: resolves #19368 --------- ## What is the current behavior? The form submits when clicking on the `ion-button`. However, users cannot submit the form when focused on a form element and pressing the `enter` button. This does not follow the behavior of the native button on a form. ## What is the new behavior? - Form now submits when a form element is focused and the `enter` button is pressed. - It also submits regardless of the amount of form elements present. - Form will not submit if the `ion-button` is disabled. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information N/A --- core/src/components/button/button.tsx | 130 +++++++++++------- .../button/test/form-reference/button.e2e.ts | 100 ++++++++++++-- 2 files changed, 169 insertions(+), 61 deletions(-) diff --git a/core/src/components/button/button.tsx b/core/src/components/button/button.tsx index a4e73d15f7..facb64f8d0 100644 --- a/core/src/components/button/button.tsx +++ b/core/src/components/button/button.tsx @@ -1,5 +1,5 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; -import { Component, Element, Event, Host, Prop, h } from '@stencil/core'; +import { Component, Element, Event, Host, Prop, Watch, h } from '@stencil/core'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; import type { Attributes } from '@utils/helpers'; import { inheritAriaAttributes, hasShadowDom } from '@utils/helpers'; @@ -32,6 +32,8 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf private inItem = false; private inListHeader = false; private inToolbar = false; + private formButtonEl: HTMLButtonElement | null = null; + private formEl: HTMLFormElement | null = null; private inheritedAttributes: Attributes = {}; @Element() el!: HTMLElement; @@ -52,6 +54,13 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf * If `true`, the user cannot interact with the button. */ @Prop({ reflect: true }) disabled = false; + @Watch('disabled') + disabledChanged() { + const { disabled } = this; + if (this.formButtonEl) { + this.formButtonEl.disabled = disabled; + } + } /** * Set to `"block"` for a full-width button or to `"full"` for a full-width button @@ -144,6 +153,22 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf */ @Event() ionBlur!: EventEmitter; + connectedCallback(): void { + // Allow form to be submitted through `ion-button` + if (this.type !== 'button' && hasShadowDom(this.el)) { + this.formEl = this.findForm(); + if (this.formEl) { + // Create a hidden native button inside of the form + this.formButtonEl = document.createElement('button'); + this.formButtonEl.type = this.type; + this.formButtonEl.style.display = 'none'; + // Only submit if the button is not disabled. + this.formButtonEl.disabled = this.disabled; + this.formEl.appendChild(this.formButtonEl); + } + } + } + componentWillLoad() { this.inToolbar = !!this.el.closest('ion-buttons'); this.inListHeader = !!this.el.closest('ion-list-header'); @@ -177,12 +202,63 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf return form; } if (typeof form === 'string') { - const el = document.getElementById(form); - if (el instanceof HTMLFormElement) { - return el; + // Check if the string provided is a form id. + const el: HTMLElement | null = document.getElementById(form); + if (el) { + if (el instanceof HTMLFormElement) { + return el; + } else { + /** + * The developer specified a string for the form attribute, but the + * element with that id is not a form element. + */ + printIonWarning( + `Form with selector: "#${form}" could not be found. Verify that the id is attached to a
element.`, + this.el + ); + return null; + } + } else { + /** + * The developer specified a string for the form attribute, but the + * element with that id could not be found in the DOM. + */ + printIonWarning( + `Form with selector: "#${form}" could not be found. Verify that the id is correct and the form is rendered in the DOM.`, + this.el + ); + return null; } } - return null; + if (form !== undefined) { + /** + * The developer specified a HTMLElement for the form attribute, + * but the element is not a HTMLFormElement. + * This will also catch if the developer tries to pass in null + * as the form attribute. + */ + printIonWarning( + `The provided "form" element is invalid. Verify that the form is a HTMLFormElement and rendered in the DOM.`, + this.el + ); + return null; + } + /** + * If the form element is not set, the button may be inside + * of a form element. Query the closest form element to the button. + */ + return this.el.closest('form'); + } + + private submitForm(ev: Event) { + // this button wants to specifically submit a form + // climb up the dom to see if we're in a + // and if so, then use JS to submit it + if (this.formEl && this.formButtonEl) { + ev.preventDefault(); + + this.formButtonEl.click(); + } } private handleClick = (ev: Event) => { @@ -190,49 +266,7 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf if (this.type === 'button') { openURL(this.href, ev, this.routerDirection, this.routerAnimation); } else if (hasShadowDom(el)) { - // this button wants to specifically submit a form - // climb up the dom to see if we're in a - // and if so, then use JS to submit it - let formEl = this.findForm(); - const { form } = this; - - if (!formEl && form !== undefined) { - /** - * The developer specified a form selector for - * the button to submit, but it was not found. - */ - if (typeof form === 'string') { - printIonWarning( - `Form with selector: "#${form}" could not be found. Verify that the id is correct and the form is rendered in the DOM.`, - el - ); - } else { - printIonWarning( - `The provided "form" element is invalid. Verify that the form is a HTMLFormElement and rendered in the DOM.`, - el - ); - } - return; - } - - if (!formEl) { - /** - * If the form element is not set, the button may be inside - * of a form element. Query the closest form element to the button. - */ - formEl = el.closest('form'); - } - - if (formEl) { - ev.preventDefault(); - - const fakeButton = document.createElement('button'); - fakeButton.type = this.type; - fakeButton.style.display = 'none'; - formEl.appendChild(fakeButton); - fakeButton.click(); - fakeButton.remove(); - } + this.submitForm(ev); } }; diff --git a/core/src/components/button/test/form-reference/button.e2e.ts b/core/src/components/button/test/form-reference/button.e2e.ts index dacd22601b..69e525be4b 100644 --- a/core/src/components/button/test/form-reference/button.e2e.ts +++ b/core/src/components/button/test/form-reference/button.e2e.ts @@ -44,7 +44,7 @@ configs({ directions: ['ltr'], modes: ['ios'] }).forEach(({ title, config }) => await page.setContent( ` - Submit + Submit
`, config @@ -56,19 +56,93 @@ configs({ directions: ['ltr'], modes: ['ios'] }).forEach(({ title, config }) => expect(submitEvent).toHaveReceivedEvent(); }); + + test('should submit the closest form by pressing the `enter` button on a form element', async ({ + page, + }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/19368', + }); + + await page.setContent( + ` +
+ + Submit +
+ `, + config + ); + + const submitEvent = await page.spyOnEvent('submit'); + + await page.press('input', 'Enter'); + + expect(submitEvent).toHaveReceivedEvent(); + }); + + test('should submit the closest form with multiple elements by pressing the `enter` button', async ({ + page, + }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/19368', + }); + + await page.setContent( + ` +
+ + + Submit +
+ `, + config + ); + + const submitEvent = await page.spyOnEvent('submit'); + + await page.press('input', 'Enter'); + + expect(submitEvent).toHaveReceivedEvent(); + }); + + test('should not submit the closest form when button is disabled', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/19368', + }); + + await page.setContent( + ` +
+ + Submit +
+ `, + config + ); + + const submitEvent = await page.spyOnEvent('submit'); + + await page.press('input', 'Enter'); + + expect(submitEvent).not.toHaveReceivedEvent(); + }); }); test.describe(title('should throw a warning if the form cannot be found'), () => { test('form is a string selector', async ({ page }) => { - await page.setContent(`Submit`, config); - const logs: string[] = []; page.on('console', (msg) => { - logs.push(msg.text()); + if (msg.type() === 'warning') { + logs.push(msg.text()); + } }); - await page.click('ion-button'); + await page.setContent(`Submit`, config); expect(logs.length).toBe(1); expect(logs[0]).toContain( @@ -77,6 +151,14 @@ configs({ directions: ['ltr'], modes: ['ios'] }).forEach(({ title, config }) => }); test('form is an element reference', async ({ page }) => { + const logs: string[] = []; + + page.on('console', (msg) => { + if (msg.type() === 'warning') { + logs.push(msg.text()); + } + }); + await page.setContent( ` Submit @@ -90,14 +172,6 @@ configs({ directions: ['ltr'], modes: ['ios'] }).forEach(({ title, config }) => config ); - const logs: string[] = []; - - page.on('console', (msg) => { - logs.push(msg.text()); - }); - - await page.click('ion-button'); - expect(logs.length).toBe(1); expect(logs[0]).toContain( '[Ionic Warning]: The provided "form" element is invalid. Verify that the form is a HTMLFormElement and rendered in the DOM.'