NejcZdovc / ng2-select2

Select 2 wraper for angular2
MIT License
123 stars 93 forks source link

Add support for angular2 forms #63

Open gergelybodor opened 7 years ago

gergelybodor commented 7 years ago

Hi! I coded a working example of form implementation for your select2 component! We are actually using this code in one of our web-app. Please review our changes!

achimha commented 7 years ago

Would be nice if this PR got merged. A similar previous one is still open?

gergelybodor commented 7 years ago

Yes. The other one seems incomplete to me. I tried using that in a project and didn't work.

achimha commented 7 years ago

I think this PR is problematic and should probably not be applied. The writeValue method is called early at initialization and it has its own select2 init call in it which means every control gets initialized twice and this breaks things. I can work around it like this:

private isSelect2Initialized() {
        return this.element.hasClass('select2-hidden-accessible');
}

writeValue(newValue: any) : void {
        this.renderer.setElementProperty(this.selector.nativeElement, 'value', newValue);
        if (!this.isSelect2Initialized()) {
            return;
        }
        this.element.trigger('change.select2');

        this.onChange(newValue);
        this.onTouched();
        this.valueChanged.emit({
            value: newValue,
            data: this.element.select2('data')
        });
    }
achimha commented 7 years ago

Another point is that I believe we should not return just the value (i.e. a string). This often means information loss. Instead we should return an object with the value and the data associated.

gergelybodor commented 7 years ago

Another point is that I believe we should not return just the value (i.e. a string). This often means information loss. Instead we should return an object with the value and the data associated.

This PR is about adding angular2 form support for your component. I really don't care what you want to return in your output. That shouldn't be the subject of this PR.

achimha commented 7 years ago

The question is what the "value" of a select2 in a form should be. Just the string? What if the item has much more than its text? How would you get hold of the object? element.val() is just a subset of element.select2('data').

Therefore I am not sure this is the right approach.

gergelybodor commented 7 years ago

The value input is of type string or string array as far as i know. When you initialize your form with select2 in it, you initialize it with a string or string array. When you change the form or submit it, the form control contains the current value of the select2, that is a string or string array. So I dont really understand your statement "What if the item has much more than its text?" You get the Select2OptionData object's id value out of the form, and that is perfectly fine by me.

achimha commented 7 years ago

The question is -- should there be a separate API to get the complete record or should the main API be able to handle it? Look at the following use case:

$('#myselect2').data('select2').trigger('select', {
  data: { id: 1, text: 'foo', myproperty: false }
});

This invocation you need for an AJAX Select2 to set a value programmatically.

Isn't it true that most people use Select2 for showing/selecting data via AJAX? The local data use case is is of course much simpler but is this why people go for a complex control like Select2? I am worried about modelling the API for just one use case.

We could extend the current API to take string|string[]|Object and then make the appropriate Select2 call when an object is passed.

gergelybodor commented 7 years ago

I just don't think passing any other object other than the selected values into a form is practical. If you need any other fields of the Select2OptionData object, just write a simple get function. I also don't see any reason to not just returning a string or string array to the form. I don't recall ever having any kind of information loss. If you have any sugestions of how to implement passing an object to the form, I would be happy to modify this PR, but otherwise I believe this code works as is.

pralhadstha commented 7 years ago

Is there any other method that helps in solving the issue of formControlName not supporting by this package? When will this pull request be merged??

keyvhinng commented 7 years ago

I know that it may not be the proper place for questions but I need to make some modifications to ng2-selet2's source code for a custom behavior. What is the process to modify the source code and build it ? If I run the 'build' script defined in package.json I have the following issue: error TS2339: Property 'amd' does not exist on type '{ (): JQuery; (it: IdTextPair): JQuery; (method: "val"): any; (method: "val", value: any, trigger...'