josecebe / twbs-pagination

jQuery pagination plugin (bootstrap powered)
http://josecebe.github.io/twbs-pagination/
Other
1.1k stars 401 forks source link

Move trigger page from show property to setupEvents. This closes the #148 #151

Closed ngmathiou closed 6 years ago

ngmathiou commented 7 years ago

In our project we use server side paging, many times we want to disable paging because there is unsaved data in the view. Using your code as was we had a minor problem when we should block the navigation. By clicking the navigate button ('next','last', etc.) the UI was updating but our "logic" was blocking the navigation. So, the paginator control was changing pageNo but not regarding data from our server side paging. We moved the trigger event of page outside show function to setupEvents.

We propose to expose two properties like onPageClick, for example onPageChanging and onPageChanged. onPageChanging: will contains all the "logic" that must be executed before make any changes in UI onPageChanged: will run after onPageChanging and basically will just update the UI.

Edit: Also, the onPageClick event fires the first time we initialize the component, witch looks strange and should not happen.

josecebe commented 6 years ago

Your proposal of creating a new method 'onPageChanging' seems good to me.

But I don't think that moving the 'page' trigger to 'setupEvents' method is the better solution for this.

You can disable the first 'onPageClick' including this option into your twbsPagination: initiateStartPageClick: false

Do you think this commit is a proper solution for your case? https://github.com/josecebe/twbs-pagination/commit/fb94441e3b2c829119e1ee0c1c36079ff9157012

ngmathiou commented 6 years ago

Seems good for our case. Thank you 👍