bvaughn/angular-form-for

Custom value of empty option for select

Closed this issue · 16 comments

Hello again. Fixes you did for select are working really great. However, I realised there is another issue related to select.

As we talked in #110 there were some issues with treating falsy values. When object attribute is a null it is converted to undefined. Empty option & placeholder works fine.

But as I pointed in #110, I need to nullify object attribute with null or empty string. Because when I select blank option the object atrribute is set to undefined and this undefined attribute is not sent via my request. And since undefined attribute is not present in params, my backend does not know it needs to be set to null.

You wrote that empty string will solve my problem. It did before your changes, but this solution no longer works. When I change $scope.emptyOption[$scope.valueAttribute] to null or empty string then empty option is duplicated.

Sorry to hear that this issue is still giving you trouble, Palo. It's kind of a difficult edge case to handle.

Because when I select blank option the object atrribute is set to undefined and this undefined attribute is not sent via my request. And since undefined attribute is not present in params, my backend does not know it needs to be set to null.

Why is the undefined value not being sent with your params? Is there a simpler solution to this (just updating your application code) than changing selectField?

I tried to fix it using interceptors but it is more complicated then it looks like.

First option using interceptors - if attribute is undefined, I have to convert it to null value or empty string. But I do not want to do this for each attribute of object (this will cause lot of other issues in backend part).

Second option is manually convert each attribute (only for selects) but this will be pain in the ass.

I'm using https://github.com/FineLinePrototyping/angularjs-rails-resource plugin to handle API calls and this is how it works. Basically it is good enough, because as I said I do not want to send each undefined attribute via request. But I want to send undefined/null/empty string values from selects...

I understand this "issue" is edge case, but I think lot of people can find this useful. I tried to work on this, but my JS knowledge is not good enough to fix something like this.

If you'll find some spare time to work on this - I'll be really grateful! But if you won't have time - it's okay, no hard feelings :-).

Ah, I understand a little better now. Thanks for elaborating.

I'll leave the issue open then and try to get to it as soon as I can.
Angular's ngSelect/ngOptions are quickly becoming my least favorite
components to work with though. I wish I could opt-out of using them and
just manage this portion of the DOM myself... :)

On Monday, July 13, 2015, Palo Delinčák notifications@github.com wrote:

I tried to fix it using interceptors but it is more complicated then it
looks like.

First option using interceptors - if attribute is undefined, I have to
convert it to null value or empty string. But I do not want to do this for
each attribute of object (this will cause lot of other issues in backend
part).

Second option is manually convert each attribute (only for selects) but
this will be pain in the ass.

I'm using https://github.com/FineLinePrototyping/angularjs-rails-resource
plugin to handle API calls and this is how it works. Basically it is good
enough, because as I said I do not want to send each undefined attribute
via request. But I want to send undefined/null/empty string values from
selects...

I understand this "issue" is edge case, but I think lot of people can find
this useful. I tried to work on this, but my JS knowledge is not good
enough to fix something like this.

If you'll find some spare time to work on this - I'll be really grateful!
But if you won't have time - it's okay, no hard feelings :-).


Reply to this email directly or view it on GitHub
#141 (comment)
.

Oh, great! Yes, I know I'm little bit annoying with my issues... I'm sorry about that and I definitely owe you some beers! So when I'm back in SF, I'll bring you some beers from Europe 🍺!

European beer sounds good ;) thanks

On Monday, July 13, 2015, Palo Delinčák notifications@github.com wrote:

Oh, great! Yes, I know I'm little bit annoying with my issues... I'm sorry
about that and I definitely owe you some beers! So when I'm back in SF,
I'll bring you some beers from Europe [image: 🍺]!


Reply to this email directly or view it on GitHub
#141 (comment)
.

Hello Brian, any news on this?

Hiya. No news. Haven't forgotten about this issue but haven't had the bandwidth to work on it yet. Sorry mate, things have been busy.

Ah, good news. I have an idea for a way to approach this that I think should work. Hopefully I'll be able to get to it this evening or tomorrow.

I wonder if you would be willing to test this for me using either the diff below or this updated plunker.

