Change defaultBinding on dataset property in aurelia-slickgrid custom element about aurelia-slickgrid HOT 11 CLOSED

I'm not sure that you can go with a oneWay binding on dataset, especially Example 11 where it adds item on the fly.

You can give it a try, I might be wrong but I thought twoWay was more appropriate. Like in my current App, I a "Create Item" which will add it to the grid (on first row) once the item is created, just like in Example 11

from aurelia-slickgrid.

it should still work. I think the two way means that if you reset the dataset to something else it changes in the view model too. Give it a try and let me know your result when you have a chance, the change would be:

-  @bindable({ defaultBindingMode: bindingMode.twoWay }) dataset: any[];
+  @bindable dataset: any[];

if you do discover a potential issue let me know and i can look at it, or this can just be added to the documentation.

from aurelia-slickgrid.

Well actually, the Service is probably doing the work, not the binding. So yeah you're probably right. What benefit will that give us? Performance?

Anyhow, can you make sure that the Example 11 still works after changing that. If that is ok, then I don't mind going ahead. We could also include it in the next upcoming release, if you have time to create PR

from aurelia-slickgrid.

there are two benefits i can think of

1. Maybe a performance boost? Probably not noticeable, but Aurelia doesn't have to setup the two way communication on a dataset. This is a guess and not tested
2. I can use a @computedFrom property on my viewmodel without specifying one-way in the view without getting that foreign error. So that is less to remember to do/setup for the user. In my app i dynamically generate urls using the router and use that dataset in my grid. For example, my server dataset is elements and my viewmodel dataset is editUrls in my viewmodel:

  @computedFrom('elements')
  get editUrls() {
    return this.elements.map(e => ({
      ...e,
      editUrl: this.router.generate('component-edit', { id: e.id })
    }));
  }

Example 11 does work, but I will be sure to test the other examples. I will submit a PR if you think this is helpful.

EDIT or I can just update the documentation with a "how to" if someone wants to do this? I'll let you decide.

from aurelia-slickgrid.

As long as all examples still works, I'm all good with it and yes I know we might not visually see a boost but it will still be there, less twoWay is always better.

I assume you are using the grid without Backend Service API, right? I mean you are simply passing your dataset to the same property, correct? If so, I assume you will be the first person to notice any issues with them, since I'm mostly using the Backend Service API (GraphQL) on my side (in Angular that is, sadly I most admit).

Updating wikis are always good, we can always refer to them if any questions are brought up and these are all helpful. I often go back and read my own wikis, to know how I use things lol. The grid now has so many features, wikis are the best to keep track and be helpful to users.

EDIT
I now understand your @computedFrom, do we always have to use that or does the previous way of using it still works? I wouldn't want to break anyone's code, at least not before a version 2.x.

from aurelia-slickgrid.

i agree about wikis, I will do some thorough testing the best I can before making this change.

from aurelia-slickgrid.

ok when did u plan to have a PR for this? I was planning a release by tomorrow evening, should I wait for it or go ahead with a release tomorrow?

By the way, I'm going in vacation starting Saturday morning on the 14th. You'll be on your own after (for 3 weeks that is) :P
I don't mind you do releases though

from aurelia-slickgrid.

Go ahead and release, I want to make sure I test this enough since it is a small change that has a big impact.

Enjoy your much deserved vacation! I do not think I will need to release anything new while you are gone. From what I have seen so far today since I am working on the grid, it has everything I need.

from aurelia-slickgrid.

Just saw

EDIT
I now understand your @computedFrom, do we always have to use that or does the previous way of using it still works? I wouldn't want to break anyone's code, at least not before a version 2.x.

You do not have to use @computedFrom after this change is in. I just used it in the example so aurelia did not keep retrieving the data every second. If we did not use @computedFrom then aurelia would fallback to its dirty checking and call the getter every second.

from aurelia-slickgrid.

Sounds good then

from aurelia-slickgrid.

closed by #65

from aurelia-slickgrid.