backbone.marionette icon indicating copy to clipboard operation
backbone.marionette copied to clipboard

CollectionView Comparator documentation is confusing when using functions

Open burnhamup opened this issue 3 years ago • 1 comments

Description

I was trying to sort my CollectionView with different types of comparators, and the behavior was kind of confusing and inconsistent to the documentation.

Specifically I was looking at this example from the docs near getComparator. https://marionettejs.com/docs/v4.1.2/marionette.collectionview.html#sorting-the-children

import { CollectionView } from 'backbone.marionette';

const MyCollectionView = CollectionView.extend({
  sortAsc(model) {
    return -model.get('order');
  },
  sortDesc(model) {
    return model.get('order');
  },
  getComparator() {
    // The collectionView's model
    if (this.model.get('sorted') === 'ASC') {
      return this.sortAsc;
    }

    return this.sortDesc;
  }
});

Expected behavior

According to the documentation, I would expect the CollectionView to sort the models in it's collection according to the 'order' property. If I have a collection with models with an age attribute, I would expect the following call to sort them oldest to youngest.

    sortAgeDesc() {
  	this.setComparator((model) => {
    	return -model.get('age');
    })
  }

Actual behavior

The above code fails with Uncaught TypeError: model.get is not a function

This is because the CollectionView actually passes in instances of the child views, and not the models. So something like

  sortAgeDescWithView() {
    this.setComparator((childView) => {
    	return -childView.model.get('age');
    })
  }

actually works how I would expect.

I created a JSFiddle here https://jsfiddle.net/a2xjcvp3/1/

I'm not sure if the issue is just confusing documentation. The example in the docs makes it seem like any functions you set as the comparator for a CollectionView will be given model instances. If the code is working as intended, than only the documentation needs to be fixed.

As an aside the setComparator('age') works as expected.

Environment

  1. Marionette version: 4.1.3
  2. Backbone version: 1.4.0
  3. Additional build tools, etc:

burnhamup avatar Feb 23 '21 01:02 burnhamup

Yep I think that's holdover documentation from v3 that didn't get updated when comparators were passed the model instead of the view. Those sort methods should be like sortAsc({ model }) {

paulfalgout avatar Feb 23 '21 13:02 paulfalgout