blakeembrey / metalsmith-pagination

A Metalsmith plugin for paginating arrays and collections
MIT License
34 stars 10 forks source link

Why is page 1 generated twice when options.first is set? #5

Closed leonderijke closed 9 years ago

leonderijke commented 9 years ago

As can be seen in the tests, when the first option is set, a firstPage and a pageOne are created:

        var firstPage = files['articles/index.html']
        var pageOne = files['articles/page/1/index.html']
        (...)
        expect(firstPage).to.exist
        expect(firstPage).to.not.equal(pageOne)
        expect(firstPage.pagination.next).to.equal(pageTwo)
        expect(firstPage.pagination.previous).to.not.exist

        expect(pageOne).to.exist
        expect(pageOne.pagination.next).to.equal(pageTwo)
        expect(pageOne.pagination.previous).to.not.exist

The contents of firstPage and pageOne are the same, so I'm wondering: why is pageOne created?

blakeembrey commented 9 years ago

@leonderijke When I wrote this, it was mostly for consistency. Since firstPage could be anything, I wanted a way for people to navigate back to page 1 easily without having to find it. E.g. Just replace /page/2 with /page/1. So, personal preference. It may be worth adding canonical support for SEO cases, but do you have any other concerns?

leonderijke commented 9 years ago

SEO is indeed my primary concern. Although I find it a bit unusual to have two "page 1's". I'd prefer not generating pageOne when there is a firstPage over adding canonical support. What do you think?

blakeembrey commented 9 years ago

I'll add an option to disable pageOne, when you have a separate firstPage option enabled - it's easy enough to support.

blakeembrey commented 9 years ago

Added noPageOne option with v1.2.0