diff --git a/dist/form-for.js b/dist/form-for.js
index 9c004d8..394bbd8 100644
--- a/dist/form-for.js
+++ b/dist/form-for.js
@@ -1636,7 +1636,8 @@ var formFor;
                 // Otherwise the Angular select directive will generate an additional empty <option> ~ see #110
                 // Angular 1.2.x-1.3.x may generate an empty <option> regardless, unless the non-selection is undefined.
                 if ($scope.model.bindable === null || $scope.model.bindable === undefined || $scope.model.bindable === '') {
-                    $scope.model.bindable = undefined;
+                    //$scope.model.bindable = undefined;
+                    $scope.emptyOption[$scope.valueAttribute] = $scope.model.bindable;
                 }
                 $scope.bindableOptions.splice(0);
                 if (!$scope.model.bindable || $scope.allowBlank) {
diff --git a/dist/form-for.min.js b/dist/form-for.min.js
index c3c127c..6d187d9 100644
--- a/dist/form-for.min.js
+++ b/dist/form-for.min.js
@@ -1,3 +1,4 @@
-angular.module("formFor",[]);var formFor;!function(e){var t=function(){function e(){this.restrict="A"}return e.prototype.link=function(e,
-e[e.COLLECTION_MAX_SIZE="COLLECTION_MAX_SIZE"]="COLLECTION_MAX_SIZE",e[e.COLLECTION_MIN_SIZE="COLLECTION_MIN_SIZE"]="COLLECTION_MIN_SIZE"
+angular.module("formFor",[]);var formFor;!function(e){var t=function(){function e(){this.restrict="A"}return e.prototype.link=function(e,
+
+!function(e){!function(e){e[e.COLLECTION_MAX_SIZE="COLLECTION_MAX_SIZE"]="COLLECTION_MAX_SIZE",e[e.COLLECTION_MIN_SIZE="COLLECTION_MIN_SI
 //# sourceMappingURL=form-for.min.js.map
\ No newline at end of file
diff --git a/dist/form-for.min.js.map b/dist/form-for.min.js.map
index b619d54..376d1ce 100644
--- a/dist/form-for.min.js.map
+++ b/dist/form-for.min.js.map
@@ -1 +1 @@
-{"version":3,"file":"form-for.min.js","sources":["form-for.js"],"names":["angular","module","formFor","AriaManager","this","restrict","pr
\ No newline at end of file
+{"version":3,"file":"form-for.min.js","sources":["form-for.js"],"names":["angular","module","formFor","AriaManager","this","restrict","pr
\ No newline at end of file
diff --git a/source/directives/select-field.ts b/source/directives/select-field.ts
index c7c2079..20f2e42 100644
--- a/source/directives/select-field.ts
+++ b/source/directives/select-field.ts
@@ -295,7 +295,8 @@ module formFor {
         if ($scope.model.bindable === null ||
             $scope.model.bindable === undefined ||
             $scope.model.bindable === '') {
-          $scope.model.bindable = undefined;
+          //$scope.model.bindable = undefined;
+          $scope.emptyOption[$scope.valueAttribute] = $scope.model.bindable;
         }

         $scope.bindableOptions.splice(0);

Note that this fix requires Angular 1.4.x. (1.3.x had some issues and regressions with select menus that I'm not interested in patching around.)

I'm feeling pretty good about this change so I'm going to move forward with it. Reach out to me with any questions or concerns.

The fix for this issue has been released in version 4.1.1.

Please remember that Angular 1.4.x is strongly suggested to avoid select field issues.

Hey, thanks for your time here!

I'm not sure if I understand how it works. Because when you change (in your Plunker) 6) from value 4 to empty value - it becomes undefined. And it should become null or empty string as I wrote in comments above.

Should I modify $scope.emptyOption[$scope.valueAttribute] and change it to null?

And also when I change 7) to some option and then to empty value it becomes empty string and not undefined as in other cases.

This change sets the value of the empty option to be equal to the
initially-selected value (if that value is falsy). This seemed to be the
only easy way to satisfy your feature request and work within
ngSelect/ngOption limitations.

As I understood your initial request, this should enable you to use null or
empty string.

Does this not work for you? And if not, why?

On Monday, August 3, 2015, Palo Delinčák notifications@github.com wrote:

Hey, thanks for your time here!

I'm not sure if I understand how it works. Because when you change (in
your Plunker) 6) from value 4 to empty value - it becomes undefined. And
it should become null or empty string as I wrote in comments above.

Should I modify $scope.emptyOption[$scope.valueAttribute] and change it
to null?

And also when I change 7) to some option and then to empty value it
becomes empty string and not undefined as in other cases.


Reply to this email directly or view it on GitHub
#141 (comment)
.

Unfortunately, it does not work for me. For example 6) from your Plunker - it has preselected value 4 and when I change it to empty option - then I need to update ng-model with null/empty string and not with undefined.

And why? As I wrote earlier - undefined values are not sent in my requests and without this I can't nullify attribute in my backend.

I see. I'm sorry that this solution doesn't work for you. I guess I misunderstood what you were initially saying. I don't have a lot of extra time these days to devote to formFor and it's easy to lose context of more subtle issues like this when I'm only checking in every week or so.

I'll re-open this issue but I'm not going to be able to commit to fixing it by any specific time. I'd welcome contributions (from you or someone else) though if you'd be willing to take a shot at it. :)

Release 4.1.3 is available with your fix. Thanks again for your contribution! :)