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

onChange blocked #46

Closed
PaddyMann opened this issue Feb 20, 2015 · 15 comments
Closed

onChange blocked #46

PaddyMann opened this issue Feb 20, 2015 · 15 comments

Comments

@PaddyMann
Copy link

Setting an onChange config option has no effect.

Could be fixed by adding a line to selectize.js config.onChange:

      config.onChange = function(value){
        if( !angular.equals(selectize.items, scope.ngModel) )
          scope.$evalAsync(function(){
            var value = angular.copy(selectize.items);
            if (config.maxItems == 1) {
              value = value[0]
            }
            modelCtrl.$setViewValue( value );
          });
        scope.config.onChange(value);
      }

For some reason my onChange function now gets called twice. Better than not at all though....

@PaddyMann
Copy link
Author

Here's a better solution that doesn't call onChange twice:

      config.onChange = function(){
        if( !angular.equals(selectize.items, scope.ngModel) )
          scope.$evalAsync(function(){
            var value = angular.copy(selectize.items);
            if (config.maxItems == 1) {
              value = value[0]
            }

            if(modelCtrl.$viewValue != value) {
              modelCtrl.$setViewValue( value );
              scope.config.onChange(value);
            }
          });
      }

@machineboy2045
Copy link
Owner

Why not use a $watch() statement in your angular controller?

@PaddyMann
Copy link
Author

I could have, but I assumed all of the config options to work and would have saved a couple of hours of headscratching had I known the of the issue :)

I was going to add that adding a $watch to the $digest cycle for each selectize input isn't ideal either, but then I'm unsure how that compares performance-wise with this implementation.

@gottfrois
Copy link

+1 on this, same issue, onChange called twice

@donpinkus
Copy link

+1 also seeing onChance called twice

sounds like we should just watch the model for now

@geemang2000
Copy link

+1 onChange does nothing, should expose same events as selectize

@geemang2000
Copy link

Nor does onInitialize

@geemang2000
Copy link

For what it's worth, the ng-change directive seems to work.

ng-change="change(app.settings)"

@miketeix
Copy link

+1 onChange is being called twice

2 similar comments
@andrewdelprete
Copy link

+1 onChange is being called twice

@Uelb
Copy link

Uelb commented Jul 28, 2015

+1 onChange is being called twice

@andrewdelprete
Copy link

👏

@PhiLhoSoft
Copy link
Contributor

As I wrote as comment to your commit (but I feel it belongs more here), I think you still need to set the silent flag to the setValue calls. Not only it prevents unnecessary calls (minor), but it also fixes the double call to onChange in the case of single selection mode.

PhiLhoSoft added a commit to PhiLhoSoft/angular-selectize that referenced this issue Aug 6, 2015
- setValue is always silent (see machineboy2045#46)
- deactivate watch when updating the options list internally
- user entered options, when removed, must be removed from the list
- tested in http://plnkr.co/edit/2352rt
@machineboy2045
Copy link
Owner

I've created a v3 branch that uses the latest version of Selectize:
https://github.com/machineboy2045/angular-selectize/tree/v3.0.0

I was able to simplify the Angular directive a lot. I'd be curious what you
think.

I like the idea of adding support for objects. But I'm worried if we add
functionality that doesn't exist in Selectize itself, then we could diverge
from it so far that we can't take advantage of updates. It would be ideal
if Selectize would add support for objects. But who knows when that would
be.

What if I made an experimental branch? Maybe there could be a better name
for it. And we could add object support to it?

Also, thanks for all your contributions. I know you've been very active
answering questions in the issue pages. :)

On Thu, Aug 6, 2015 at 1:30 AM, Philippe Lhoste notifications@github.com
wrote:

As I wrote as comment to your commit (but I feel it belongs more here), I
think you still need to set the silent flag to the setValue calls. Not only
it prevents unnecessary calls (minor), but it also fixes the double call to
onChange in the case of single selection mode.


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

@PhiLhoSoft
Copy link
Contributor

Yes, it is nice, it has a cleaner style.
Suggestion: take the opportunity of a new major version number to rename the file from selectize.js to angular-selectize.js, to avoid some confusion (eg. when debugging).

Support for objects: would be nice, I can live without, it just has the overhead of searching the object in the list of options.
Perhaps you can add an angular-selectize specific configuration to enable this.
It is not a major divergence from Selectize, it is just an added feature in the wrapping.

A more risky operation (toward future versions of Selectize) would be to monkey-patch it to fix some issues, like I did in selectize/selectize.js#783
Something that would be useful to have in the wrapper, but currently I just have it in the service wrapping angular-selectize.

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

No branches or pull requests

9 participants