Skip to content

adds limit param to set number of results#1

Open
dvdokkum wants to merge 2 commits intochartbeat-labs:masterfrom
dvdokkum:master
Open

adds limit param to set number of results#1
dvdokkum wants to merge 2 commits intochartbeat-labs:masterfrom
dvdokkum:master

Conversation

@dvdokkum
Copy link

@dvdokkum dvdokkum commented Nov 1, 2013

Set the number of Top Pages to return using '&limit=XX' in the URL. Useful for displays with a portrait orientation.

Copy link

Choose a reason for hiding this comment

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

Are you trying to check if 'limit' is defined on the configService object? This seems to specifically just check that configService.limit is not the value NaN. You'd probably want to do just a generic truthy check, since i imagine the limit should never be set to 0 legitimately? Could also just check that typeof limit is 'number'.

@dvdokkum
Copy link
Author

dvdokkum commented Nov 4, 2013

Thanks for the help @bowman224, updated with what I think is the more standard method...?

Copy link

Choose a reason for hiding this comment

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

Did you try this out? I dont think all params are added to the configService. Alternatively you could do

if ($routeParams.limit) {
limits[host] = $routeParams.limit;
}

Copy link

Choose a reason for hiding this comment

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

Also, alternatively you could extend DEFAULT_LIMIT to $routeParams.limit

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.

3 participants