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

The selectedItems input is never used #85

Open
Fredx87 opened this issue Sep 18, 2019 · 7 comments
Open

The selectedItems input is never used #85

Fredx87 opened this issue Sep 18, 2019 · 7 comments

Comments

@Fredx87
Copy link

Fredx87 commented Sep 18, 2019

The selectedItems input is not used inside the dts-select-container component. If the user pass an array with some items already selected, they will not be considered by the component.

This is an example: https://stackblitz.com/edit/ngx-drag-to-select-yqxr4t

In the HomeComponent I'm passing an array with some values:

<dts-select-container [selectedItems]="selected">

but they are not marked as selected in the component. Moreover, if I use the "banana box" syntax, the component will clear the selected array and delete the values that are present in the passed array

@d3lm
Copy link
Owner

d3lm commented Sep 18, 2019

Hey there!

Yes that is correct and also more or less by design. That input is not meant to be used to pass in an array of items that are selected. There are other APIs that you can use to programatically select items. The input exists so that we can use a two-way binding on the select container, e.g. [(selectedItems)]="selectedItems". This is only possible if there is an @Input with that name and an @Output called selectedItemsChange, that is the same name as for the input but with a "Change" suffix in the end.

Does that make sense?

@Fredx87
Copy link
Author

Fredx87 commented Sep 18, 2019

"Two-way data binding" means that the binding is bidirectional: from the parent component to the child and vice-versa. For this reason in Angular templates is indicated with the "banana box" syntax, remarking that the variable passed is used both as an input and an output.

Since the select container-component is not using the input, i don't think it can be called two-way data binding, but it can be considered as an output, that is the same of the select output.

Moreover, the documentation says that selectedItems is an input that contains "Collection of items that are currently selected".

@kaziupir
Copy link

So this is only output, not input.

@d3lm
Copy link
Owner

d3lm commented Oct 21, 2019

Yea, you are right. It's not a true two-way data binding in the sense that whatever you pass in is also used internally. Changing this is definitely a breaking change and I am hesitating to make it a true way data binding. I'd rather not use that cause I am personally against bi-directional bindings because it's harder to keep track of changes and I prefer a uni-directional data flow. I only added support for the "banana in the box" syntax to make it easier to bind a model to select item changes, so that you don't have to assign this yourself, e.g. (selectedItemsChange)="myModel = $event", which is more verbose than [(selectedItems)]="myModel", and even tho it looks like a bi-directional binding it's just library -> user-land and not vice-versa.

If you want to select items programatically there's API on the select container.

@vincentjdc
Copy link

Hi,

This is a really weard behavior. Most Angular 2+ developers will expect a true 2-way binding when seing something like [(something)]

I understand that you don't like 2-way binding. But it's harder to keep track of changes when you see something that is like 2-way binding without being it...

As that change is a break change, could it not be an option to passe to DragToSelectModule.forRoot() ?

Maybe something like :

DragToSelectModule.forRoot({
    selectedItemsTwoWayDataBinding: true
});

@vincentjdc
Copy link

I made some changes to let the possibility to have two-way binding :

vincentjdc@f814a12

What do you think ?

@d3lm
Copy link
Owner

d3lm commented Mar 9, 2020

@vincentjdc Yes I like the idea of using the config to enable this. I see if I can get this in soon.

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

4 participants