Components: mat-dialog-close should close the dialog with undefined value, not empty string by default

[Maximaximum]

Bug, feature request, or proposal:

If a button has an mat-dialog-close attribute with no value, then clicking the button should resolve MatDialogRef.afterClosed() observable with an undefined value, not an empty string ("").

What is the expected behavior?

If a button has an mat-dialog-close attribute with no value, then clicking the button should resolve MatDialogRef.afterClosed() Observable with an undefined value.

What is the current behavior?

Currently, if a button has an mat-dialog-close attribute with no value, then clicking the button resolves MatDialogRef.afterClosed() Observable with an empty string ("").

What are the steps to reproduce?

StackBlitz: https://stackblitz.com/edit/angular-qijqgu
Please see line 25 of dialog-overview-example.ts

What is the use-case or motivation for changing an existing behavior?

Empty string does not have any special meaning in terms of Javascript. An undefined value is a much more appropriate value for closing a dialog without an actual value.

Checking if the dialog result equals to an empty string seems to be weird, especially when the expected value is a number (or any other type expect string).

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Angular 5.1.1, Material 5.0.0-rc.3, any OS and browser

[crisbeto]

The issue with doing this is that we can't really know whether the empty string is because the user didn't pass anything in to the attribute or whether they actually wanted to close it with an empty string.

[Maximaximum]

Aha, I see. No matter if it's just <button mat-close-dialog> or <button mat-close-dialog=""> or <button [mat-close-dialog]="emptyStringProperty">, a directive operates with an empty string value.

It appears that this issue can be overcome by explicitly specifying undfined value: <button [mat-dialog-close]="undefined">.

However, this does not seem to be a very clean solution, taken into account that this fix actually has to be applied to any dialogs used throughout the application. It also adds an unneeded overhead to the change detection loop.

Why not add an additional mat-dialog-cancel directive then? It would operate similarly to 'mat-dialog-close', but always resolve the observable with undefined. It seems to be a nice and clean way to overcome this issue.

[crisbeto]

I'm not sure that it's a nice way to handle it. For one, it makes the API more confusing since now you have two directives that do more or less the same. It also means that we'd have to maintain both of those directives. Closing this one for now, but it can be reopened if a better solution comes up.

[julianobrasil]

I hadn't noticed too. After you open this issue I checked one of my projects and it was asking for data
in the server unecessarily (I was checking for null return after the dialog was closed). Fortunately it's very easy to adapt the code.

@crisbeto, maybe it could have some note about it in the docs

[meurig]

Why can't [mat-dialog-close]="undefined" be the default behaviour?

It works when you state it explicitly, so I don't see how this can be a technical limitation.

Using an empty string to mean 'nothing' just seems wrong.

[seba161189]

Try to save the initial value when the dialog is open so you can return if you press close.
COMPONENT
form: FormGroup;
initSalary: number; //in close return this one
constructor(
private formBuilder: FormBuilder,
public dialogRef: MatDialogRef,
@Inject(MAT_DIALOG_DATA) private data
) {}

ngOnInit() {
this.form = this.formBuilder.group({
salary: this.data ? this.data.salary : ''
});

this.initSalary = this.form.value.salary;

}

submit(form) {
this.dialogRef.close(${form.value.salary});
}

close() {
this.dialogRef.close(this.initSalary);
}
}

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{_This action has been performed automatically by a bot._}