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

feat(pagination): added pagination urls to sitemap #19

Merged
merged 3 commits into from Apr 7, 2021

Conversation

rq-abrahamsson
Copy link
Contributor

Needed to index all pages created by pagination and this is how I did it. Could maybe be useful for someone else.

@nunof07
Copy link
Member

nunof07 commented Apr 2, 2021

Hi @rq-abrahamsson and thanks!

Can you show an example of how this should be used?

@rq-abrahamsson
Copy link
Contributor Author

Hi @nunof07!

I'm not super experienced with Eleventy and have not written any plugin for it before so I hope my explanation will make sense.

But the use case I had was that I get data for a list of football pitches and I want to create one page for each with the same template so I did that with pagination and set size to 1. With the example data below this will create two pages and I want to index them both. So when using this plugin for generating a sitemap only the url for one of the generated pages was added which was not how I was expecting it to work. But maybe the current behavior is how it's expected to work for some?

pitches.html:

---
pagination:
  data: pitches
  size: 1
  alias: pitch
permalink: '{{ pitch.url }}'
layout: templates/layouts/base.liquid
---
<div class="main-content" >
  <h1>{{ pitch.name }}</h1>
</div>

pitches.json

[
  {
    "name": "Name of first pitch",
    "url": "/name-of-first-pitch/",
    ...
  },
  {
    "name": "Name of second pitch",
    "url": "/name-of-second-pitch/",
    ...
  }
]

@nunof07
Copy link
Member

nunof07 commented Apr 2, 2021

Thanks for the explanation. Hadn't tried this use case yet. Makes sense that the sitemap should include all paginated items.

I will setup an example with pagination and run some tests to make sure everything is covered.

Thanks again, appreciate it!

@nunof07 nunof07 merged commit b999f8f into quasibit:master Apr 7, 2021
@github-actions
Copy link

github-actions bot commented Apr 7, 2021

🎉 This PR is included in version 2.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nunof07
Copy link
Member

nunof07 commented Apr 7, 2021

Hi @rq-abrahamsson ,

I've made a few changes to your PR. Added a pagination example and refactored the functions a bit.

Now calls the flatMap before the ignore check, so paginated items can also be ignored and spreads the full paginated page object and not just the url property, so we can pass down other properties.

Thanks again and let me know if you find any issue with it!

@BrianMitchL
Copy link

BrianMitchL commented Apr 18, 2021

I use pagination to generate a tag page for each occurrence of a tag on my site. After updating to v2.1.0 which includes this, the sitemap is adding a <url> entry for each tag, each time a new tag is found. For example, I have 13 tags right now, so there are 13 duplicate <url> entries for each tag. I'm not entirely sure what's going on here, but we might want to filter out duplicates if the URLs are identical? This could be because I set addAllPagesToCollections to true in the pagination configuration as well, so the tags were already being pulled in v2.0.5.

Poking around even more, v2.1.0 adds multiple entires for all of my /posts/ paginated pages, but doesn't include the page number, which results in duplicate entries as well. In v2.0.5 there was only one /posts/ entry, which seems to be what this PR was intending on fixing. Any thoughts with this? I'd be happy to poke at it some more, but this broke two different pagination configs on my site, so I thought I'd at least call it out 🙂

@nunof07
Copy link
Member

nunof07 commented Apr 18, 2021

Hi @BrianMitchL , thank you for reporting this.
Regarding the tags, it seems that it is due to addAllPagesToCollections like you said. Wondering if that would have been the best solution from the start to include paginated items in the sitemap instead of introducing a change 🤔, but now it's probably better to ignore duplicates.
For the posts I have to check why the page numbers are not being included.
I'll look into it this week. Sorry for the troubles 😅.

@BrianMitchL
Copy link

No worries! Let me know if you want me to test anything out (commenting here, or tagging me in a new PR or something), or try to help with a fix.

This was referenced Apr 19, 2021
@nunof07
Copy link
Member

nunof07 commented Apr 19, 2021

Hi @BrianMitchL , I pushed a couple of fixes for both issues (v2.1.3). Let me know if you still have any issues. Thanks!

@BrianMitchL
Copy link

@nunof07 v2.1.3 looks great!! 🚀 👏

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

Successfully merging this pull request may close these issues.

None yet

3 participants