Permalink
Browse files

Adopt changes from @agios, detail below:

* Cleanup implementation

* Instead of using pager.html.setting[:first_last] = false,
  please simply set pager.html.setting[:first_text] and
  pager.html.setting[:last_text] to nil. This way, we don't
  need to remember another option.

* Instead of using div with style="display:inline", simply use span

* active_class by default set to "pagination_active" instead of "active"

* Also introduced inactive_class, which would be used in links.
  By default, the name is "pagination_inactive". If you don't like
  this, please set setting[:inactive_class] to nil.
  • Loading branch information...
1 parent 2707b00 commit 8ec01df82dc78796121d4634133450eb95dfaedf @godfat committed Nov 11, 2011
Showing with 53 additions and 28 deletions.
  1. +1 −1 lib/pagify/helper/detail/setting.rb
  2. +23 −12 lib/pagify/helper/html.rb
  3. +3 −3 test/helper_web.rb
  4. +26 −12 test/test_html.rb
@@ -12,7 +12,7 @@ def initialize helper, parent = {}
end
def [] key
- if @attributes[key]
+ if @attributes.key?(key)
@attributes[key]
else
@parent[key]
View
@@ -20,9 +20,9 @@ def self.default_attributes
:ellipsis => '...',
:query_name => :page,
:links_type => :links_full,
- :active_class => 'active',
- :first_last => true,
- :wrapper_class => 'pagination' })
+ :inactive_class => 'pagination_inactive',
+ :active_class => 'pagination_active',
+ :wrapper_class => 'pagination' })
end
def links_full page, &block
@@ -53,29 +53,32 @@ def links_navigate page
def links page
page = normalize_page(page)
size = pager.size
- attrs = extract_html_attributes
+ a = extract_html_attributes
prepare_links(page).map{ |i|
if i == page
case page
- when 1; "<div class=\"#{setting[:active_class]}\" style=\"display: inline;\">#{setting[:first_last] ? setting[:first_text] : i}</div>"
- when size; "<div class=\"#{setting[:active_class]}\" style=\"display: inline;\">#{setting[:first_last] ? setting[ :last_text] : i}</div>"
- else; "<div class=\"#{setting[:active_class]}\" style=\"display: inline;\">#{i}</div>"
+ when 1 ; wrap_active(setting[:first_text] || i)
+ when size; wrap_active(setting[ :last_text] || i)
+ else ; wrap_active(i)
end
else
case i
- when 1; "<a href=\"#{yield(i)}\"#{attrs}>#{setting[:first_last] ? setting[:first_text] : i}</a>"
- when size; "<a href=\"#{yield(i)}\"#{attrs}>#{setting[:first_last] ? setting[ :last_text] : i}</a>"
- when Fixnum; "<a href=\"#{yield(i)}\"#{attrs}>#{i}</a>"
- else; i.to_s
+ when 1 ; wrap_inactive(setting[:first_text] || i, a){yield(i)}
+ when size; wrap_inactive(setting[ :last_text] || i, a){yield(i)}
+ when Fixnum
+ wrap_inactive(i, a){yield(i)}
+ else ; i.to_s
end
end
}.join(setting[:separator])
end
private
def extract_html_attributes
- attrs = setting.additional_attributes.
+ attrs = {:class => setting[:inactive_class]}.
+ merge(setting.additional_attributes).
+ select{ |_, value| value }.
map{ |key, value| "#{key}=\"#{value}\"" }.join(' ')
attrs = ' ' + attrs if attrs != ''
@@ -85,6 +88,14 @@ def extract_html_attributes
def normalize_page page
pager.__send__(:normalize_page, page)
end
+
+ def wrap_active label
+ %Q{<span class="#{setting[:active_class]}">#{label}</span>}
+ end
+
+ def wrap_inactive label, attr
+ %Q{<a href="#{yield}"#{attr}>#{label}</a>}
+ end
end # of HTML
setup HTML
end # of Helper
View
@@ -10,14 +10,14 @@ def test_query_name
end
def test_pagify_links
- assert_equal('<div class="pagination"><a href="/a/b?page=2">&lt; Previous</a> <a href="/a/b?page=4">Next &gt;</a><br /><a href="/a/b?page=1">&laquo; First</a> <a href="/a/b?page=2">2</a> 3 <a href="/a/b?page=4">4</a> <a href="/a/b?page=5">Last &raquo;</a></div>', pagify_links(@data))
+ assert_equal('<div class="pagination"><a href="/a/b?page=2" class="pagination_inactive">&lt; Previous</a> <a href="/a/b?page=4" class="pagination_inactive">Next &gt;</a><br/><a href="/a/b?page=1" class="pagination_inactive">&laquo; First</a> <a href="/a/b?page=2" class="pagination_inactive">2</a> <span class="pagination_active">3</span> <a href="/a/b?page=4" class="pagination_inactive">4</a> <a href="/a/b?page=5" class="pagination_inactive">Last &raquo;</a></div>', pagify_links(@data))
@data.pager.html.setting[:links_type] = :links
- assert_equal('<div class="pagination"><a href="/a/b?page=1">&laquo; First</a> <a href="/a/b?page=2">2</a> 3 <a href="/a/b?page=4">4</a> <a href="/a/b?page=5">Last &raquo;</a></div>', pagify_links(@data))
+ assert_equal('<div class="pagination"><a href="/a/b?page=1" class="pagination_inactive">&laquo; First</a> <a href="/a/b?page=2" class="pagination_inactive">2</a> <span class="pagination_active">3</span> <a href="/a/b?page=4" class="pagination_inactive">4</a> <a href="/a/b?page=5" class="pagination_inactive">Last &raquo;</a></div>', pagify_links(@data))
end
def test_custom_path
- assert_equal('<div class="pagination"><a href="XD?page=2">&lt; Previous</a> <a href="XD?page=4">Next &gt;</a><br /><a href="XD?page=1">&laquo; First</a> <a href="XD?page=2">2</a> 3 <a href="XD?page=4">4</a> <a href="XD?page=5">Last &raquo;</a></div>', pagify_links(@data){'XD'})
+ assert_equal('<div class="pagination"><a href="XD?page=2" class="pagination_inactive">&lt; Previous</a> <a href="XD?page=4" class="pagination_inactive">Next &gt;</a><br/><a href="XD?page=1" class="pagination_inactive">&laquo; First</a> <a href="XD?page=2" class="pagination_inactive">2</a> <span class="pagination_active">3</span> <a href="XD?page=4" class="pagination_inactive">4</a> <a href="XD?page=5" class="pagination_inactive">Last &raquo;</a></div>', pagify_links(@data){'XD'})
end
end
View
@@ -56,6 +56,7 @@ def test_generate_html
pager = Pagify::ArrayPager.new((1..1000).to_a, :per_page => 10)
users = pager[50]
+ pager.html.setting[:class] = nil
assert_equal(#'<a href="49">&lt; Previous</a> 50 <a href="51">Next &gt;</a><br/>'+
'<a href="1">&laquo; First</a> ' +
@@ -70,7 +71,7 @@ def test_generate_html
'<a href="47">47</a> ' +
'<a href="48">48</a> ' +
'<a href="49">49</a> ' +
- '50 ' +
+ '<span class="pagination_active">50</span> ' +
'<a href="51">51</a> ' +
'<a href="52">52</a> ' +
'<a href="53">53</a> ' +
@@ -101,7 +102,7 @@ def test_with_class
'<a href="47" class="pagify">47</a>,,' +
'<a href="48" class="pagify">48</a>,,' +
'<a href="49" class="pagify">49</a>,,' +
- '50,,' +
+ '<span class="pagination_active">50</span>,,' +
'<a href="51" class="pagify">51</a>,,' +
'<a href="52" class="pagify">52</a>,,' +
'<a href="53" class="pagify">53</a>,,' +
@@ -143,23 +144,28 @@ def test_null_pager
def test_2_pages
pager = Pagify::ArrayPager.new([1,2,3], :per_page => 2)
- assert_equal '&laquo; First <a href="2">Last &raquo;</a>', pager.html.links(1, &:to_s)
- assert_equal '<a href="1">&laquo; First</a> Last &raquo;', pager.html.links(2, &:to_s)
+ pager.html.setting[:inactive_class] = nil
+ pager.html.setting[:active_class] = nil
+
+ assert_equal '<span class="">&laquo; First</span> <a href="2">Last &raquo;</a>', pager.html.links(1, &:to_s)
+ assert_equal '<a href="1">&laquo; First</a> <span class="">Last &raquo;</span>', pager.html.links(2, &:to_s)
assert_equal '<a href="2">Next &gt;</a>', pager.html.links_navigate(1, &:to_s)
assert_equal '<a href="1">&lt; Previous</a>', pager.html.links_navigate(2, &:to_s)
end
def test_3_pages
pager = Pagify::ArrayPager.new([1,2,3,4,5], :per_page => 2)
+ pager.html.setting[:class] = nil
+ pager.html.setting[:active_class] = 'a'
- assert_equal '&laquo; First <a href="2">2</a> <a href="3">Last &raquo;</a>',
+ assert_equal '<span class="a">&laquo; First</span> <a href="2">2</a> <a href="3">Last &raquo;</a>',
pager.html.links(1, &:to_s)
- assert_equal '<a href="1">&laquo; First</a> 2 <a href="3">Last &raquo;</a>',
+ assert_equal '<a href="1">&laquo; First</a> <span class="a">2</span> <a href="3">Last &raquo;</a>',
pager.html.links(2, &:to_s)
- assert_equal '<a href="1">&laquo; First</a> <a href="2">2</a> Last &raquo;',
+ assert_equal '<a href="1">&laquo; First</a> <a href="2">2</a> <span class="a">Last &raquo;</span>',
pager.html.links(3, &:to_s)
assert_equal '<a href="2">Next &gt;</a>', pager.html.links_navigate(1, &:to_s)
@@ -172,19 +178,27 @@ def test_3_pages
def test_more_pages_to_left_or_right
pager = Pagify::ArrayPager.new((1..33).to_a, :per_page => 3)
+ pager.html.setting[:first_text] = nil
+ pager.html.setting[:last_text] = nil
+ pager.html.setting[:class] = nil
+ pager.html.setting[:active_class] = 'g'
- first = ['&laquo; First', '<a href="1">&laquo; First</a>']
- last = ['Last &raquo;', '<a href="11">Last &raquo;</a>']
+ first = ['<span class="g">1</span>', '<a href="1">1</a>']
+ last = ['<span class="g">11</span>', '<a href="11">11</a>']
(1..11).each{ |page|
- expected = (2..10).map{ |i| i == page ? i.to_s : "<a href=\"#{i}\">#{i}</a>" }
+ expected = (2..10).map{ |i|
+ if i == page
+ %Q{<span class="g">#{i}</span>}
+ else
+ %Q{<a href="#{i}">#{i}</a>}
+ end
+ }
expected.unshift( page == 1 ? first.first : first.last )
expected.push( page == 11 ? last.first : last.last )
assert_equal expected.join(' '), pager.html.links(page, &:to_s)
}
-
end
-
end

0 comments on commit 8ec01df

Please sign in to comment.