Binding `undefined` should not change default values

[angelaki]

When binding e.g. aspectRatio to the image cropper, an "undefined" gets treaded as false rather than the default value (true). This is because every settings get set hard in a loop (https://github.com/Mawi137/ngx-image-cropper/blob/9f26c8a1b06e6aa4dcdcd79c3faa519f3e406500/projects/ngx-image-cropper/src/lib/interfaces/cropper.settings.ts#L45).

Would you like (me) to make undefined set the default value and only null set the value? Or does this feel like a breaking change to you? Right now I must repeat your default values in my control reflecting your properties. I cannot use [maintainAspectRatio]="maintainAspectRatio" without setting it to true initially.

[Mawi137]

Isn't that how @Inputs always work in Angular? When you add [aspectRatio]="undefined" it will pass the undefined as far as I know. Unlike functions where you can indeed specify default values for your arguments.

[angelaki]

Not quite sure, I can check e.g. Angular Material these days. But I guess it totally depends on the use-case. I'd expect null to be set, while undefined should fall be to the default value.

[Mawi137]

If it is, then it's probably the lib that chose to explicitly set a default value. In the image cropper the value you pass to the @Input is used, which is the normal behavior. So it's up to you to pass either true or false. undefined will indeed be interpreted as false, because JavaScript.

[angelaki]

In my case I need to wrap your control and set all the required properties via bindings. Now I'd need to set their inital value to your default value rather than undefined. Material's behavior differs here.

Grüße

Tristan Sprößer

---

From: Martijn @.> Sent: Thursday, July 21, 2022 12:33:00 AM To: Mawi137/ngx-image-cropper @.> Cc: Tristan Sprößer @.>; Author @.> Subject: Re: [Mawi137/ngx-image-cropper] Binding undefined should not change default values (Issue #512)

If it is, then it's probably the lib that chose to explicitly set a default value. In the image cropper the value you pass to the @inputhttps://github.com/input is used, which is the normal behavior. So it's up to you to pass either true or false. undefined will indeed be interpreted as false, because JavaScript.

— Reply to this email directly, view it on GitHubhttps://github.com/Mawi137/ngx-image-cropper/issues/512#issuecomment-1190828288, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AJGSUBLVC5XAC7EB4HG4H3LVVB5BZANCNFSM53V26K2A. You are receiving this because you authored the thread.Message ID: @.***>

[Mawi137]

Can you give an example of how Angular Material handles it?

[angelaki]

E.g. selects value is only set, if it is not false: https://github.com/angular/components/blob/main/src/material/select/select.ts#L408-L421. You could check if the value is number. But since you're handling your property setters dynamically imho assuming unassigned gets treated differently than null would be a good idea.

[Mawi137]

Good point, now passing undefined works correctly because of JavaScript, but it would indeed be better if default values would be set if null or undefined is passed in the Input. Feel free to open up a PR. I think the CropperSettings would be the best place to implement it.

[angelaki]

Sorry I don't find the time for it right now - I've got it on my backlog 😉

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.