minimal-mistakes icon indicating copy to clipboard operation
minimal-mistakes copied to clipboard

Add proper support for sverrirs' Jekyll Paginate V2 plugin (excl. AutoPages)

Open iBug opened this issue 5 years ago • 19 comments
trafficstars

Add full support for the Jekyll Paginate V2 plugin.

The original paginator.html has been renamed to paginator-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 for site.paginate (V1 and V2 in compatible mode), and then site.pagination.enabled (true V2).

The new V2 template fully utilizes extra attributes available for the paginator object. There's no site.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 V2 paginator 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 in site.pagination.permalink, so I had to strip it off, or else it'll produce page2index.html for a specified value of page:num, causing 404. This is, however, error-prone, should any user choose to have index.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 and paginator.next_page_path were employed wherever possible. These two attributes have existed for long.

iBug avatar Aug 01 '20 14:08 iBug

Are there any test files you could add to /test so I can easily see the results of a jekyll-paginate index.html vs. a fully supported jekyll-paginate-v2 index.html?

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...

mmistakes avatar Aug 04 '20 14:08 mmistakes

I don't think there's anything to add. The current code (paginator.html) detects and adapts the plugins automatically as described. All you need to change in order to preview the V2 paginator is _config.yml.

To use V2, remove paginate and paginate_path and add the following config:

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.

iBug avatar Aug 04 '20 15:08 iBug

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.

mmistakes avatar Aug 04 '20 16:08 mmistakes

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?)

iBug avatar Aug 04 '20 16:08 iBug

This issue has been automatically marked as stale because it has not had recent activity.

If this is a bug and you can still reproduce this error on the master branch, please reply with any additional information you have about it in order to keep the issue open.

If this is a feature request, please consider whether it can be accomplished in another way. If it cannot, please elaborate on why it is core to this project and why you feel more than 80% of users would find this beneficial.

This issue will automatically be closed in 7 days if no further activity occurs. Thank you for all your contributions.

stale[bot] avatar Sep 05 '20 20:09 stale[bot]

@mmistakes Any updates?

iBug avatar Sep 06 '20 03:09 iBug

This issue has been automatically marked as stale because it has not had recent activity.

If this is a bug and you can still reproduce this error on the master branch, please reply with any additional information you have about it in order to keep the issue open.

If this is a feature request, please consider whether it can be accomplished in another way. If it cannot, please elaborate on why it is core to this project and why you feel more than 80% of users would find this beneficial.

This issue will automatically be closed in 7 days if no further activity occurs. Thank you for all your contributions.

stale[bot] avatar Oct 12 '20 01:10 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity.

If this is a bug and you can still reproduce this error on the master branch, please reply with any additional information you have about it in order to keep the issue open.

If this is a feature request, please consider whether it can be accomplished in another way. If it cannot, please elaborate on why it is core to this project and why you feel more than 80% of users would find this beneficial.

This issue will automatically be closed in 7 days if no further activity occurs. Thank you for all your contributions.

stale[bot] avatar Dec 25 '20 14:12 stale[bot]

@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.

iBug avatar Feb 07 '21 16:02 iBug

This pull request has been automatically marked as stale because it has not had recent activity.

This pull request will automatically be closed in 7 days if no further activity occurs. Thank you for all your contributions.

github-actions[bot] avatar May 13 '21 02:05 github-actions[bot]

iBug avatar May 22 '21 03:05 iBug

As you can see at here

https://github.com/mmistakes/minimal-mistakes/blob/89de4d46db3b7abf08748eeb41e24c34c935f4bf/_includes/paginator.html#L36-L48

The property site.paginate_path is still used in paginator.

I believe that we still need add that even in V2 if I didn't miss something else.

Matt-V50 avatar Jul 30 '21 05:07 Matt-V50

@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?

iBug avatar Aug 01 '21 05:08 iBug

I found that the page url is point to 404

I have finish this by using _config.yml below

# jykell Paginate 
paginate_path: '/blog/:num/'
pagination: # jekyll-paginate-v2
  enabled: true
  debug: true  # open debug
  per_page: 5
  permalink: '/blog/:num/'
  sort_field: 'date'
  sort_reverse: true
  trail:
    before: 2
    after: 2

Matt-V50 avatar Aug 01 '21 06:08 Matt-V50

@FavorMylikes

This PR was never merged so what's in the repo still targets only Jekyll Paginate (no V2).

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.

iBug avatar Aug 01 '21 06:08 iBug

Ok. Thank you for your example, I'll try to implement a version.

Matt-V50 avatar Aug 01 '21 06:08 Matt-V50

@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 3a065dcc59d8db687d26ee1407a93850ff12a81a.

iBug avatar Aug 01 '21 07:08 iBug

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?

iBug avatar May 29 '22 14:05 iBug

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.

iBug avatar Apr 22 '24 15:04 iBug

I can't even get the basic pagination working by following the instructions for paginate v2 in the document. I updated _config.yml and added a _posts/index.html. Nothing happens - the home page displays all the posts I ever created, and /posts returns 404. The collections pagination instructions are confusing and I've opened https://github.com/mmistakes/minimal-mistakes/issues/4982 to get it addressed.

asarkar avatar Sep 17 '24 05:09 asarkar