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(forms): don't prevent default behavior for forms with method="dialog" #47308

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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,10 @@ 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. Note that we need to null check the `event` and the `target`, because
// some internal apps call this method directly with the wrong arguments.
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