onPaginate not working

[ghost]

I cannot access the global variables inside the onPaginate method.

I am using EC6 standard.

Here is my controller ` class ContactsController { constructor($state, ContactsService, $q, $timeout) { this.selected = []; this.query = { order: 'name', limit: 5, page: 1 }; this.limitOptions = [5, 10, 15]; this.items = null; this.columnHeaders = null; this.ContactsService = ContactsService; this.progress = false; this.$q = $q; this.$timeout = $timeout; this.getContactsHeaders(); }

getContactsHeaders() {
    let promise = this.getHeaders();
    promise.then((headers) => {
        this.columnHeaders = headers;
        this.getItems();
    });
}

getHeaders(callback) {
    return this.$q((resolve, reject) => {
        resolve(this.ContactsService.getHeaders());
    });
}

getItems() {
    this.progress = true;
    let promise = this.getContacts();
    promise.then((contacts) => {
        this.items = contacts;
    }, (reason) => {
        console.log('Failed: ' + reason);
    });
};

getContacts(callback) {
    return this.$q((resolve, reject) => {
        resolve(this.ContactsService.getContacts(this.query));
        reject("Error Occured");
    });
}

onPaginate(page, limit) {
    console.log("query: " + this.query);
}

}

export default ContactsController;`

Here is the html `

Contacts

            </md-toolbar>
            <md-table-container ng-if="cc.items">
                <table md-table md-row-select multiple ng-model="cc.selected" md-progress="cc.progress">
                    <thead md-head md-order="cc.query.order">
                        <tr md-row>
                            <th md-column ng-repeat="column in cc.columnHeaders">
                                <span>{{column}}</span>
                            </th>
                        </tr>
                    </thead>
                    <tbody md-body>
                        <tr md-row md-select="item" md-select-id="item.id" md-auto-select ng-repeat="item in cc.items.data">
                            <td md-cell>{{item.name}}</td>
                            <td md-cell>{{item.reporting_person}}</td>
                            <td md-cell>{{item.address}}</td>
                            <td md-cell>{{item.email}}</td>
                        </tr>
                    </tbody>
                </table>
            </md-table-container>
            <md-table-pagination md-limit="cc.query.limit" md-limit-options="cc.limitOptions" md-page="cc.query.page" md-total="{{cc.items.count}}" md-on-paginate="cc.onPaginate" md-page-select="true"></md-table-pagination>
        </div>`

Other methods works fine except when i try to go to next pages which uses the onPaginate mthod.

[iudelsmann]

The problem happens because the javascript scope is not the controller, so "this" is not what you're expecting it to be. What I did for now is to assign the method to the $scope of the controller, then when it is called it will correct the scope. Something like this:

constructor($scope) {
    this.$scope = $scope
    this.$scope.onPaginate = () => this.onPaginate();
}

onPaginate() {
    [....]
}

Then of course you don't need the "cc.onPaginate", only "onPaginate". I'm still looking for a better option if anyone knows one.

[BrunnerLivio]

I'm having the same issue. Thanks a lot for the workaround @iudelsmann. That will do it. But I really don't like that you have to rely on this workaround. Why even expect a function as parameter on md-on-paginate event in the first place?

Why not use the '&' attribute selector in mdTablePagination.js instead of '='. More info

@daniel-nagy

[GabrielGil]

Definitely. I am also with @BrunnerLivio. It is considered a good practice as from AngularJS 1.6 to use the controller in components as the view model. Thus it is no longer needed to bind to $scope nor to create a vm var.

Also, when using ES6 makes much more sense to link the method, and not passing a function that is completely isolated. Consider the following:

import templateUrl from './template.html';

export const MenuComponent = {
  templateUrl,
  bindings: {
    menu: '<',
  },
  controller: class MenuComponent {
    constructor(DessertsService) {
      'ngInject';
      this.DessertsService = DessertsService;
    }
    $onInit() {
      this.query = {
        limit: 25,
      }
      this.getDesserts();
    }
    getDesserts() {
      let promise = this.DessertsService.get(this.query);
      promise.then(desserts => this.desserts); // `this` is not defined if executed from 'md-on-paginate' callback
    }
};

If you accept a PR to migrate the callbacks into bindings for a next release, I could work on that.

Note, just to back me up a little bit, check Todd Motto's (Google dev expert) styleguide on this ;) https://github.com/toddmotto/angularjs-styleguide

[BrunnerLivio]

Seems like this repository is not really active. Someone considering having a talk with @daniel-nagy about adding collaborators to the repository, so we don't start creating a pull request, and then in the end won't ever be accepted because inactivity of the repoauthor?

[GabrielGil]

I would be keen to be one of those collaborators, as I am heavily using this module, and I believe I could contribute. Specially to make it more ES6 friendly, and more options for module loaders usage.

[daniel-nagy]

This is a side effect of how Javascript works. When you borrow methods from objects they loose context.

For example,

let foo = {
  bar() {
    console.log(this === foo);
  }
};

let {bar} = foo;

foo.bar(); // true

bar(); // false

In this case you would need to bind foo to bar to prevent it from loosing context.

foo.bar = foo.bar.bind(foo);

let {bar} = foo;

bar(); // true

If you are using es6 classes there are two ways you can bind class methods to the class.

1. Define class methods using the arrow syntax

   class Foo {
    bar = () => {
    }
   }

   If you're using TypeScript you get this syntax out of the box. If you're using Babel you will most likely need to add a plugin.

   Be aware that this syntax can cause problems with inheritance and the use of the super method.

2. Bind class methods in the constructor

   class Foo {
    constructor() {
      this.bar = this.bar.bind(this);
    }
   
    bar() {
    }
   }

   This is probably the best way to preserve context on class methods.

[daniel-nagy]

TLDR: Binding directly to the method is more flexible and leads to less developer confusion.

@BrunnerLivio The reason I didn't use & for binding to methods is because it becomes awkward to pass parameters to the method. When you use the & syntax you get a wrapper around the original expression that gets evaluated on the parent scope. That means I would have to overwrite the context and the developer would have to make sure they spell the parameters exactly as I define them.

For example, It would look something like this

// in the pagination directive
self.onPaginate({page: self.page, limit: self.limit});

<!-- in the developer's template -->
<!-- if page and limit are any other name this will fail -->
<md-table-pagination md-on-paginate="$ctrl.onPaginate(page, limit)"></md-table-pagination>

The alternative would be I pass nothing to the method and make the developer responsible for maintaining a reference to the page and limit within the scope of their controller.

// in the pagination directive
scope.eval('mdOnPaginate');

<!-- in the developer's template -->
<!-- now the developer can pass anything within the current scope -->
<md-table-pagination md-on-paginate="$ctrl.onPaginate($ctrl.page, $ctrl.limit)"></md-table-pagination>

If I were to rewrite it I would probably do this. Then the developer can pass anything visible within their current scope to the callback.

Update

There were also issues with calling the onPaginate callback before two way data binding had a chance to update. The result is the page and limit variables in the developer's scope would still have the previous value. I would need to wrap the callback in a $timeout to wait until the next digest to call the onPaginate callback.

[BrunnerLivio]

@daniel-nagy I also think the scope.eval('mdOnPaginate');-option is the best one. What speaks against rewriting it? Sure it would break all current dependent applications, but in the long term the users would definitely benefit. If you consider releasing a 1.0.0, this issue should be included in my opinion.