From 7d1a57329cda0b11b2aad416ed9d39255b34a479 Mon Sep 17 00:00:00 2001 From: Domizio Demichelis Date: Sun, 24 Jun 2018 12:16:22 +0200 Subject: [PATCH] internal consistency renaming of local/test variables; minor fixes and improvements --- docs/extras/responsive.md | 2 +- lib/pagy/extras/bootstrap.rb | 11 ++- lib/pagy/extras/compact.rb | 35 +++++----- lib/pagy/extras/i18n.rb | 1 - lib/pagy/extras/items.rb | 12 ++-- lib/pagy/extras/javascripts/pagy.js | 102 ++++++++++++---------------- lib/pagy/extras/responsive.rb | 14 ++-- lib/pagy/frontend.rb | 12 ++-- test/pagy/backend_test.rb | 2 +- test/test_helper/backend.rb | 2 +- 10 files changed, 89 insertions(+), 104 deletions(-) diff --git a/docs/extras/responsive.md b/docs/extras/responsive.md index f52ff0ea8..bf1653ef1 100644 --- a/docs/extras/responsive.md +++ b/docs/extras/responsive.md @@ -38,7 +38,7 @@ This extra is composed of the [responsive.rb](https://github.com/ddnexus/pagy/bl ### :breakpoints -The `:breakpoint` variable is a non-core variable added by the `responsive` extra: it allows you to control how the page links will get shown at different widths. It is a hash where the keys are integers representing the breakpoint widths in pixels and the values are the Pagy `:size` variables to be applied for that width. +The `:breakpoints` variable is a non-core variable added by the `responsive` extra: it allows you to control how the page links will get shown at different widths. It is a hash where the keys are integers representing the breakpoint widths in pixels and the values are the Pagy `:size` variables to be applied for that width. For example: ```ruby diff --git a/lib/pagy/extras/bootstrap.rb b/lib/pagy/extras/bootstrap.rb index 1ab4ff7d0..ad57e3c52 100644 --- a/lib/pagy/extras/bootstrap.rb +++ b/lib/pagy/extras/bootstrap.rb @@ -7,21 +7,20 @@ module Frontend # Pagination for bootstrap: it returns the html with the series of links to the pages def pagy_nav_bootstrap(pagy) - tags, link, p_prev, p_next = +'', pagy_link_proc(pagy, 'class="page-link"'), pagy.prev, pagy.next + html, link, p_prev, p_next = +'', pagy_link_proc(pagy, 'class="page-link"'), pagy.prev, pagy.next - tags << (p_prev ? %() + html << (p_prev ? %() : %()) pagy.series.each do |item| # series example: [1, :gap, 7, 8, "9", 10, 11, :gap, 36] - tags << if item.is_a?(Integer); %(
  • #{link.call item}
    • ) # page link + html << if item.is_a?(Integer); %(
  • #{link.call item}
    • ) # page link elsif item.is_a?(String) ; %(
  • #{link.call item}
    • ) # active page elsif item == :gap ; %(
  • #{pagy_t('pagy.nav.gap')}
    • ) # page gap end end - tags << (p_next ? %() + html << (p_next ? %() : %()) - %() + %() end end end - diff --git a/lib/pagy/extras/compact.rb b/lib/pagy/extras/compact.rb index b7e4f8ee7..4e893e259 100644 --- a/lib/pagy/extras/compact.rb +++ b/lib/pagy/extras/compact.rb @@ -6,38 +6,37 @@ class Pagy module Frontend # Generic compact pagination: it returns the html with the series of links to the pages - # we use a numeric input tag to set the page and the PagyCompact javascript to navigate + # we use a numeric input tag to set the page and the Pagy.compact javascript to navigate def pagy_nav_compact(pagy, id=caller(1,1)[0].hash) - tags, link, p_prev, p_next, p_page, p_pages = +'', pagy_link_proc(pagy), pagy.prev, pagy.next, pagy.page, pagy.pages + html, link, p_prev, p_next, p_page, p_pages = +'', pagy_link_proc(pagy), pagy.prev, pagy.next, pagy.page, pagy.pages - tags << %() end # Compact pagination for bootstrap: it returns the html with the series of links to the pages - # we use a numeric input tag to set the page and the PagyCompact javascript to navigate + # we use a numeric input tag to set the page and the Pagy.compact javascript to navigate def pagy_nav_bootstrap_compact(pagy, id=caller(1,1)[0].hash) - tags, link, p_prev, p_next, p_page, p_pages = +'', pagy_link_proc(pagy), pagy.prev, pagy.next, pagy.page, pagy.pages + html, link, p_prev, p_next, p_page, p_pages = +'', pagy_link_proc(pagy), pagy.prev, pagy.next, pagy.page, pagy.pages - tags << %() end end end - diff --git a/lib/pagy/extras/i18n.rb b/lib/pagy/extras/i18n.rb index 002eb46af..1eb08d3fc 100644 --- a/lib/pagy/extras/i18n.rb +++ b/lib/pagy/extras/i18n.rb @@ -13,4 +13,3 @@ def pagy_t(*args) end end - diff --git a/lib/pagy/extras/items.rb b/lib/pagy/extras/items.rb index 1fd14db6b..89e6d8ad0 100644 --- a/lib/pagy/extras/items.rb +++ b/lib/pagy/extras/items.rb @@ -22,21 +22,21 @@ def pagy_get_vars(collection, vars) module Frontend - # this works with all Rack-based frameworks (Sinatra, Padrino, Rails, ...) + # This works with all Rack-based frameworks (Sinatra, Padrino, Rails, ...) def pagy_url_for(page, pagy) p_vars = pagy.vars; params = request.GET.merge(p_vars[:page_param] => page, p_vars[:items_param] => p_vars[:items], **p_vars[:params]) "#{request.path}?#{Rack::Utils.build_nested_query(pagy_get_params(params))}#{p_vars[:anchor]}" end - # return the items selector HTML. For example "Show [20] items per page" + # Return the items selector HTML. For example "Show [20] items per page" def pagy_items_selector(pagy, id=caller(1,1)[0].hash) pagy = pagy.clone; p_vars = pagy.vars; p_items = p_vars[:items]; p_vars[:items] = "#{MARKER}-items-" - tags = +%() - tags << %() + html = +%() + html << %() input = %() - tags << %(#{pagy_t('pagy.items.show')} #{input} #{pagy_t('pagy.items.items')}) - tags << %() + html << %(#{pagy_t('pagy.items.show')} #{input} #{pagy_t('pagy.items.items')}) + html << %() end end diff --git a/lib/pagy/extras/javascripts/pagy.js b/lib/pagy/extras/javascripts/pagy.js index e81cb1a5c..4fc2e08a8 100644 --- a/lib/pagy/extras/javascripts/pagy.js +++ b/lib/pagy/extras/javascripts/pagy.js @@ -1,81 +1,69 @@ -// See the Pagy documentation: https://ddnexus.github.io/pagy/extras +// See the Pagy documentation: https://ddnexus.github.io/pagy/extras#javascript function Pagy(){} +Pagy.addInputEventListeners = function(input, handler){ + // select the content on click: easier for typing a number + input.addEventListener('click', function(){ this.select() }); + // go when the input looses focus + input.addEventListener('focusout', handler); + // … and when pressing enter inside the input + input.addEventListener('keyup', function(e){ if (e.which === 13) handler() }.bind(this)); + }; + Pagy.compact = function(id, marker, page){ var pagyNav = document.getElementById('pagy-nav-'+id), input = pagyNav.getElementsByTagName('input')[0], - link = pagyNav.getElementsByTagName('a')[0]; - - var go = function(){ - if (page !== input.value) { - var href = link.getAttribute('href').replace(marker, input.value); - link.setAttribute('href', href); - link.click(); - } - }; - - // select the content on click: easier for typing a number - input.addEventListener('click', function(){ this.select() }); - // go when the input looses focus - input.addEventListener('focusout', go); - // … and when pressing enter inside the input - input.addEventListener('keyup', function(e){ if (e.which === 13) go() }.bind(this)); - - }; - + link = pagyNav.getElementsByTagName('a')[0], + go = function(){ + if (page !== input.value) { + var href = link.getAttribute('href').replace(marker, input.value); + link.setAttribute('href', href); + link.click(); + } + }; + Pagy.addInputEventListeners(input, go); + }; Pagy.items = function(id, marker, from){ var pagyNav = document.getElementById('pagy-items-'+id), input = pagyNav.getElementsByTagName('input')[0], current = input.value, - link = pagyNav.getElementsByTagName('a')[0]; - - var go = function(){ - var items = input.value; - if (current !== items) { - var page = Math.max(Math.ceil(from / items),1); - var href = link.getAttribute('href').replace(marker+'-page-', page).replace(marker+'-items-', items); - link.setAttribute('href', href); - link.click(); - } - }; - - // select the content on click: easier for typing a number - input.addEventListener('click', function(){ this.select() }); - // go when the input looses focus - input.addEventListener('focusout', go); - // … and when pressing enter inside the input - input.addEventListener('keyup', function(e){ if (e.which === 13) go() }.bind(this)); - + link = pagyNav.getElementsByTagName('a')[0], + go = function(){ + var items = input.value; + if (current !== items) { + var page = Math.max(Math.ceil(from / items),1); + var href = link.getAttribute('href').replace(marker+'-page-', page).replace(marker+'-items-', items); + link.setAttribute('href', href); + link.click(); + } + }; + Pagy.addInputEventListeners(input, go); }; -Pagy.responsive = function(id, items, widths, series){ +Pagy.responsive = function(id, tags, widths, series){ var pagyNav = document.getElementById('pagy-nav-'+id), pagyBox = pagyNav.firstChild || pagyNav, pagyParent = pagyNav.parentElement, - lastWidth = undefined; - - var render = function(){ - var parentWidth = parseInt(pagyParent.clientWidth), - width = widths.find(function(w){return parentWidth > w}); - if (width !== lastWidth) { - while (pagyBox.firstChild) { pagyBox.removeChild(pagyBox.firstChild) } - var tags = items['prev']; - series[width].forEach(function(item){tags += items[item]}); - tags += items['next']; - pagyBox.insertAdjacentHTML('beforeend', tags); - lastWidth = width; - } - }; - + lastWidth = undefined, + render = function(){ + var parentWidth = parseInt(pagyParent.clientWidth), + width = widths.find(function(w){return parentWidth > w}); + if (width !== lastWidth) { + while (pagyBox.firstChild) { pagyBox.removeChild(pagyBox.firstChild) } + var html = tags['prev']; + series[width].forEach(function(item){html += tags[item]}); + html += tags['next']; + pagyBox.insertAdjacentHTML('beforeend', html); + lastWidth = width; + } + }; if (window.attachEvent) { window.attachEvent('onresize', render) } else if (window.addEventListener) { window.addEventListener('resize', render, true) } - render(); }; - Pagy.init = function(){ ['compact', 'items', 'responsive'].forEach(function(name){ Array.from(document.getElementsByClassName("pagy-"+name)).forEach(function(json) { diff --git a/lib/pagy/extras/responsive.rb b/lib/pagy/extras/responsive.rb index 36dc5955b..4bb3f721e 100644 --- a/lib/pagy/extras/responsive.rb +++ b/lib/pagy/extras/responsive.rb @@ -22,7 +22,7 @@ class Pagy # :widths is the desc-ordered array of widths (passed to the PagyResponsive javascript function) def responsive @responsive ||= {items: [], series: {}, widths:[]}.tap do |r| - @vars[:breakpoints].key?(0) || raise(ArgumentError, "expected :breakpoints to contain the 0 size; got #{@vars[:breakpoint].inspect}") + @vars[:breakpoints].key?(0) || raise(ArgumentError, "expected :breakpoints to contain the 0 size; got #{@vars[:breakpoints].inspect}") @vars[:breakpoints].each {|width, size| r[:items] |= r[:series][width] = series(size)} r[:widths] = r[:series].keys.sort!{|a,b| b <=> a} end @@ -32,26 +32,26 @@ def responsive module Frontend # Generic responsive pagination: it returns the html with the series of links to the pages - # we build the tags as a json object string and render them with the PagyResponsive javascript + # rendered by the Pagy.responsive javascript def pagy_nav_responsive(pagy, id=caller(1,1)[0].hash) tags, link, p_prev, p_next, responsive = {}, pagy_link_proc(pagy), pagy.prev, pagy.next, pagy.responsive - tags[:prev] = (p_prev ? %(#{link.call p_prev, pagy_t('pagy.nav.prev'), 'aria-label="previous"'} ) - : %(#{pagy_t('pagy.nav.prev')} )) + tags['prev'] = (p_prev ? %(#{link.call p_prev, pagy_t('pagy.nav.prev'), 'aria-label="previous"'} ) + : %(#{pagy_t('pagy.nav.prev')} )) responsive[:items].each do |item| # series example: [1, :gap, 7, 8, "9", 10, 11, :gap, 36] tags[item.to_s] = if item.is_a?(Integer); %(#{link.call item} ) # page link elsif item.is_a?(String) ; %(#{item} ) # current page elsif item == :gap ; %(#{pagy_t('pagy.nav.gap')} ) # page gap end end - tags[:next] = (p_next ? %(#{link.call p_next, pagy_t('pagy.nav.next'), 'aria-label="next"'}) - : %(#{pagy_t('pagy.nav.next')})) + tags['next'] = (p_next ? %(#{link.call p_next, pagy_t('pagy.nav.next'), 'aria-label="next"'}) + : %(#{pagy_t('pagy.nav.next')})) script = %() %(#{script}) end # Responsive pagination for bootstrap: it returns the html with the series of links to the pages - # we build the tags as a json object string and render them with the PagyResponsive javascript + # rendered by the Pagy.responsive javascript def pagy_nav_bootstrap_responsive(pagy, id=caller(1,1)[0].hash) tags, link, p_prev, p_next, responsive = {}, pagy_link_proc(pagy, 'class="page-link"'), pagy.prev, pagy.next, pagy.responsive diff --git a/lib/pagy/frontend.rb b/lib/pagy/frontend.rb index 41442c0de..81d36ef51 100644 --- a/lib/pagy/frontend.rb +++ b/lib/pagy/frontend.rb @@ -11,19 +11,19 @@ module Frontend # Generic pagination: it returns the html with the series of links to the pages def pagy_nav(pagy) - tags, link, p_prev, p_next = +'', pagy_link_proc(pagy), pagy.prev, pagy.next + html, link, p_prev, p_next = +'', pagy_link_proc(pagy), pagy.prev, pagy.next - tags << (p_prev ? %(#{link.call p_prev, pagy_t('pagy.nav.prev'), 'aria-label="previous"'} ) + html << (p_prev ? %(#{link.call p_prev, pagy_t('pagy.nav.prev'), 'aria-label="previous"'} ) : %(#{pagy_t('pagy.nav.prev')} )) pagy.series.each do |item| # series example: [1, :gap, 7, 8, "9", 10, 11, :gap, 36] - tags << if item.is_a?(Integer); %(#{link.call item} ) # page link + html << if item.is_a?(Integer); %(#{link.call item} ) # page link elsif item.is_a?(String) ; %(#{item} ) # current page elsif item == :gap ; %(#{pagy_t('pagy.nav.gap')} ) # page gap end end - tags << (p_next ? %(#{link.call p_next, pagy_t('pagy.nav.next'), 'aria-label="next"'}) + html << (p_next ? %(#{link.call p_next, pagy_t('pagy.nav.next'), 'aria-label="next"'}) : %(#{pagy_t('pagy.nav.next')})) - %() + %() end # Return examples: "Displaying items 41-60 of 324 in total" or "Displaying Products 41-60 of 324 in total" @@ -33,7 +33,7 @@ def pagy_info(pagy) pagy_t(path, item_name: name, count: pagy.count, from: pagy.from, to: pagy.to) end - # this works with all Rack-based frameworks (Sinatra, Padrino, Rails, ...) + # This works with all Rack-based frameworks (Sinatra, Padrino, Rails, ...) def pagy_url_for(page, pagy) p_vars = pagy.vars; params = request.GET.merge(p_vars[:page_param] => page, **p_vars[:params]) "#{request.path}?#{Rack::Utils.build_nested_query(pagy_get_params(params))}#{p_vars[:anchor]}" diff --git a/test/pagy/backend_test.rb b/test/pagy/backend_test.rb index 75922ca9d..f6589951a 100644 --- a/test/pagy/backend_test.rb +++ b/test/pagy/backend_test.rb @@ -66,7 +66,7 @@ def test_pagy_get_vars_with_vars end def test_pagy_get_vars_with_grouped_collection - @collection = GroupedCollection.new((1..1000).to_a) + @collection = TestGroupedCollection.new((1..1000).to_a) vars = {page: 2, items: 10, link_extra: 'X'} merged = backend.send :pagy_get_vars, @collection, vars assert_includes(merged.keys, :count) diff --git a/test/test_helper/backend.rb b/test/test_helper/backend.rb index f58268df7..e4b874934 100644 --- a/test/test_helper/backend.rb +++ b/test/test_helper/backend.rb @@ -31,7 +31,7 @@ def count(*_) end -class GroupedCollection < TestCollection +class TestGroupedCollection < TestCollection def count(*_) @collection.map { |value| [value, value + 1] }.to_h