ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.9k stars 13.52k forks source link

Implicit form submit doesn't work in Ionic 4 #15682

Closed simonhaenisch closed 5 years ago

simonhaenisch commented 5 years ago

Bug Report

@ionic/core@4.0.0-beta.11 @stencil/core@0.12.4

Describe the Bug

I created a login form (started with the Ionic Stencil PWA Starter) using form, ion-input and ion-button. When I try to implicitly submit the form by pressing enter in one of the inputs, the form does not get submitted, despite my button having the type="submit" attribute set.

Steps to Reproduce

  1. Create a form
  2. Focus on one of the inputs
  3. Press enter
  4. Nothing happens

Related Code

Fiddle: http://jsfiddle.net/ok8zn5e6/2/

<ion-list>
  <form onSubmit={e => this.submitLogin(e)}>
    <ion-item>
      <ion-label position="stacked">
        Email
      </ion-label>
      <ion-input name="username" type="email" placeholder="you@domain.com" required autofocus />
    </ion-item>
    <ion-item>
      <ion-label position="stacked">
        Password
      </ion-label>
      <ion-input name="password" type="password" placeholder="********" required />
    </ion-item>

    <ion-button type="submit" color="primary">
      <ion-icon slot="end" name="log-in" />
      Log In
    </ion-button>
  </form>
</ion-list>

Expected Behavior

The implicit form submit should work because the form contains a button with type="submit".

Additional Context

http://stonefishy.github.io/blog/2015/06/30/implicit-submission-of-form-when-pressing-enter-key/

simonhaenisch commented 5 years ago

As a temporary workaround this kind of works (with <form ref={form => (this.form = form)}> in the template):

form: HTMLFormElement;

@Listen('keydown.enter')
handleEnterKey(e: KeyboardEvent) {
  if ((e.target as HTMLIonInputElement).name === 'password') {
    (this.form
      .querySelector('ion-button[type="submit"]')
      .shadowRoot.querySelector('button[type="submit"]') as HTMLButtonElement).click();
  }
}

Calling this.form.submit() doesn't work because it doesn't trigger the submit event. Calling .click() on the ion-button doesn't actually click the underlying native button.

As a side note, the browser's built-in form validation (e. g. required, type="email") does not work either when using ion-input instead of just input. Also, when setting text-center on the input, the input jumps around when focussed. This just makes me want to create my own input and button components 🤭.

FdezRomero commented 5 years ago

I was about to open this same issue. I traced the problem down to the onClick method of ion-button here: https://github.com/ionic-team/ionic/blob/master/core/src/components/button/button.tsx#L128:L144

It seems that the fake button technique isn't working as expected in Chrome and Safari. It works on Firefox though. The implicit submit was working on previous betas before turning on the Shadow DOM, so it may be related.

Replacing the ion-button on a form with a regular button works as expected.

simonhaenisch commented 5 years ago

@FdezRomero thanks for the additional info, so it might be that the inputs lose the form context because each input lives in a separate Shadow DOM? Just did a search for it and found this: https://github.com/w3c/webcomponents/issues/649.

I think I had tried using a regular button but I only got it to work when also replacing the ion-inputs with regular inputs.

Is there a way of turning off Shadow DOM for a component including all it's child elements? I tried the shadow: false option on the Component decorator, but it didn't work (I'm assuming it only turns it off for the host element). Or is there an option to opt out of Shadow DOM?

FdezRomero commented 5 years ago

@simonhaenisch I mentioned the change to Shadow DOM not as the cause, but as a point in time where it started happening in case it helps.

If you look at the code I linked before, the onClick method creates a hidden button of the same type, appends it to the parent form in Light DOM and clicks it, which triggers the form submit event. This should enable the implicit submission, but it doesn't.

Maybe @manucorporat can give us more info to submit a PR :wink:

Edit: Now that I think of it, if the hidden button is only created and removed when the ion-button is clicked, it wouldn't work for the implicit submission.

FdezRomero commented 5 years ago

So I was talking with Mike from Ionic and learnt that the implicit form submission only works with both native inputs and a native submit input/button, as they have the necessary internal JS in the browser for it to work.

Since these native elements are now placed inside the Shadow DOM of the custom elements, this platform feature won't work. The behavior can be replicated in Angular by using the keyup.enter event:

<form [formGroup]="myForm" (ngSubmit)="submit()" (keyup.enter)="submit()">
mhartington commented 5 years ago

So there's a few things going on here.

1) Native inputs, submit inputs, and browser built-ins

So vanilla use-case where things work

<form onsubmit="checkData(event)">
  <input type="text" required>
  <input type="password" required>
  <input type="submit">
</form>

In this case, the form and inputs will submit on enter because of the built in HTML FormValidation/handling. This is something built into the browser, just as long as the form is valid and there is a valid handler for onsubmit

2) Ionic inputs and ionic button

Use case where things do not work

