Re-work pagination and add tests. #38
Conversation
…ults Set host/namespace defaults to use ember-cli express server
- The ember-cli-ember-data addon is deprecated
switched the dependency to ember-data instead
Fix for "Cannot read property 'pkg' of null" error
| @@ -23,13 +23,30 @@ export default DS.RESTSerializer.extend({ | |||
| * @param {Object} payload | |||
| */ | |||
| extractMeta: function(store, type, payload) { | |||
|
|
|||
| function extractPageNumber(url) { | |||
There was a problem hiding this comment.
Not sure about the name url for this argument. Maybe queryString fits better?
There was a problem hiding this comment.
next and previous are URLs and the regex requires ? so the parameter should be a URL with a query string. How about queryStringURL?
There was a problem hiding this comment.
For those following: the last comment from @dustinfarris is gone but he now agree that the parameter url is correctly named.
benkonrath
force-pushed
the
pagination
branch
from
223a92a to
7501b10
Compare
|
Two issues still need to be addressed which are in hidden / out-dated comments:
|
|
I think we should set to if (meta.next) {
...I think I'd like to see |
|
Yep, I agree with both suggestions. I'll have time tomorrow morning to re-work this and checkout your docs PR. |
|
Still needs docs which I'll add later today or tomorrow. |
|
I suggest adding a separate document for Pagination. If you add it to the mkdocs.yml file it will show up in the sidebar. |
|
I'm a bit delayed on this. I'll have more time at the end of this week / next week. I want to update one of my projects to use this before it goes in so that I have a working example to help write clear documentation. I also want to make sure this can work with https://github.com/mharris717/ember-cli-pagination. |
|
@benkonrath I moved version-1.0 branch to master and submitted v0.5.0. Can you edit this PR to point at master? |
* extractPageNumber is now a private method on the serializer. * null is returned from extractPageNumber when the page query param isn't in the url. * Create unit tests specifically for extractPageNumber and remove the previous indirect tests. * next / previous are now always included in the metadata.
|
The work in this PR will continue in #45. |
Here's a first version. This version doesn't include
nextorpreviousin the metadata if a number for page can't be found. This is only really expected to happen whennextorpreviousare null for the first and last page. An alternative would be includenextandpreviousin the metadata and set them to -1 for the first and last pages. What do you think?I also still need to write user facing docs but I'm waiting for that to be setup.