Add ARIA attributes to bootstrap helpers (closes #612); refactor nav …

…tests

lib/pagy/extras/bootstrap.rb

@@ -8,18 +8,20 @@ class Pagy # :nodoc:
   # The resulting code may not look very elegant, but produces the best benchmarks
   module BootstrapExtra
     # Pagination for bootstrap: it returns the html with the series of links to the pages
-    def pagy_bootstrap_nav(pagy, pagy_id: nil, link_extra: '', **vars)
+    def pagy_bootstrap_nav(pagy, pagy_id: nil, link_extra: '',
+                           page_label: nil, page_i18n_key: nil, **vars)
       p_id = %( id="#{pagy_id}") if pagy_id
       link = pagy_link_proc(pagy, link_extra: %(class="page-link" #{link_extra}))
 
-      html = +%(<nav#{p_id} class="pagy-bootstrap-nav"><ul class="pagination">)
+      html = +%(<nav#{p_id} class="pagy-bootstrap-nav">) +
+              %(<ul class="pagination" #{pagy_aria_label(pagy, page_label, page_i18n_key)}>) # rubocop:disable Layout/MultilineOperationIndentation
       html << pagy_bootstrap_prev_html(pagy, link)
       pagy.series(**vars).each do |item| # series example: [1, :gap, 7, 8, "9", 10, 11, :gap, 36]
         html << case item
                 when Integer
                   %(<li class="page-item">#{link.call item}</li>)
                 when String
-                  %(<li class="page-item active">#{link.call item}</li>)
+                  %(<li class="page-item active" aria-current="page">#{link.call item}</li>)
                 when :gap
                   %(<li class="page-item gap disabled"><a href="#" class="page-link">#{pagy_t 'pagy.nav.gap'}</a></li>)
                 else raise InternalError, "expected item types in series to be Integer, String or :gap; got #{item.inspect}"
@@ -30,22 +32,26 @@ def pagy_bootstrap_nav(pagy, pagy_id: nil, link_extra: '', **vars)
     end
 
     # Javascript pagination for bootstrap: it returns a nav and a JSON tag used by the pagy.js file
-    def pagy_bootstrap_nav_js(pagy, pagy_id: nil, link_extra: '', **vars)
+    def pagy_bootstrap_nav_js(pagy, pagy_id: nil, link_extra: '',
+                              page_label: nil, page_i18n_key: nil, **vars)
       sequels = pagy.sequels(**vars)
       p_id = %( id="#{pagy_id}") if pagy_id
       link = pagy_link_proc(pagy, link_extra: %(class="page-link" #{link_extra}))
       tags = { 'before' => %(<ul class="pagination">#{pagy_bootstrap_prev_html pagy, link}),
-               'link'   => %(<li class="page-item">#{mark = link.call(PAGE_PLACEHOLDER, LABEL_PLACEHOLDER)}</li>),
-               'active' => %(<li class="page-item active">#{mark}</li>),
+               'link'   => %(<li class="page-item">#{html = link.call(PAGE_PLACEHOLDER, LABEL_PLACEHOLDER)}</li>),
+               'active' => %(<li class="page-item active">#{html}</li>),
                'gap'    => %(<li class="page-item gap disabled"><a href="#" class="page-link">#{pagy_t 'pagy.nav.gap'}</a></li>),
                'after'  => %(#{pagy_bootstrap_next_html pagy, link}</ul>) }
 
       %(<nav#{p_id} class="#{'pagy-rjs ' if sequels.size > 1}pagy-bootstrap-nav-js" #{
-        pagy_data(pagy, :nav, tags, sequels, pagy.label_sequels(sequels))}></nav>)
+          pagy_aria_label(pagy, page_label, page_i18n_key)} #{
+          pagy_data(pagy, :nav, tags, sequels, pagy.label_sequels(sequels))
+        }></nav>)
     end
 
     # Javascript combo pagination for bootstrap: it returns a nav and a JSON tag used by the pagy.js file
-    def pagy_bootstrap_combo_nav_js(pagy, pagy_id: nil, link_extra: '')
+    def pagy_bootstrap_combo_nav_js(pagy, pagy_id: nil, link_extra: '',
+                                    page_label: nil, page_i18n_key: nil)
       p_id    = %( id="#{pagy_id}") if pagy_id
       link    = pagy_link_proc(pagy, link_extra:)
       p_page  = pagy.page
@@ -54,7 +60,8 @@ def pagy_bootstrap_combo_nav_js(pagy, pagy_id: nil, link_extra: '')
                     p_page}" style="padding: 0; border: none; text-align: center; width: #{
                     p_pages.to_s.length + 1}rem;">)
 
-      %(<nav#{p_id} class="pagy-bootstrap-combo-nav-js pagination"><div class="btn-group" role="group" #{
+      %(<nav#{p_id} class="pagy-bootstrap-combo-nav-js pagination" #{
+          pagy_aria_label(pagy, page_label, page_i18n_key)}><div class="btn-group" role="group" #{
           pagy_data(pagy, :combo, pagy_marked_link(link))}>#{
           if (p_prev = pagy.prev)
             link.call p_prev, pagy_t('pagy.nav.prev'), 'aria-label="previous" class="prev btn btn-primary"'
@@ -75,17 +82,21 @@ def pagy_bootstrap_combo_nav_js(pagy, pagy_id: nil, link_extra: '')
 
     def pagy_bootstrap_prev_html(pagy, link)
       if (p_prev = pagy.prev)
-        %(<li class="page-item prev">#{link.call p_prev, pagy_t('pagy.nav.prev'), 'aria-label="previous"'}</li>)
+        %(<li class="page-item prev">#{link.call p_prev, pagy_t('pagy.nav.prev')}</li>)
       else
-        %(<li class="page-item prev disabled"><a href="#" class="page-link">#{pagy_t 'pagy.nav.prev'}</a></li>)
+        %(<li class="page-item prev disabled"><a href="#" class="page-link" aria-disabled="true">#{
+            pagy_t 'pagy.nav.prev'
+          }</a></li>)
       end
     end
 
     def pagy_bootstrap_next_html(pagy, link)
       if (p_next = pagy.next)
-        %(<li class="page-item next">#{link.call p_next, pagy_t('pagy.nav.next'), 'aria-label="next"'}</li>)
+        %(<li class="page-item next">#{link.call p_next, pagy_t('pagy.nav.next')}</li>)
       else
-        %(<li class="page-item next disabled"><a href="#" class="page-link">#{pagy_t 'pagy.nav.next'}</a></li>)
+        %(<li class="page-item next disabled"><a href="#" class="page-link" aria-disabled="true">#{
+            pagy_t 'pagy.nav.next'
+          }</a></li>)
       end
     end
   end

lib/pagy/frontend.rb

@@ -58,12 +58,14 @@ def pagy_info(pagy, pagy_id: nil, item_name: nil, item_i18n_key: nil)
     def pagy_link_proc(pagy, link_extra: '')
       p_prev      = pagy.prev
       p_next      = pagy.next
+      p_page      = pagy.page
       left, right = %(<a href="#{pagy_url_for(pagy, PAGE_PLACEHOLDER)}" #{
                         pagy.vars[:link_extra]} #{link_extra}).split(PAGE_PLACEHOLDER, 2)
       lambda do |page, text = pagy.label_for(page), extra_attrs = ''|
         %(#{left}#{page}#{right}#{ case page
                                    when p_prev then ' rel="prev"'
                                    when p_next then ' rel="next"'
+                                   when p_page then ' aria-current="page"'
                                    else             ''
                                    end } #{extra_attrs}>#{text}</a>)
       end

test/helper/nav_tests.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module NavTests
+  def nav_check(method, pagy)  # e.g. pagy_bootstrap_nav
+    _(app.send(method, pagy)).must_rematch :plain
+    _(app.send(method, pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra')).must_rematch :extras
+  end
+
+  def nav_js_check(method, pagy)  # e.g. pagy_bootstrap_nav_js
+    _(app.send(method, pagy)).must_rematch :plain
+    _(app.send(method, pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra',
+                      steps: { 0 => [1, 2, 2, 1], 600 => [1, 3, 3, 1] })).must_rematch :extras
+  end
+
+  def combo_nav_js_check(method, pagy)  # e.g. pagy_bootstrap_combo_nav_js
+    _(app.send(method, pagy)).must_rematch :plain
+    _(app.send(method, pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra')).must_rematch :extras
+  end
+end

test/helper/warning_filters.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module WarningFilters
+  module Calendar
+    def warn(message, category: nil, **kwargs)
+      if message.match?('Calendar#page_at')
+        # ignore
+      else
+        super
+      end
+    end
+  end
+end
+Warning.extend WarningFilters::Calendar

test/pagy/extras/bootstrap_test.rb

@@ -8,22 +8,17 @@
 
 describe 'pagy/extras/bootstrap' do
   let(:app) { MockApp.new }
+  include NavTests
 
   describe '#pagy_bootstrap_nav' do
     it 'renders first page' do
-      pagy = Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 1)
-      _(app.pagy_bootstrap_nav(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_nav(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra')).must_rematch :extras
+      nav_check :pagy_bootstrap_nav, Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 1)
     end
     it 'renders intermediate page' do
-      pagy = Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 20)
-      _(app.pagy_bootstrap_nav(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_nav(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra')).must_rematch :extras
+      nav_check :pagy_bootstrap_nav, Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 20)
     end
     it 'renders last page' do
-      pagy = Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 50)
-      _(app.pagy_bootstrap_nav(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_nav(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra')).must_rematch :extras
+      nav_check :pagy_bootstrap_nav, Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 50)
     end
     it 'should raise for wrong series' do
       _ { app.pagy_bootstrap_nav(PagyBuggy.new(count: 100)) }.must_raise Pagy::InternalError
@@ -32,36 +27,21 @@
 
   describe '#pagy_bootstrap_nav_js' do
     it 'renders first page' do
-      pagy = Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 1)
-      _(app.pagy_bootstrap_nav_js(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_nav_js(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra',
-                                         steps: { 0 => [1, 2, 2, 1], 600 => [1, 3, 3, 1] })).must_rematch :extras
+      nav_js_check :pagy_bootstrap_nav_js, Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 1)
     end
     it 'renders first page when used with Pagy::Countless' do
       require 'pagy/extras/countless'
 
-      pagy, = Pagy::Countless.new(page: 1).finalize(0)
-      _(app.pagy_bootstrap_nav_js(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_nav_js(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra',
-                                          steps: { 0 => [1, 2, 2, 1], 600 => [1, 3, 3, 1] })).must_rematch :extras
+      nav_js_check :pagy_bootstrap_nav_js, Pagy::Countless.new(page: 1).finalize(0)
     end
     it 'renders intermediate page' do
-      pagy = Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 20)
-      _(app.pagy_bootstrap_nav_js(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_nav_js(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra',
-                                         steps: { 0 => [1, 2, 2, 1], 600 => [1, 3, 3, 1] })).must_rematch :extras
+      nav_js_check :pagy_bootstrap_nav_js, Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 20)
     end
     it 'renders last page' do
-      pagy = Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 50)
-      _(app.pagy_bootstrap_nav_js(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_nav_js(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra',
-                                         steps: { 0 => [1, 2, 2, 1], 600 => [1, 3, 3, 1] })).must_rematch :extras
+      nav_js_check :pagy_bootstrap_nav_js, Pagy.new(size: [1, 4, 4, 1], count: 1000, page: 50)
     end
     it 'renders with :steps' do
-      pagy = Pagy.new(count: 1000, page: 20, steps: { 0 => [1, 2, 2, 1], 500 => [2, 3, 3, 2] })
-      _(app.pagy_bootstrap_nav_js(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_nav_js(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra',
-                                         steps: { 0 => [1, 2, 2, 1], 600 => [1, 3, 3, 1] })).must_rematch :extras
+      nav_js_check :pagy_bootstrap_nav_js, Pagy.new(count: 1000, page: 20, steps: { 0 => [1, 2, 2, 1], 500 => [2, 3, 3, 2] })
     end
     it 'raises with missing step 0' do
       pagy = Pagy.new(count: 1000, page: 20, steps: { 0 => [1, 2, 2, 1], 600 => [1, 3, 3, 1] })
@@ -71,19 +51,13 @@
 
   describe '#pagy_bootstrap_combo_nav_js' do
     it 'renders first page' do
-      pagy = Pagy.new(count: 103, page: 1)
-      _(app.pagy_bootstrap_combo_nav_js(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_combo_nav_js(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra')).must_rematch :extras
+      combo_nav_js_check :pagy_bootstrap_combo_nav_js, Pagy.new(count: 103, page: 1)
     end
     it 'renders intermediate page' do
-      pagy = Pagy.new(count: 103, page: 3)
-      _(app.pagy_bootstrap_combo_nav_js(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_combo_nav_js(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra')).must_rematch :extras
+      combo_nav_js_check :pagy_bootstrap_combo_nav_js, Pagy.new(count: 103, page: 3)
     end
     it 'renders last page' do
-      pagy = Pagy.new(count: 103, page: 6)
-      _(app.pagy_bootstrap_combo_nav_js(pagy)).must_rematch :plain
-      _(app.pagy_bootstrap_combo_nav_js(pagy, pagy_id: 'test-nav-id', link_extra: 'link-extra')).must_rematch :extras
+      combo_nav_js_check :pagy_bootstrap_combo_nav_js, Pagy.new(count: 103, page: 6)
     end
   end
 end