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

Enhance the select field with null value support #294

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

ulion
Copy link
Contributor

@ulion ulion commented Mar 11, 2015

Angular's select with ngOptions has 3 type about the null value (the value="" options). reference: https://docs.angularjs.org/api/ng/directive/ngOptions

  1. current angular-schema-form default implement, which will add a empty value options which result null by default if the model value does not match any enum/titleMap value and be selected by default. but once user selected any non-empty value, that empty value option is gone, which make the user have to make a choice but without a default selection if the field is also required.
  2. if put an option value="" tag in the select tag, the empty option will be there like the previous type, except that the empty option won't get disappeared after user selected other option, so it make the null value always an option.
  3. if put an invisible option value="", even with ng-if="false", it will never populate an empty option, the select will have the first option selected if the model value does not match any option value.

So, above 3 type of use cases are all useful, but current angular-schema-form implement only give use the first use case support. This PR will add the later 2 type of select/ng-options support, so we can do with the null value better in many use cases.

e.g.

  • for a select field, the null value is a valid option, and we want it always selectable, then we should make the schema type be ['null', 'string'], and use any following methods:
    1. schema use enum, without titleMap. Then should put null as one of the possible value in the enum (if use enum schema, because tv4 will validate the value to the enum value list, so if use enum, null should be in the enum), this will make the null value always selectable. if want non-blank label for null value option, set form's allowAutoNullOption property for the select field to the label text you want.
    2. schema use enum, with titleMap. Same as above, except the code will auto detect the titleMap (array type titleMap only) with the null value, if found, will auto use the name found as the null value option label (code internally auto setup allowAutoNullOption to the label), while you can still override it by set allowAutoNullOption to value you want.
    3. schema not use enum, with titleMap (array type titleMap only). same as above, code will detect the null value label and auto setup allowAutoNullOption to the label text.
  • some times, you do not want a default empty option, you want the first option be selected by default, it can also be done by this PR, just set the form property allowAutoNullOption of the select field to false will do that.
  • There are also some rare cased, you want null value but not be the first option, for such use case, this PR can still do that, by set allowAutoNullOption to false, and include the null value in enum or your titleMap at the prefered position will simply do it.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.23%) to 80.17% when pulling 1dfa6c3 on ulion:enhance_select_null_support into 446d846 on Textalk:development.

@davidlgj
Copy link
Contributor

Hi @ulion,

thanks for the PR! This is a sorely needed feature indeed. I'm in the process of merging it, and I created an example page for all of the versions you describe above (The branch is here, don't forget to gulp if you try it https://github.com/Textalk/angular-schema-form/tree/ulion-enhance_select_null_support)

It looks nice, but there is one scenario that I'm not sure about:

some times, you do not want a default empty option, you want the first option be selected by
default, it can also be done by this PR, just set the form property allowAutoNullOption of the select
field to false will do that.

When I try it does what it says and selects the first option, but it doesn't change the model, I guess using a default in the schema makes more sense so I don't think this needs to be fixed, just documented.

@ulion
Copy link
Contributor Author

ulion commented Mar 11, 2015

ops, yes, in many scene, the form does not update back to model, but maybe
it's required, I don't know how to let the form force update to model, do
you?

e.g. in some use case, I have text input, but the initial value is null,
when submit form, it may fail the validation. but imo the form should
override the initial value with it's "current value" which should be. but I
just don't know how to let form update to model. if you know simple call
can to that, please tell me. thx.

besides, about the default behaviour of this PR, I'm a little thought, which
break the titleMap's array order if null value is already there. maybe
better way is to not break the order, internally make it auto set
allowAutoNullOption to false? what do you think?

2015-03-12 5:50 GMT+08:00 David Jensen notifications@github.com:

Hi @ulion https://github.com/ulion,

thanks for the PR! This is a sorely needed feature indeed. I'm in the
process of merging it, and I created an example page for all of the
versions you describe above (The branch is here, don't forget to gulp if
you try it
https://github.com/Textalk/angular-schema-form/tree/ulion-enhance_select_null_support
)

It looks nice, but there is one scenario that I'm not sure about:

some times, you do not want a default empty option, you want the first
option be selected by
default, it can also be done by this PR, just set the form property
allowAutoNullOption of the select
field to false will do that.

When I try it does what it says and selects the first option, but it
doesn't change the model, I guess using a default in the schema makes
more sense so I don't think this needs to be fixed, just documented.


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

Ulion

