Skip to content
This repository has been archived by the owner on Sep 10, 2020. It is now read-only.

Allow "hitsPerPage" and "page" query parameters #192

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Allow "hitsPerPage" and "page" query parameters #192

wants to merge 5 commits into from

Conversation

tholman
Copy link

@tholman tholman commented Aug 10, 2017

This PR allows the apiServer to pass through hitsPerPage and page query params, as per the Algolia api

Includes:

  • Small changes to apiServer.js to allow the passing of the params
  • Changes to the documentation page, showing how to use the params

templates/api.html Outdated Show resolved Hide resolved
templates/api.html Outdated Show resolved Hide resolved
@tholman
Copy link
Author

tholman commented Aug 15, 2017

Any questions/clarifications needed here? I'm happy to chat.

@tholman
Copy link
Author

tholman commented Aug 22, 2017

Anywhere I can drop a few $'s to get this & #191 looked at? :) cc @PeterDaveHello

apiServer.js Show resolved Hide resolved
Copy link
Contributor

@PeterDaveHello PeterDaveHello left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request and sorry for the late review!
There are few empty line changes not related to the function itself, please revert it.

I have only two concerns here:

  1. Would hitsPerPage be appropriate enough or can we use something more friendly than hits here?
  2. A little bit familiar with the above issue, I wonder how many websites using page = 0 as the first page in the parameter? What do you think about we treat 1 as the first page and just minus one and then send to API?

apiServer.js Outdated Show resolved Hide resolved
apiServer.js Outdated Show resolved Hide resolved
apiServer.js Outdated Show resolved Hide resolved
apiServer.js Outdated Show resolved Hide resolved
@tholman
Copy link
Author

tholman commented Sep 8, 2017

Would hitsPerPage be appropriate enough or can we use something more friendly than hits here?

I went with hitsPerPage as the query parameter, for the most part so the API matches with the algolia API, which we're using here to get the results anyway. Happy to change it, I only feel like it adds a small layer of unneeded confusion.

A little bit familiar with the above issue, I wonder how many websites using page = 0 as the first page in the parameter? What do you think about we treat 1 as the first page and just minus one and then send to API?

Same point here as with the hitsPerPage issue, feels strange to defer from the algolia api that we're interfacing with in the first place.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants