Skip to content

Bug: first/prev links not rendered on second page #622

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

Closed
bart-degreed opened this issue Nov 13, 2019 · 3 comments
Closed

Bug: first/prev links not rendered on second page #622

bart-degreed opened this issue Nov 13, 2019 · 3 comments

Comments

@bart-degreed
Copy link
Contributor

Description

When using paging, the first and prev links are not rendered on the second page.

Example:
First page is at URL /api/books (A), which contains a link to next page: /api/books/?page[number]=1 (B). That page contains a link to its next page: /api/books/?page[number]=2 (C).

Observed behavior:
Page C contains top-level links for first and prev, but they are missing on page B.

Expected behavior:
I think the correct behavior would be for first and prev to exist also on page B, because it is the second page.

I traced this down to LinkBuilder.SetPageLinks(...), which conditionally sets them:

if (_pageService.CurrentPage > 1)
{
    links.First = GetPageLink(primaryResource, 1, _pageService.PageSize);
    links.Prev = GetPageLink(primaryResource, _pageService.CurrentPage - 1, _pageService.PageSize);
}

I think the condition should be changed to > 0, as index 0 appears to be the first page.

Environment

latest version of Develop branch

@maurei
Copy link
Member

maurei commented Nov 14, 2019

Thanks for reporting this. I will be looking into it in the beginning of next week.

@maurei
Copy link
Member

maurei commented Nov 19, 2019

I think the condition should be changed to > 0, as index 0 appears to be the first page.

Note that the index is 1-based, which means that the value for Next was also generated wrongly: it displayed page number 1 under Next when having request the first page.

#625 fixes this, and will release a new alpha version by the end of today. Thanks for reporting!

@bart-degreed
Copy link
Contributor Author

Thanks for the fix!

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

No branches or pull requests

2 participants