@davidlgj
Copy link
Contributor

e.g. in some use case, I have text input, but the initial value is null,
when submit form, it may fail the validation. but imo the form should
override the initial value with it's "current value" which should be. but I
just don't know how to let form update to model. if you know simple call
can to that, please tell me. thx.

Its the behavior of ng-model in the case of the input, unless it's changed it doesn't set anything. And it also, as you might have noticed, removes fields that are emptied.

One way is to set defaults in the JSON Schema, but a easy way to update the model from within a field type (in custom types) is to use the service sfSelect to set a default value. (checkout source comment for further documentation) Ex:

sfSelect(scope.form.key, scope.model, '') //Sets empty string for a specific field

besides, about the default behaviour of this PR, I'm a little thought, which
break the titleMap's array order if null value is already there. maybe
better way is to not break the order, internally make it auto set
allowAutoNullOption to false? what do you think?

hmmm... that might be. I'll look into it tonight.

@ulion
Copy link
Contributor Author

ulion commented Mar 13, 2015

So there is no apply or some call can force the form elements to refresh
it's current value to model?
If there is a way to do that, I'd like to know. because the input does not
override it's current value to model (e.g. input value is "", model is
null, but the input won't update the value to the model, if no user
interaction happen on the input) I'd like have a way to let the input think
there is user interaction happens, then it will try to update it's current
value to the model. is there a generic way to do this (not for certain
initial null or empty value, just force any input update value to model)?

2015-03-14 5:29 GMT+08:00 David Jensen notifications@github.com:

e.g. in some use case, I have text input, but the initial value is null,
when submit form, it may fail the validation. but imo the form should
override the initial value with it's "current value" which should be. but I
just don't know how to let form update to model. if you know simple call
can to that, please tell me. thx.

Its the behavior of ng-model in the case of the input, unless it's
changed it doesn't set anything. And it also, as you might have noticed,
removes fields that are emptied.

One way is to set defaults in the JSON Schema, but a easy way to update
the model from within a field type (in custom types) is to use the service
sfSelect to set a default value. (checkout source comment for further
documentation) Ex:

sfSelect(scope.form.key, scope.model, '') //Sets empty string for a specific field

besides, about the default behaviour of this PR, I'm a little thought,
which
break the titleMap's array order if null value is already there. maybe
better way is to not break the order, internally make it auto set
allowAutoNullOption to false? what do you think?

hmmm... that might be. I'll look into it tonight.


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

Ulion

@davidlgj
Copy link
Contributor

Maybe, check the source code for the input directive in angular, it might be that you can trigger a "change" event? https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L1055

The ngModelController also has a couple of functions, $setViewValue (and in 1.3 $commitViewValue) which should work.

But I'm not sure what happens with the model value if viewValue is "", I don't think it does anything.

@ulion
Copy link
Contributor Author

ulion commented Mar 13, 2015

That helps, I will test whether the "input" or "change" event would trigger
the input value update to model.

2015-03-14 6:52 GMT+08:00 David Jensen notifications@github.com:

Maybe, check the source code for the input directive in angular, it might
be that you can trigger a "change" event?
https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L1055

The ngModelController also has a couple of functions, $setViewValue (and
in 1.3 $commitViewValue) which should work.

But I'm not sure what happens with the model value if viewValue is "", I
don't think it does anything.


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

Ulion

@davidlgj
Copy link
Contributor

davidlgj commented Mar 20, 2015

Just so you know, I had to fix a release before I could get this merged. I want write some user docs etc... but it will come.

@ulion
Copy link
Contributor Author

ulion commented Mar 20, 2015

just added a commit on this PR to not change options order of titleMap,
even null value detected in it.

2015-03-21 6:24 GMT+08:00 David Jensen notifications@github.com:

Just so you now, I had to fix a release before I could get this merged. I
want write some user docs etc... but it will come.


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

Ulion

@davidlgj davidlgj mentioned this pull request Apr 1, 2015
@dstelter
Copy link

dstelter commented May 9, 2015

Any estimates on when this feature might get merged? Is there any way I could assist the process?

@davidlgj
Copy link
Contributor

@dstelter sorry... no ETA, but it is top of my list now. I've also got half of a merge done. I'll share as soon as possible and then I'd love some help testing it out.

@PhilBroadbent
Copy link

Hi David - this this get merged in?

@Anthropic
Copy link
Member

@davidlgj any chance you can find a moment to fill me in on how far you got with this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants