mgwidmann / scrivener_html

HTML view helpers for Scrivener
MIT License
125 stars 208 forks source link

all links in pagination marked as canonical except for next, prev #92

Open StarWar opened 5 years ago

StarWar commented 5 years ago

The library marks all page links except for next & prev as canonical. Which I think is wrong? It should only set the current page as canonical and make it a disabled link. However, this lib makes the current link a simple li element but tags all other links as canonical.

Currently, it outputs the following links:- I'm on the page 4. However, it doesn't mark page 4 as canonical. It marks page 1 and 2 as canonical and page 3 as prev.

<ul class="pagination">
<li class="waves-effect"><a class="" href="/categories/health?page=3" rel="prev"><i class="material-icons">chevron_left</i></a></li>
<li class="waves-effect"><a class="" href="/categories/health" rel="canonical">1</a></li>
<li class="waves-effect"><a class="" href="/categories/health?page=2" rel="canonical">2</a></li>
<li class="waves-effect"><a class="" href="/categories/health?page=3" rel="prev">3</a></li>
<li class="active"><a class="">4</a></li></ul>

The library should also provide a method so that we can add next, prev and canonical links in the head using rel links or provide a feature to disable canonical tag marking in pagination links. the next and prev are marked correctly. However, I think marking every other link as canonical in the pagination is wrong. Some SEO expert may explain or share more informed opinion regarding it. This is a nice library and I'm planning to use in my first elixir project. My opinion is based on how Kaminari (a ruby gem) works.

mgwidmann commented 4 years ago

If this is a bug as you describe, it should be reproducible in a unit test. As such, if you can open a PR I would be willing to accept it if it proved the existence of said bug.