-
Notifications
You must be signed in to change notification settings - Fork 25.6k
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
Add proper support for sverrirs' Jekyll Paginate V2 plugin (excl. AutoPages) #2636
Conversation
Are there any test files you could add to Messing with this makes me leary as there's no easy way for me to verify it won't break someones site. The old Jekyll paginate plugin has its quirks, but it's reliable. In my experience v2 has even more quirks that can be hard to work thru sometimes and want to be careful how we support it. Since this theme is so popular I'm already having nightmares about the GH issues rolling in due to misconfiguring the v2 plugin... |
I don't think there's anything to add. The current code ( To use V2, remove pagination:
enabled: true
collection: 'posts'
per_page: 5
permalink: '/page/:num/' # Pages are index.html inside this folder (default)
title: ':title - page :num'
limit: 0
sort_field: 'date'
sort_reverse: true
trail:
# Just to show it works
before: 1
after: 3 That's what I adapted for. For those who are using V1, they should not even see any visible change. |
OK. Could you amend the docs to include your preferred configs above for v2 pagination. I'll test v1 and v2 off this branch when I get a chance just to make sure everything looks good. |
I absolutely will do. I held off from updating the docs just in case you dislike this idea. Also I just noticed that I missed two configuration keys from V2: pagination:
# Optional, the default file extension for generated pages (e.g html, json, xml).
# Internally this is set to html by default
extension: html
# Optional, the default name of the index file for generated pages (e.g. 'index.html')
# Without file extension
indexpage: 'index' I haven't tested yet, but it seems easy to break if anyone sets them to anything other than the default values. Again this is like the farthest corner case that's highly impractical to me. (Shall we try to cover it?) |
[a152d46] That one failed, use own code from mmistakes/minimal-mistakes#2636
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
@mmistakes If you find it hard to test this out on the docs site, feel free to drop by ibug.io where this very code has been employed for years (not literally, just half a year) and hasn't broken for once. In fact, it's been running perfectly in two places:
I'll amend the documentation once I get the word that this is accepted. |
Jekyll Paginate V2 tries to keep the page trail length consistent for all pages, making the first and last few pages have a longer after/before trail than it normally would have. This breaks the previous calculation-based approach for determining the presense of an ellipsis. The new approach just checks if there are missing numbers between 1 and the first item in the page trail, and inserts an ellipsis if there is any. It now works perfectly for these cases.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
As you can see at here minimal-mistakes/_includes/paginator.html Lines 36 to 48 in 89de4d4
The property I believe that we still need add that even in |
@FavorMylikes I didn't get your point. This PR was never merged so what's in the repo still targets only Jekyll Paginate (no V2). Could you be a bit more detailed? |
I found that the I have finish this by using
|
To clarify: If you're using this theme with stock setup, you're not getting any Paginate V2 support. You'll have to incorporate what's presented in this PR into your repo by yourself. If you need an example, my website repo could give some insights. |
Ok. Thank you for your example, I'll try to implement a version. |
@mmistakes Any chance this PR gets another look? There seems like a non-trivial number of potential audience. Also I've updated the docs in 3a065dc. |
Perhaps we should think this in another way. Nothing comes out perfect, and it's a norm to have trial-and-error to a certain extent. I promise I'll be after this feature (Paginate V2 support) since it's my implementation and contribution, as well as that I'm a Paginate V2 user myself. You got me. Consider merging it in first and see what responses users have? |
Two years later I'm now in charge of this theme (thanks Michael!). This PR was (and still is) one of my most ambitious so it's definitely got attention. |
I can't even get the basic pagination working by following the instructions for paginate v2 in the document. I updated |
Add full support for the Jekyll Paginate V2 plugin.
The original
paginator.html
has been renamed topaginator-v1.html
, and a new "switcher" is placed with that name. The switcher detects whether the V1 template or the V2 template should be used, by first looking forsite.paginate
(V1 and V2 in compatible mode), and thensite.pagination.enabled
(true V2).The new V2 template fully utilizes extra attributes available for the
paginator
object. There's nosite.xxx
accessed except for UI text (l10n) which was left intact. The starting, ending, first page (and the ellipsis after), last page (and the ellipsis before), previous page, next page are all implemented using the V2paginator
attributes, making it super compatible to user configuration.There's a special note for the page trail. In my local testing, JPV2 adds
index.html
to whatever specified insite.pagination.permalink
, so I had to strip it off, or else it'll producepage2index.html
for a specified value ofpage:num
, causing 404. This is, however, error-prone, should any user choose to haveindex.html
somewhere in the middle of their pagination URL. I chose to ignore this as it's rather an anti-pattern.The V1 template only underwent a few small changes where
paginator.previous_page_path
andpaginator.next_page_path
were employed wherever possible. These two attributes have existed for long.