Skip to content

Commit

Permalink
Refactoring of nav aria attributes, and nav styles tests
Browse files Browse the repository at this point in the history
  • Loading branch information
ddnexus committed Jan 26, 2024
1 parent 8d16f47 commit 618b7ca
Show file tree
Hide file tree
Showing 29 changed files with 1,574 additions and 1,713 deletions.
4 changes: 1 addition & 3 deletions e2e/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@ export default defineConfig(
setupNodeEvents(on, config) {
htmlvalidate.install(on, {
rules: {
// a few frameworks use ul or div for pagination, and aria-role="navigation" will trigger it
// a few frameworks use improper elements to render various roles
"prefer-native-element": "off",
// not needed in test environment
"require-sri": "off",
// the ARIA refactoring will fix that
"unique-landmark": "off"
}
});
},
Expand Down
20 changes: 13 additions & 7 deletions e2e/pagy_app.ru
Original file line number Diff line number Diff line change
Expand Up @@ -151,19 +151,21 @@ __END__
<hr>

<p><%= "pagy_#{name_fragment}nav" %></p>
<%= send(:"pagy_#{name_fragment}nav", @pagy, pagy_id: 'nav') %>
<%= send(:"pagy_#{name_fragment}nav", @pagy, pagy_id: 'nav', page_label: 'Pages nav') %>
<hr>

<p><%= "pagy_#{name_fragment}nav_js" %></p>
<%= send(:"pagy_#{name_fragment}nav_js", @pagy, pagy_id: 'nav-js') %>
<%= send(:"pagy_#{name_fragment}nav_js", @pagy, pagy_id: 'nav-js', page_label: 'Pages nav_js') %>
<hr>

<p><%= "pagy_#{name_fragment}nav_js" %> (responsive)</p>
<%= send(:"pagy_#{name_fragment}nav_js", @pagy, pagy_id: 'nav-js-responsive', steps: { 0 => [1,3,3,1], 600 => [2,4,4,2], 900 => [3,4,4,3] }) %>
<%= send(:"pagy_#{name_fragment}nav_js", @pagy, pagy_id: 'nav-js-responsive',
page_label: 'Pages nav_js_responsive',
steps: { 0 => [1,3,3,1], 600 => [2,4,4,2], 900 => [3,4,4,3] }) %>
<hr>

<p><%= "pagy_#{name_fragment}combo_nav_js" %></p>
<%= send(:"pagy_#{name_fragment}combo_nav_js", @pagy, pagy_id: 'combo-nav-js') %>
<%= send(:"pagy_#{name_fragment}combo_nav_js", @pagy, pagy_id: 'combo-nav-js', page_label: 'Pages combo_nav_js') %>
<hr>


Expand All @@ -177,13 +179,17 @@ __END__
<hr>

<p><%= "pagy_#{name_fragment}nav" %></p>
<%= send(:"pagy_#{name_fragment}nav", @calendar[:month], pagy_id: 'nav') %>
<%= send(:"pagy_#{name_fragment}nav", @calendar[:month], pagy_id: 'nav',
page_label: 'Pages nav') %>
<hr>

<p><%= "pagy_#{name_fragment}nav_js" %></p>
<%= send(:"pagy_#{name_fragment}nav_js", @calendar[:month], pagy_id: 'nav-js') %>
<%= send(:"pagy_#{name_fragment}nav_js", @calendar[:month], pagy_id: 'nav-js',
page_label: 'Pages nav_js') %>
<hr>

<p><%= "pagy_#{name_fragment}nav_js" %> (responsive)</p>
<%= send(:"pagy_#{name_fragment}nav_js", @calendar[:month], pagy_id: 'nav-js-responsive', steps: { 0 => [1,3,3,1], 600 => [2,4,4,2], 900 => [3,4,4,3] }) %>
<%= send(:"pagy_#{name_fragment}nav_js", @calendar[:month], pagy_id: 'nav-js-responsive',
page_label: 'Pages combo_nav_js',
steps: { 0 => [1,3,3,1], 600 => [2,4,4,2], 900 => [3,4,4,3] }) %>
<hr>
994 changes: 497 additions & 497 deletions e2e/snapshots.js

Large diffs are not rendered by default.

22 changes: 12 additions & 10 deletions lib/pagy/extras/bootstrap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,18 @@ def pagy_bootstrap_nav(pagy, pagy_id: nil, link_extra: '',
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" #{pagy_aria_label(pagy, page_label, page_i18n_key)}>) # rubocop:disable Layout/MultilineOperationIndentation
html = +%(<nav#{p_id} class="pagy-bootstrap-nav" #{
pagy_aria_label(pagy, page_label, page_i18n_key)}><ul class="pagination">)
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>)
%(<li class="page-item">#{link.call(item)}</li>)
when String
%(<li class="page-item active" aria-current="page">#{link.call item}</li>)
%(<li class="page-item active">#{link.call(item)}</li>)
when :gap
%(<li class="page-item gap disabled"><a href="#" class="page-link">#{pagy_t 'pagy.nav.gap'}</a></li>)
%(<li class="page-item gap disabled"><a href="#" class="page-link" aria-disabled="true">#{
pagy_t 'pagy.nav.gap'}</a></li>)
else raise InternalError, "expected item types in series to be Integer, String or :gap; got #{item.inspect}"
end
end
Expand All @@ -40,7 +41,8 @@ def pagy_bootstrap_nav_js(pagy, pagy_id: nil, link_extra: '',
tags = { 'before' => %(<ul class="pagination">#{pagy_bootstrap_prev_html pagy, link}),
'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>),
'gap' => %(<li class="page-item gap disabled"><a href="#" class="page-link" aria-disabled="true">#{
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" #{
Expand All @@ -64,16 +66,16 @@ def pagy_bootstrap_combo_nav_js(pagy, pagy_id: nil, link_extra: '',
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"'
link.call p_prev, pagy_t('pagy.nav.prev'), 'class="prev btn btn-primary"'
else
%(<a class="prev btn btn-primary disabled" href="#">#{pagy_t('pagy.nav.prev')}</a>)
%(<a class="prev btn btn-primary disabled" href="#" aria-disabled="true">#{pagy_t('pagy.nav.prev')}</a>)
end
}<div class="pagy-combo-input btn btn-secondary" style="white-space: nowrap;">#{
pagy_t 'pagy.combo_nav_js', page_input: input, count: p_page, pages: p_pages}</div>#{
if (p_next = pagy.next)
link.call p_next, pagy_t('pagy.nav.next'), 'aria-label="next" class="next btn btn-primary"'
link.call p_next, pagy_t('pagy.nav.next'), 'class="next btn btn-primary"'
else
%(<a class="next btn btn-primary disabled" href="#">#{pagy_t 'pagy.nav.next'}</a>)
%(<a class="next btn btn-primary disabled" href="#" aria-disabled="true">#{pagy_t 'pagy.nav.next'}</a>)
end
}</div></nav>)
end
Expand Down
45 changes: 23 additions & 22 deletions lib/pagy/extras/bulma.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,21 @@ class Pagy # :nodoc:
# The resulting code may not look very elegant, but produces the best benchmarks
module BulmaExtra
# Pagination for bulma: it returns the html with the series of links to the pages
def pagy_bulma_nav(pagy, pagy_id: nil, link_extra: '', **vars)
def pagy_bulma_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:)

html = +%(<nav#{p_id} class="pagy-bulma-nav pagination is-centered" aria-label="pagination">)
html = +%(<nav#{p_id} class="pagy-bulma-nav pagination is-centered" #{
pagy_aria_label(pagy, page_label, page_i18n_key)}>)
html << pagy_bulma_prev_next_html(pagy, link)
html << %(<ul class="pagination-list">)
pagy.series(**vars).each do |item| # series example: [1, :gap, 7, 8, "9", 10, 11, :gap, 36]
html << case item
when Integer
%(<li>#{link.call item, pagy.label_for(item), %(class="pagination-link" aria-label="goto page #{item}")}</li>)
%(<li>#{link.call(item, pagy.label_for(item), %(class="pagination-link"))}</li>)
when String
%(<li>#{link.call item, pagy.label_for(item),
%(class="pagination-link is-current" aria-label="page #{item}" aria-current="page")}</li>)
%(<li>#{link.call(item, pagy.label_for(item), %(class="pagination-link is-current"))}</li>)
when :gap
%(<li><span class="pagination-ellipsis">#{pagy_t 'pagy.nav.gap'}</span></li>)
else raise InternalError, "expected item types in series to be Integer, String or :gap; got #{item.inspect}"
Expand All @@ -31,46 +32,46 @@ def pagy_bulma_nav(pagy, pagy_id: nil, link_extra: '', **vars)
end

# Javascript pagination for bulma: it returns a nav and a JSON tag used by the Pagy.nav javascript
def pagy_bulma_nav_js(pagy, pagy_id: nil, link_extra: '', **vars)
def pagy_bulma_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:)
tags = { 'before' => %(#{pagy_bulma_prev_next_html(pagy, link)}<ul class="pagination-list">),
'link' => %(<li>#{link.call PAGE_PLACEHOLDER, LABEL_PLACEHOLDER,
%(class="pagination-link" aria-label="goto page #{PAGE_PLACEHOLDER}")}</li>),
'active' => %(<li>#{link.call PAGE_PLACEHOLDER, LABEL_PLACEHOLDER,
%(class="pagination-link is-current" aria-current="page" aria-label="page #{
PAGE_PLACEHOLDER}")}</li>),
'link' => %(<li>#{link.call(PAGE_PLACEHOLDER, LABEL_PLACEHOLDER, %(class="pagination-link"))}</li>),
'active' => %(<li>#{link.call(PAGE_PLACEHOLDER, LABEL_PLACEHOLDER, %(class="pagination-link is-current"))}</li>),
'gap' => %(<li><span class="pagination-ellipsis">#{pagy_t 'pagy.nav.gap'}</span></li>),
'after' => '</ul>' }

%(<nav#{p_id} class="#{'pagy-rjs ' if sequels.size > 1}pagy-bulma-nav-js pagination is-centered" aria-label="pagination" #{
%(<nav#{p_id} class="#{'pagy-rjs ' if sequels.size > 1}pagy-bulma-nav-js pagination is-centered" #{
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 bulma: it returns a nav and a JSON tag used by the pagy.js file
def pagy_bulma_combo_nav_js(pagy, pagy_id: nil, link_extra: '')
def pagy_bulma_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
p_pages = pagy.pages
input = %(<input class="input" type="number" min="1" max="#{p_pages}" value="#{
p_page}" style="padding: 0; text-align: center; width: #{p_pages.to_s.length + 1}rem; margin:0 0.3rem;">)

html = %(<nav#{p_id} class="pagy-bulma-combo-nav-js" aria-label="pagination">)
html = %(<nav#{p_id} class="pagy-bulma-combo-nav-js" #{pagy_aria_label(pagy, page_label, page_i18n_key)}>)
%(#{html}<div class="field is-grouped is-grouped-centered" role="group" #{
pagy_data(pagy, :combo, pagy_marked_link(link))}>#{
if (p_prev = pagy.prev)
%(<p class="control">#{link.call p_prev, pagy_t('pagy.nav.prev'), 'class="button" aria-label="previous page"'}</p>)
%(<p class="control">#{link.call p_prev, pagy_t('pagy.nav.prev'), 'class="button"'}</p>)
else
%(<p class="control"><a class="button" disabled>#{pagy_t 'pagy.nav.prev'}</a></p>)
%(<p class="control"><a class="button" disabled aria-disabled="true">#{pagy_t 'pagy.nav.prev'}</a></p>)
end
}<div class="pagy-combo-input control level is-mobile">#{
pagy_t 'pagy.combo_nav_js', page_input: input, count: p_page, pages: p_pages}</div>#{
if (p_next = pagy.next)
%(<p class="control">#{link.call p_next, pagy_t('pagy.nav.next'), 'class="button" aria-label="next page"'}</p>)
%(<p class="control">#{link.call p_next, pagy_t('pagy.nav.next'), 'class="button"'}</p>)
else
%(<p class="control"><a class="button" disabled>#{pagy_t 'pagy.nav.next'}</a></p>)
%(<p class="control"><a class="button" disabled aria-disabled="true">#{pagy_t 'pagy.nav.next'}</a></p>)
end
}</div></nav>)
end
Expand All @@ -79,14 +80,14 @@ def pagy_bulma_combo_nav_js(pagy, pagy_id: nil, link_extra: '')

def pagy_bulma_prev_next_html(pagy, link)
html = +if (p_prev = pagy.prev)
link.call p_prev, pagy_t('pagy.nav.prev'), 'class="pagination-previous" aria-label="previous page"'
link.call p_prev, pagy_t('pagy.nav.prev'), 'class="pagination-previous"'
else
%(<a class="pagination-previous" disabled>#{pagy_t 'pagy.nav.prev'}</a>)
%(<a class="pagination-previous" disabled aria-disabled="true">#{pagy_t 'pagy.nav.prev'}</a>)
end
html << if (p_next = pagy.next)
link.call p_next, pagy_t('pagy.nav.next'), 'class="pagination-next" aria-label="next page"'
link.call p_next, pagy_t('pagy.nav.next'), 'class="pagination-next"'
else
%(<a class="pagination-next" disabled>#{pagy_t 'pagy.nav.next'}</a>)
%(<a class="pagination-next" disabled aria-disabled="true">#{pagy_t 'pagy.nav.next'}</a>)
end
end
end
Expand Down
Loading

0 comments on commit 618b7ca

Please sign in to comment.