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

chore(pagination): pageChanged triggered multiple times #138

Closed
templth opened this issue Feb 4, 2016 · 2 comments
Closed

chore(pagination): pageChanged triggered multiple times #138

templth opened this issue Feb 4, 2016 · 2 comments
Assignees

Comments

@templth
Copy link

templth commented Feb 4, 2016

Hi all,

I open this issue following a question on StackOverflow. See http://stackoverflow.com/questions/35081261/ng2-bootstrap-pagination-pagechanged-triggered-multiple-times/.

We saw that the pageChanged event is fired three times at the initialization of the pagination component. We expected to have only one.

After investigating a bit, it's linked to the use of a TypeScript setter for the page property with a custom event. Because Angular2 fires events asynchronously (see this issue: angular/angular#6311), setting values to the page property fires event after the component initialization

We saw an inited property but it doesn't seem to be useful in this context...

We wonder if it's a normal behavior or a bug.

Here are some proposals to only have one events fired at the component startup:

(file node_modules/ng2-bootstrap/components/pagination/pagination.ts):

  • Update the set page block to trigger events only when the inited property is true:

    public set page(value) {
      this._page = (value > this.totalPages) ? this.totalPages : (value || 1);
    
      if (this.inited) { // <---------------
        this.pageChanged.next({
          page: this._page,
          itemsPerPage: this.itemsPerPage
        });
      }
    }
    
  • Update the ngOnInit method not to set the inited property to true at its end:

    ngOnInit() {
      (...)
      //this.inited = true;
    }
    
  • Set the inited property to true at the end of the first call of the writeValue:

    writeValue(value:number) {
      this.page = value;
      this.pages = this.getPages(this.page, this.totalPages);
    
      if (!this.inited) {
        this.inited = true;
      }
    }
    

Thanks very much for your feedback!
Thierry

@krzysztofsaja
Copy link
Contributor

Hi,
I've discovered this bug today as well. However, I believe that 'pageChanged' event shouldn't be fired even on component init but only if page is really changed. I've sent pull request for this bug.

Thanks,
Krzysztof

@templth
Copy link
Author

templth commented Feb 5, 2016

Agreed! +1

Thanks very much!
Thierry

@valorkin valorkin changed the title ng2-bootstrap pagination pageChanged triggered multiple times chore(pagination): pageChanged triggered multiple times Feb 6, 2016
@valorkin valorkin self-assigned this Feb 6, 2016
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

3 participants