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

Support sorting by nested properties #5

Open
ohadschn opened this issue Dec 14, 2014 · 5 comments
Open

Support sorting by nested properties #5

ohadschn opened this issue Dec 14, 2014 · 5 comments
Milestone

Comments

@ohadschn
Copy link
Contributor

The current sorting code starts with the following:

rows.sort (a,b) =>
        aVal = ko.utils.unwrapObservable a[@sortField()]
        bVal = ko.utils.unwrapObservable b[@sortField()]

As such, only flat properties of the row object can be used for sorting.
For example, suppose my row looks like this:

{
    foo:
    {
        bar: "foobar"
    }
}

It would have been nice to be able to sort by foo.bar

@cmbankester
Copy link
Contributor

Typically, the filter is bound directly to a text/search input and aren't often accessed programmatically (at least that's the way we usually use it). I can see the functionality you're describing being mighty useful, though. If you want to submit a PR, I'd be glad to merge it in. Otherwise, I'll get to this one when I have some free time on my hands.

@ohadschn
Copy link
Contributor Author

ohadschn commented Dec 16, 2014

Hi @cmbankester,

I was referring to the sorting feature (not filtering), which is typically bound to a property on the row. For example if we take demo.html, a row looks like this:

function City(view, row) {
          this.view = view;
          this.population = ko.observable(row.population);
          this.countryName = row.country_name;
          this.cityName = row.city_name;
        }

And in the HTML you have a binding such as:

<th data-bind="click: toggleSort('cityName')" class="sortable">

What I'm saying is that if cityName were a complex object, say:

cityName = 
{
   shortName : "NY",
   fullName: "New York"
}

Then the following would not work presently:

<th data-bind="click: toggleSort('cityName.shortName')" class="sortable">

@ohadschn
Copy link
Contributor Author

On an unrelated note, I don't want all the issues I've opened to make the wrong impression. I think knockout-datatable is great. I would not have gone to the trouble of going over the sources (learning CoffeeScript and LESS for this purpose alone) and opening all these issues otherwise. I have searched long and hard and I believe this is the best solution of its kind. Kudos!

@cmbankester
Copy link
Contributor

@ohadschn Thanks! I didn't get the wrong impression at all, I am very pleased that these issues have been opened. It's great to see other people getting use out of our open-source products.

One approach to the sort-by-object-attributes situation you've outlined above could be to alias cityName.shortName as cityShortName on your row, making your row look like:

function City(view, row) {
  this.view = view;
  this.population = ko.observable(row.population);
  this.countryName = row.country_name;
  this.cityName = row.city_name;
  this.cityShortName = this.cityName.shortName;
}

with corresponding binding:

<th data-bind="click: toggleSort('cityShortName')" class="sortable">

Would that be acceptable? I don't believe it would be much trouble to add support for sorting by object attributes, I'm just used to flattening my sortable/filterable row attributes.

@ohadschn
Copy link
Contributor Author

Yes, that's precisely what I did in my case, so for me it was totally acceptable :)

Perhaps there could be some other scenarios where row structure has to be maintained though, or at least easier to keep the way it is (I know I had to go and rename a lot of stuff, which is tiresome in untyped languages like javascript and HTML)

@cmbankester cmbankester added this to the 0.6.0 milestone Dec 20, 2014
@cmbankester cmbankester modified the milestones: 1.0.0, 0.6.0 Jan 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants