olefredrik / FoundationPress

FoundationPress is a WordPress starter theme based on Foundation 6 by Zurb
https://foundationpress.olefredrik.com
MIT License
2.7k stars 867 forks source link

foundationpress_pagination() - Invalid HTML in output, output doesn't match Foundation's docs #1371

Closed pixelbrad closed 5 years ago

pixelbrad commented 5 years ago

How can this bug be reproduced?

  1. Call foundationpress_pagination() in one of your templates.
  2. Navigate to a page that outputs foundationpress_pagination markup.
  3. View page source. You can quickly identify mis-matched opening and closing HTML tags.

What did you expect to happen?

I expected to see valid HTML markup.

What happened instead?

I saw invalid HTML markup.

Please List the Following:

Include Test Case (if applicable):
Not applicable, but see below for example of markup.

Invalid markup example Indented and commented markup for the sake of this example.

<ul class='pagination text-center' role='navigation' aria-label='Pagination'> <!-- Invalid role attribute -->
    <li><a class="prev" href="//localhost:3000/news-media/page/1/">&laquo;</a></li> <!-- Does not match Foundation's docs -->
    <li><a class='' href='//localhost:3000/news-media/page/1/'>1</a></li>
    <li><span aria-current='page' class=' current'>2</a></li> <!-- Mismatched open/close tag -->
    <li><a class='' href='//localhost:3000/news-media/page/3/'>3</a></li>
    <li><a class='' href='//localhost:3000/news-media/page/4/'>4</a></li>
    <li><a class='' href='//localhost:3000/news-media/page/5/'>5</a></li>
    <li><a class='' href='//localhost:3000/news-media/page/6/'>6</a></li>
    <li><span class='dots'>&hellip;</span></li> <!-- Does not match Foundation's docs -->
    <li><a class='' href='//localhost:3000/news-media/page/12/'>12</a></li>
    <li><a class="next" href="//localhost:3000/news-media/page/2/">&raquo;</a></li> <!-- Does not match Foundation's docs -->
</ul>
pixelbrad commented 5 years ago

I forked this repo and comitted some changes. Let's see if I can remember how to do a proper PR that references this issue.

pixelbrad commented 5 years ago

Markup example after this PR:

<nav aria-label="Pagination">
    <ul class="pagination text-center">
        <li class="previous"><a href="//localhost:3000/news-media/page/1/" aria-label="Previous page"></a></li>
        <li><a href="//localhost:3000/news-media/page/1/" aria-label="Page 1">1</a>
        <li class="current" aria-current="page"><span class="show-for-sr">You're on page </span>2</li>
        <li><a href="//localhost:3000/news-media/page/3/" aria-label="Page 3">3</a>
        <li><a href="//localhost:3000/news-media/page/4/" aria-label="Page 4">4</a>
        <li><a href="//localhost:3000/news-media/page/5/" aria-label="Page 5">5</a>
        <li><a href="//localhost:3000/news-media/page/6/" aria-label="Page 6">6</a>
        <li><a href="//localhost:3000/news-media/page/7/" aria-label="Page 7">7</a>
        <li class="ellipsis" aria-hidden="true"></li>
        <li><a href="//localhost:3000/news-media/page/12/" aria-label="Page 12">12</a>
        <li class="next"><a href="//localhost:3000/news-media/page/3/" aria-label="Next page"></a></li>
    </ul>
</nav>
strikemike2k commented 5 years ago

@pixelbrad did a great job enhancing the syntax for this function. The PR below is a solution that I implemented to quickly fix this issue.

Markup example after PR https://github.com/olefredrik/FoundationPress/pull/1380:

<ul class='pagination text-center' role='navigation' aria-label='Pagination'>
  <li aria-current='page' class='current'>1</li>
  <li><a class='' href='//localhost/lifestyle/page/2/'>2</a></li>
  <li><a class='' href='//localhost/lifestyle/page/3/'>3</a></li>
  <li><a class='' href='//localhost/lifestyle/page/4/'>4</a></li>
  <li><a class='' href='//localhost/lifestyle/page/5/'>5</a></li>
  <li><a class='' href='//localhost/lifestyle/page/6/'>6</a></li>
  <li><span class='dots'>&hellip;</span></li>
  <li><a class='' href='//localhost/lifestyle/page/15/'>15</a></li>
  <li><a class="next" href="//localhost/lifestyle/page/2/">&raquo;</a></li>
</ul>