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

Pagination #45

Merged
merged 1 commit into from
Jan 8, 2015
Merged

Pagination #45

merged 1 commit into from
Jan 8, 2015

Conversation

benkonrath
Copy link
Collaborator

This PR is taking over from #38 because it's not possible edit the destination branch. The PR was originally against the version-1.0 branch which has now been merged into master.

@dustinfarris
Copy link
Owner

This is just waiting on docs, right?

@benkonrath
Copy link
Collaborator Author

Yeah. I'm hoping to have time for this later in the week.

@benkonrath
Copy link
Collaborator Author

Ok, I found some time to do a first version of the docs. I've asked for contributions for docs about integration with ember-cli-pagination since I don't have time to figure it out right now.

Feel free to give the docs a thorough edit for style, tone, bad grammar or whatever. I won't be offended. :)

@benkonrath
Copy link
Collaborator Author

I need to add an entry in the Changelog. I'll do that right now.


```js
if (meta.next) {
store.find('post', {page: meta.next}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a closing paren here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I just fixed it.

@dustinfarris
Copy link
Owner

Documentation looks good. I left a couple nitpicking comments.

@benkonrath
Copy link
Collaborator Author

I've updated the documentation. I added all of your corrections and added a new section about the page size that I need to finish once we have thing solidified.

@benkonrath
Copy link
Collaborator Author

The two things remaining:

  • Decide if having the pageSize in the metadata is a good idea.
  • Finish documentation about page size

@benkonrath
Copy link
Collaborator Author

After some thought, I think including the pageSize in the metadata isn't a good idea because of the issue with not having an accurate value on the last page. We can always add it later if there is a need but I think the metadata is useful for pagination without it. I'm going to remove the last commit and update the docs about the page_size DRF query param.

@benkonrath
Copy link
Collaborator Author

Ok, I've re-worked the docs for page_size. @dustinfarris Can you look at the pagination docs one more time?

@dustinfarris
Copy link
Owner

👍

dustinfarris added a commit that referenced this pull request Jan 8, 2015
@dustinfarris dustinfarris merged commit ca9ad7d into dustinfarris:master Jan 8, 2015
@benkonrath benkonrath deleted the pagination branch January 13, 2015 10:10
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

Successfully merging this pull request may close these issues.

2 participants