Skip to content

Commit

Permalink
fix(button): hidden button is added when form is set async (#27955)
Browse files Browse the repository at this point in the history
Issue number: resolves #27952

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

The hidden button in `ion-button` that is responsible for submitting the
form does not get added when the `form` property is set async.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- `ion-button` now checks to see if it needs to render a hidden button
whenever it re-renders. This allows it to account for changes to the
`type` property, `form` property, etc.

Since this code can potentially run multiple times I added an extra
check so we don't add multiple buttons to the form.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.2.3-dev.11691523847.1760ab58`
  • Loading branch information
liamdebeasi committed Aug 9, 2023
1 parent 07dee74 commit e9fa300
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 12 deletions.
37 changes: 25 additions & 12 deletions core/src/components/button/button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,19 +153,27 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf
*/
@Event() ionBlur!: EventEmitter<void>;

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);
private renderHiddenButton() {
const formEl = (this.formEl = this.findForm());
if (formEl) {
const { formButtonEl } = this;

/**
* If the form already has a rendered form button
* then do not append a new one again.
*/
if (formButtonEl !== null && formEl.contains(formButtonEl)) {
return;
}

// Create a hidden native button inside of the form
const newFormButtonEl = (this.formButtonEl = document.createElement('button'));
newFormButtonEl.type = this.type;
newFormButtonEl.style.display = 'none';
// Only submit if the button is not disabled.
newFormButtonEl.disabled = this.disabled;

formEl.appendChild(newFormButtonEl);
}
}

Expand Down Expand Up @@ -314,6 +322,11 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf
if (fill == null) {
fill = this.inToolbar || this.inListHeader ? 'clear' : 'solid';
}

{
type !== 'button' && this.renderHiddenButton();
}

return (
<Host
onClick={this.handleClick}
Expand Down
23 changes: 23 additions & 0 deletions core/src/components/button/test/form-reference/button.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,29 @@ configs({ directions: ['ltr'], modes: ['ios'] }).forEach(({ title, config }) =>

expect(submitEvent).not.toHaveReceivedEvent();
});

test('should submit the form by id when form is set async', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/27952',
});
await page.setContent(
`
<form id="myForm"></form>
<ion-button type="submit">Submit</ion-button>
`,
config
);

const submitEvent = await page.spyOnEvent('submit');
const button = page.locator('ion-button');

await button.evaluate((el: HTMLIonButtonElement) => (el.form = 'myForm'));

await page.click('ion-button');

expect(submitEvent).toHaveReceivedEvent();
});
});

test.describe(title('should throw a warning if the form cannot be found'), () => {
Expand Down
31 changes: 31 additions & 0 deletions core/src/components/button/test/form-reference/button.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { newSpecPage } from '@stencil/core/testing';
import { Button } from '../../button';

describe('Button: Hidden Form Button', () => {
it('should not add multiple buttons to the form', async () => {
const page = await newSpecPage({
components: [Button],
html: `
<form id="my-form"></form>
<ion-button form="my-form" type="submit">Submit</ion-button>
`,
});

const getButtons = () => {
return page.body.querySelectorAll('form button');
};

const form = page.body.querySelectorAll('form');
const button = page.body.querySelector('ion-button');

await page.waitForChanges();

expect(getButtons().length).toEqual(1);

// Re-render the component
button.color = 'danger';
await page.waitForChanges();

expect(getButtons().length).toEqual(1);
});
});

0 comments on commit e9fa300

Please sign in to comment.