Remove duplicated pagination class for Bootstrap

[gnclmorais]

The .pagination class at Bootstrap adds margin at the top and at the bottom of the element it is on. Looking at the current examples they have on their documentation page, only one element has this class and it’s an inner one, not the parent <nav>.

While working on one of our projects, I noticed that Pagy had way more vertical margins than it should have, so I noticed its Bootstrap helper was adding the class twice on two different elements. Since I believe the markup it outputs should be as close as possible to Bootstrap’s documentation, the class added to the <nav> element feels out of place.

[codecov]

Codecov Report

Merging #212 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #212   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          26     26           
  Lines         530    530           
=====================================
  Hits          530    530

Impacted Files	Coverage Δ
lib/pagy/extras/bootstrap.rb	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee22c98...3cb0926. Read the comment docs.

[ddnexus]

Thank you for the explanation and the PR.
That happens also in the helpers, not only in the templates. Since you are the author of the PR, please, let me know if you are going to change the helpers and tests too, or if you want me to make that changes. Thanks.

[gnclmorais]

I’m happy to do it, @ddnexus — thanks for answering! I’ll get to in next week and ping you back. 🏁

[ddnexus]

@gnclmorais Any update?

[gnclmorais]

Hey @ddnexus, sorry for the delay. 😅 I’ve removed the pagination class on the <nav> on the Bootstrap helpers, thanks for noticing it. I’ve also removed it from the pagy-bootstrap-nav-js helpers. Let me know if I can be of assistance. 👌🏻

[ddnexus]

Thank you @gnclmorais. I will review and merge it ASAP.

[thedanbob]

Fun fact, the extra pagination class had the side effect of making pagy_bootstrap_nav work on bootstrap 2. In case I'm not the only one using this gem in a legacy bootstrap site (I'll update one of these days!) here's an implementation of pagy_nav that will work instead:

  def pagy_nav(pagy)
    link, p_prev, p_next = pagy_link_proc(pagy), pagy.prev, pagy.next

    html = EMPTY + (p_prev ? %(<li>#{link.call p_prev, pagy_t('pagy.nav.prev'), 'aria-label="previous"'}</li> )
                            : %(<li class="disabled"><a href="#">#{pagy_t('pagy.nav.prev')}</a></li> ))
    pagy.series.each do |item|
      html << if    item.is_a?(Integer); %(<li>#{link.call item}</li> )
              elsif item.is_a?(String) ; %(<li class="active"><a href="#">#{item}</a></li> )
              elsif item == :gap       ; %(<li class="disabled"><a href="#">#{pagy_t('pagy.nav.gap')}</a></li> )
              end
    end
    html << (p_next ? %(<li>#{link.call p_next, pagy_t('pagy.nav.next'), 'aria-label="next"'}</li>)
                    : %(<li class="disabled"><a href="#">#{pagy_t('pagy.nav.next')}</a></li>))
    %(<div class="pagination" role="navigation" aria-label="pager"><ul>#{html}</ul></div>)
  end

[ddnexus]

Now that you mention it, I remember why I put it there!
I will try to find an easy way to make it compatible with all versions.