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 suffix work change the href behaviour? #666

Closed
Geobert opened this issue Aug 8, 2019 · 4 comments
Closed

Pagination suffix work change the href behaviour? #666

Geobert opened this issue Aug 8, 2019 · 4 comments

Comments

@Geobert
Copy link
Contributor

Geobert commented Aug 8, 2019

I've noticed something today as I've recompiled a cobalt at work. My pagination navigation is like https://github.com/Geobert/blog/blob/master/src/_includes/nav_pagination.liquid

I'm using "/{{ paginator.previous_index_permalink }}" in href.
Before #631, when serving the blog, I got http://localhost:3000/all/_p/2/ as link.
After: all/_p/2/ which leads nowhere.

So basically, I broke it…
Opening this issue to fix it ASAP :)

@Geobert
Copy link
Contributor Author

Geobert commented Aug 8, 2019

Hm, it seems that I added a leading / so with my leading / in the href it breaks the link.

I don't know if it's a bug then…

@Geobert
Copy link
Contributor Author

Geobert commented Aug 8, 2019

according to #622 the leading slash is expected.

@Geobert Geobert closed this as completed Aug 8, 2019
@Geobert
Copy link
Contributor Author

Geobert commented Aug 9, 2019

After a night of sleep, maybe even if we agreed on this result, the leading slash in paginator.previous_index_permalink is not a good idea for consistency sake:

for my about page, I have href="/about", for next/previous post we have href=/{{page.previous.permalink}}.

So it's a bit odd I have to remove the slash in href for pagination case.

I'll fix that ASAP :)

@Geobert Geobert reopened this Aug 9, 2019
@epage
Copy link
Member

epage commented Aug 9, 2019

Yes, the main thing is to be consistent with regular pagination links.

@epage epage closed this as completed in 9621b31 Aug 9, 2019
epage added a commit that referenced this issue Aug 9, 2019
fix(pagination_permalink): fixes #666 and adjust tests
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

No branches or pull requests

2 participants