<form onsubmit="checkData(event)" novalidate>
  <ion-input type="email" required></ion-input>
  <ion-input type="password" required></ion-input>
  <ion-button type="submit" color="primary">
    Log In
  </ion-button>
</form>

Why this does not work is a mixture of Custom Elements and Shadow DOM encapsulation. Since the native inputs in ion-input are encapsulated with shadow DOM, the built in browser form validation will not be able to work here. Also, because the ion-input essentially extend a base div element, there's no built in behavior for it to follow.

Why did this work in early V4 release and V3?

This worked in earlier release and in V3 because we did not use shadow DOM, so the native inputs were reachable by the browser own built-in functionality.

Ways to deal with this: In a framework landscape, like Angular, you'd have a keyup handler to list for enter. In vanilla JS, you'd have to wire that logic up yourself.

simonhaenisch commented 5 years ago

One more thing that I just remember I was actually trying to get to work is to enable the proper buttons on the mobile keyboard (i. e. the "next" button to jump to the next input on iOS). So in order to get that to work I will probably need to write my own form component, because hooking up some key event listeners won't fix that...

Edit: same thing for required on an input... because the inputs are not properly connected to the form, I can click the submit button just fine and the browser will submit the form even though my required fields are still empty.

simonhaenisch commented 5 years ago

@mhartington would it be possible to expose the shadow option of the ion-input component decorator like:

<ion-input shadow={false}></ion-input>

I know it's not super trivial, but I really don't see a point in enabling Shadow DOM for this if there is so many built-in benefits being lost. Or as an alternative, add a shadow option to setupConfig(config: IonicConfig) to globally opt out of Shadow DOM.

Or maybe a better way to go is to add a ion-form component that encapsulates all inputs within the form's Shadow DOM but doesn't have seperate Shadow DOMs for each input?

m0nzderr commented 5 years ago

+1, As pointed out by @simonhaenisch , the native keyboard behavior is totally different, so a framework-based workaround is needed in order to save the UX with forms.

laurentperroteau commented 5 years ago

@FdezRomero + @simonhaenisch solutions solve the problem

jonmikelm commented 5 years ago

Related to this problem, our designers have defined some screens where the submit ion-button of the form is situated in the header, outside the form tag.

<ion-header>
    <ion-toolbar>
        <ion-title>Title</ion-title>
        <ion-buttons slot="end">
            <ion-button icon-only (click)="triggerSumbit()">
                <ion-icon size="large" name="search"></ion-icon>
            </ion-button>
        </ion-buttons>
    </ion-toolbar>
</ion-header>

To make this ion-button in the header act as a type="sumbit" button, we have created a not-visible button inside the form tag.

<ion-content>
  <form [formGroup]="searchForm" (ngSubmit)="search()" novalidate>
    ...
    <button #hiddenButton type="submit" class="hidden"></button>
  </form>
</ion-content>

Then, on the click event of the header ion-button we trigger a click in the hidden submit button to run the ngSubmit function. It's not a really ellegant workaround.

@ViewChild('hiddenButton') hiddenButton;
...
triggerSubmit() {
  this.hiddenButton.nativeElement.click();
}

HTML button spec provides a form attribute. You can set an id to the form tag and pass it to the form attribute button outside a form tag. This way, clicking the button outside the form, works as if the button was inside the form tag.

It would be nice ion-button to support a form attribute, the same way as the native HTML button:

<ion-header>
    <ion-toolbar>
        <ion-title>Title</ion-title>
        <ion-buttons slot="end">
            <ion-button icon-only type="sumbit" form="searchForm">
                <ion-icon size="large" name="search"></ion-icon>
            </ion-button>
        </ion-buttons>
    </ion-toolbar>
</ion-header>
<ion-content>
  <form id="searchForm" [formGroup]="searchForm" (ngSubmit)="search()" novalidate>
    ...
  </form>
</ion-content>
paulstelzer commented 5 years ago

I close this because as mentioned it has to do with shadow dom. Just create a method for it and use (keyup.enter) in Angular

ajcrites commented 5 years ago

I think that a more permanent solution may be in order -- this is standard / expected behavior. This would require implementing a (keyup.enter) on every form input that performed the form submission and dismissed the keyboard as necessary. You would also have to duplicate the form submission handling for (ngSubmit) because users could still tap the submit button itself.

simonhaenisch commented 5 years ago

@paulstelzer I would also want this to be reopened, and might be possible to solve with an ion-form component? This breaks more things than just the enter key, e. g. keyboard "next" navigation on mobile.

paulstelzer commented 5 years ago

@simonhaenisch Thanks for your response. I will not open this issue because we already have two which seperate this issues very well:

15136: Button does not trigger ngSubmit

16498: Prev / Next button on mobile keyboard

So it's not useful to have different issues opened for the same topic. Is that okay for you :)

ionitron-bot[bot] commented 5 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.