Skip to content

Commit

Permalink
fix(forms): don't prevent default behavior for forms with method="dia…
Browse files Browse the repository at this point in the history
…log"

The forms `submit` event handlers have a `return false` to prevent form submissions from reloading the page, however this also prevents the browser behavior for forms with `method="dialog"`.

These changes add an exception since the `method="dialog"` doesn't refresh the page.

Fixes #47150.
  • Loading branch information
crisbeto committed Sep 7, 2022
1 parent bea953a commit ef50952
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 7 deletions.
4 changes: 3 additions & 1 deletion packages/forms/src/directives/ng_form.ts
Expand Up @@ -298,7 +298,9 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
(this as {submitted: boolean}).submitted = true;
syncPendingControls(this.form, this._directives);
this.ngSubmit.emit($event);
return false;
// Forms with `method="dialog"` have some special behavior
// that won't reload the page and that shouldn't be prevented.
return ($event?.target as HTMLFormElement | null)?.method === 'dialog';
}

/**
Expand Down
Expand Up @@ -94,8 +94,8 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
@Output() ngSubmit = new EventEmitter();

constructor(
@Optional() @Self() @Inject(NG_VALIDATORS) private validators: (Validator|ValidatorFn)[],
@Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) private asyncValidators:
@Optional() @Self() @Inject(NG_VALIDATORS) validators: (Validator|ValidatorFn)[],
@Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) asyncValidators:
(AsyncValidator|AsyncValidatorFn)[]) {
super();
this._setValidators(validators);
Expand Down Expand Up @@ -271,7 +271,9 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
(this as {submitted: boolean}).submitted = true;
syncPendingControls(this.form, this.directives);
this.ngSubmit.emit($event);
return false;
// Forms with `method="dialog"` have some special behavior
// that won't reload the page and that shouldn't be prevented.
return ($event?.target as HTMLFormElement | null)?.method === 'dialog';
}

/**
Expand Down
30 changes: 29 additions & 1 deletion packages/forms/test/reactive_integration_spec.ts
Expand Up @@ -7,7 +7,7 @@
*/

import {ɵgetDOM as getDOM} from '@angular/common';
import {Component, Directive, forwardRef, Input, NgModule, OnDestroy, Type} from '@angular/core';
import {Component, Directive, ElementRef, forwardRef, Input, NgModule, OnDestroy, Type, ViewChild} from '@angular/core';
import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
import {AbstractControl, AsyncValidator, AsyncValidatorFn, COMPOSITION_BUFFER_MODE, ControlValueAccessor, DefaultValueAccessor, FormArray, FormBuilder, FormControl, FormControlDirective, FormControlName, FormGroup, FormGroupDirective, FormsModule, MaxValidator, MinLengthValidator, MinValidator, NG_ASYNC_VALIDATORS, NG_VALIDATORS, NG_VALUE_ACCESSOR, ReactiveFormsModule, Validator, Validators} from '@angular/forms';
import {By} from '@angular/platform-browser/src/dom/debug/by';
Expand Down Expand Up @@ -2114,6 +2114,20 @@ const ValueAccessorB = createControlValueAccessor('[cva-b]');
expect(passwordControl.value).toEqual('Carson', 'Expected value to change on submit.');
expect(passwordControl.valid).toBe(true, 'Expected validation to run on submit.');
});

it('should not prevent the default action on forms with method="dialog"', fakeAsync(() => {
if (typeof HTMLDialogElement === 'undefined') {
return;
}

const fixture = initTest(NativeDialogForm);
fixture.detectChanges();
tick();
const event = dispatchEvent(fixture.componentInstance.form.nativeElement, 'submit');
fixture.detectChanges();

expect(event.defaultPrevented).toBe(false);
}));
});
});

Expand Down Expand Up @@ -5437,3 +5451,17 @@ class MinMaxFormControlComp {
min: number|string = 1;
max: number|string = 10;
}

@Component({
template: `
<dialog open>
<form #form method="dialog" [formGroup]="formGroup">
<button>Submit</button>
</form>
</dialog>
`
})
class NativeDialogForm {
@ViewChild('form') form!: ElementRef<HTMLFormElement>;
formGroup = new FormGroup({});
}
31 changes: 30 additions & 1 deletion packages/forms/test/template_integration_spec.ts
Expand Up @@ -7,7 +7,7 @@
*/

import {CommonModule, ɵgetDOM as getDOM} from '@angular/common';
import {Component, Directive, forwardRef, Input, Type, ViewChild} from '@angular/core';
import {Component, Directive, ElementRef, forwardRef, Input, Type, ViewChild} from '@angular/core';
import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing';
import {AbstractControl, AsyncValidator, COMPOSITION_BUFFER_MODE, ControlValueAccessor, FormControl, FormsModule, MaxLengthValidator, MaxValidator, MinLengthValidator, MinValidator, NG_ASYNC_VALIDATORS, NG_VALIDATORS, NG_VALUE_ACCESSOR, NgForm, NgModel, Validator} from '@angular/forms';
import {By} from '@angular/platform-browser/src/dom/debug/by';
Expand Down Expand Up @@ -1062,6 +1062,21 @@ import {NgModelCustomComp, NgModelCustomWrapper} from './value_accessor_integrat
['fired', 'fired'],
'Expected ngModelChanges to fire again on submit if value changed.');
}));


it('should not prevent the default action on forms with method="dialog"', fakeAsync(() => {
if (typeof HTMLDialogElement === 'undefined') {
return;
}

const fixture = initTest(NativeDialogForm);
fixture.detectChanges();
tick();
const event = dispatchEvent(fixture.componentInstance.form.nativeElement, 'submit');
fixture.detectChanges();

expect(event.defaultPrevented).toBe(false);
}));
});

describe('ngFormOptions', () => {
Expand Down Expand Up @@ -2932,3 +2947,17 @@ class NgModelNoMinMaxValidator {
max!: number;
@ViewChild('myDir') myDir: any;
}

@Component({
selector: 'ng-model-nested',
template: `
<dialog open>
<form #form method="dialog">
<button>Submit</button>
</form>
</dialog>
`
})
class NativeDialogForm {
@ViewChild('form') form!: ElementRef<HTMLFormElement>;
}
3 changes: 2 additions & 1 deletion packages/platform-browser/testing/src/browser_util.ts
Expand Up @@ -97,10 +97,11 @@ export class BrowserDetection {

export const browserDetection: BrowserDetection = BrowserDetection.setup();

export function dispatchEvent(element: any, eventType: any): void {
export function dispatchEvent(element: any, eventType: any): Event {
const evt: Event = getDOM().getDefaultDocument().createEvent('Event');
evt.initEvent(eventType, true, true);
getDOM().dispatchEvent(element, evt);
return evt;
}

export function createMouseEvent(eventType: string): MouseEvent {
Expand Down

0 comments on commit ef50952

Please sign in to comment.