Use ul tags to do pagination (#21258).

jplang · jplang · commit ae045cdac99e · 2015-11-28T11:13:15.000Z
Patch by Daniel Ritz. git-svn-id: http://svn.redmine.org/redmine/trunk@14897 e93f8b46-1217-0410-a6f0-8f06a7374b81
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
@@ -41,6 +41,6 @@ def render_principals_for_new_group_users(group, limit=100)
       link_to text, autocomplete_for_user_group_path(group, parameters.merge(:q => params[:q], :format => 'js')), :remote => true
     }
 
-    s + content_tag('p', links, :class => 'pagination')
+    s + content_tag('span', links, :class => 'pagination')
   end
 end
diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb
@@ -33,6 +33,6 @@ def render_principals_for_new_members(project, limit=100)
       link_to text, autocomplete_project_memberships_path(project, parameters.merge(:q => params[:q], :format => 'js')), :remote => true
     }
 
-    s + content_tag('p', links, :class => 'pagination')
+    s + content_tag('span', links, :class => 'pagination')
   end
 end
diff --git a/app/views/auth_sources/index.html.erb b/app/views/auth_sources/index.html.erb
@@ -28,4 +28,4 @@
   </tbody>
 </table>
 
-<p class="pagination"><%= pagination_links_full @auth_source_pages %></p>
+<span class="pagination"><%= pagination_links_full @auth_source_pages %></span>
diff --git a/app/views/boards/show.html.erb b/app/views/boards/show.html.erb
@@ -50,7 +50,7 @@
   <% end %>
   </tbody>
 </table>
-<p class="pagination"><%= pagination_links_full @topic_pages, @topic_count %></p>
+<span class="pagination"><%= pagination_links_full @topic_pages, @topic_count %></span>
 <% else %>
 <p class="nodata"><%= l(:label_no_data) %></p>
 <% end %>
diff --git a/app/views/issue_statuses/index.html.erb b/app/views/issue_statuses/index.html.erb
@@ -32,6 +32,6 @@
   </tbody>
 </table>
 
-<p class="pagination"><%= pagination_links_full @issue_status_pages %></p>
+<span class="pagination"><%= pagination_links_full @issue_status_pages %></span>
 
 <% html_title(l(:label_issue_status_plural)) -%>
diff --git a/app/views/issues/index.html.erb b/app/views/issues/index.html.erb
@@ -66,7 +66,7 @@
 <% else %>
 <%= render_query_totals(@query) %>
 <%= render :partial => 'issues/list', :locals => {:issues => @issues, :query => @query} %>
-<p class="pagination"><%= pagination_links_full @issue_pages, @issue_count %></p>
+<span class="pagination"><%= pagination_links_full @issue_pages, @issue_count %></span>
 <% end %>
 
 <% other_formats_links do |f| %>
diff --git a/app/views/messages/show.html.erb b/app/views/messages/show.html.erb
@@ -67,7 +67,7 @@
   <%= link_to_attachments message, :author => false %>
   </div>
 <% end %>
-<p class="pagination"><%= pagination_links_full @reply_pages, @reply_count, :per_page_links => false %></p>
+<span class="pagination"><%= pagination_links_full @reply_pages, @reply_count, :per_page_links => false %></span>
 <% end %>
 
 <% if !@topic.locked? && authorize_for('messages', 'reply') %>
diff --git a/app/views/news/index.html.erb b/app/views/news/index.html.erb
@@ -37,7 +37,7 @@
     </article>
 <% end %>
 <% end %>
-<p class="pagination"><%= pagination_links_full @news_pages %></p>
+<span class="pagination"><%= pagination_links_full @news_pages %></span>
 
 <% other_formats_links do |f| %>
   <%= f.link_to 'Atom', :url => {:project_id => @project, :key => User.current.rss_key} %>
diff --git a/app/views/repositories/revisions.html.erb b/app/views/repositories/revisions.html.erb
@@ -17,7 +17,7 @@
                        :revisions => @changesets,
                        :entry     => nil } %>
 
-<p class="pagination"><%= pagination_links_full @changeset_pages,@changeset_count %></p>
+<span class="pagination"><%= pagination_links_full @changeset_pages,@changeset_count %></span>
 
 <% content_for :header_tags do %>
   <%= stylesheet_link_tag "scm" %>
diff --git a/app/views/roles/index.html.erb b/app/views/roles/index.html.erb
@@ -29,6 +29,6 @@
   </tbody>
 </table>
 
-<p class="pagination"><%= pagination_links_full @role_pages %></p>
+<span class="pagination"><%= pagination_links_full @role_pages %></span>
 
 <% html_title(l(:label_role_plural)) -%>
diff --git a/app/views/search/index.html.erb b/app/views/search/index.html.erb
@@ -52,7 +52,7 @@
 <% end %>
 
 <% if @result_pages %>
-<p class="pagination"><%= pagination_links_full @result_pages, @result_count, :per_page_links => false %></p>
+<span class="pagination"><%= pagination_links_full @result_pages, @result_count, :per_page_links => false %></span>
 <% end %>
 
 <% html_title(l(:label_search)) -%>
diff --git a/app/views/timelog/index.html.erb b/app/views/timelog/index.html.erb
@@ -18,7 +18,7 @@
 
 <% unless @entries.empty? %>
 <%= render :partial => 'list', :locals => { :entries => @entries }%>
-<p class="pagination"><%= pagination_links_full @entry_pages, @entry_count %></p>
+<span class="pagination"><%= pagination_links_full @entry_pages, @entry_count %></span>
 
 <% other_formats_links do |f| %>
   <%= f.link_to 'Atom', :url => params.merge({:issue_id => @issue, :key => User.current.rss_key}) %>
diff --git a/app/views/trackers/index.html.erb b/app/views/trackers/index.html.erb
@@ -34,6 +34,6 @@
   </tbody>
 </table>
 
-<p class="pagination"><%= pagination_links_full @tracker_pages %></p>
+<span class="pagination"><%= pagination_links_full @tracker_pages %></span>
 
 <% html_title(l(:label_tracker_plural)) -%>
diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb
@@ -53,6 +53,6 @@
   </tbody>
 </table>
 </div>
-<p class="pagination"><%= pagination_links_full @user_pages, @user_count %></p>
+<span class="pagination"><%= pagination_links_full @user_pages, @user_count %></span>
 
 <% html_title(l(:label_user_plural)) -%>
diff --git a/lib/redmine/pagination.rb b/lib/redmine/pagination.rb
@@ -172,34 +172,40 @@ def pagination_links_each(paginator, count=nil, options={}, &block)
         per_page_links = false if count.nil?
         page_param = paginator.page_param
 
-        html = ''
+        html = '<ul class="pages">'
         if paginator.previous_page
           # \xc2\xab(utf-8) = &#171;
           text = "\xc2\xab " + l(:label_previous)
-          html << yield(text, {page_param => paginator.previous_page},
-            :class => 'previous', :accesskey => accesskey(:previous)) + ' '
+          html << content_tag('li',
+                              yield(text, {page_param => paginator.previous_page},
+                                    :accesskey => accesskey(:previous)),
+                              :class => 'previous page')
         end
 
         previous = nil
         paginator.linked_pages.each do |page|
           if previous && previous != page - 1
-            html << content_tag('span', '...', :class => 'spacer') + ' '
+            html << content_tag('li', content_tag('span', '...'), :class => 'spacer') + ' '
           end
           if page == paginator.page
-            html << content_tag('span', page.to_s, :class => 'current page')
+            html << content_tag('li', content_tag('span', page.to_s), :class => 'current')
           else
-            html << yield(page.to_s, {page_param => page}, :class => 'page')
+            html << content_tag('li',
+                                yield(page.to_s, {page_param => page}),
+                                :class => 'page')
           end
-          html << ' '
           previous = page
         end
 
         if paginator.next_page
           # \xc2\xbb(utf-8) = &#187;
           text = l(:label_next) + " \xc2\xbb"
-          html << yield(text, {page_param => paginator.next_page},
-            :class => 'next', :accesskey => accesskey(:next)) + ' '
+          html << content_tag('li',
+                              yield(text, {page_param => paginator.next_page},
+                                    :accesskey => accesskey(:next)),
+                              :class => 'next page')
         end
+        html << '</ul>'
 
         html << content_tag('span', "(#{paginator.first_item}-#{paginator.last_item}/#{paginator.item_count})", :class => 'items') + ' '
 
diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css
@@ -494,7 +494,44 @@ textarea#custom_field_default_value {width: 95%; resize:vertical}
 
 input#content_comments {width: 99%}
 
-p.pagination {margin-top:8px; font-size: 90%}
+span.pagination {margin-left:3px}
+.pagination ul.pages {
+  margin: 0 5px 0 0;
+  padding: 0;
+  display: inline;
+}
+.pagination ul.pages li {
+  display: inline-block;
+  padding: 0;
+  border: 1px solid #BBB;
+  margin-left: -1px;
+  line-height: 2em;
+}
+.pagination ul.pages li a,
+.pagination ul.pages li span {
+  padding: 4px 10px;
+}
+.pagination ul.pages li:first-child {
+  border-top-left-radius: 4px;
+  border-bottom-left-radius: 4px;
+}
+.pagination ul.pages li:last-child {
+  border-top-right-radius: 4px;
+  border-bottom-right-radius: 4px;
+}
+.pagination ul.pages li.current {
+  color: white;
+  background-color: #628DB6;
+  border-color: #628DB6;
+}
+.pagination ul.pages li.page:hover {
+  background-color: #EEE;
+}
+.pagination ul.pages li.page a:hover,
+.pagination ul.pages li.page a:active {
+  color: inherit;
+  text-decoration: inherit;
+}
 
 #search-form fieldset p {margin:0.2em 0;}
 
diff --git a/test/unit/helpers/groups_helper_test.rb b/test/unit/helpers/groups_helper_test.rb
@@ -35,8 +35,8 @@ def test_render_principals_for_new_group_users_with_limited_results_should_pagin
     group = Group.generate!
 
     result = render_principals_for_new_group_users(group, 3)
-    assert_select_in result, 'p.pagination'
-    assert_select_in result, 'span.current.page', :text => '1'
+    assert_select_in result, 'span.pagination'
+    assert_select_in result, 'span.pagination li.current span', :text => '1'
     assert_select_in result, 'a[href=?]', "/groups/#{group.id}/autocomplete_for_user.js?page=2", :text => '2'
   end
 end
diff --git a/test/unit/helpers/members_helper_test.rb b/test/unit/helpers/members_helper_test.rb
@@ -36,8 +36,8 @@ def test_render_principals_for_new_members_with_limited_results_should_paginate
     project = Project.generate!
 
     result = render_principals_for_new_members(project, 3)
-    assert_select_in result, 'p.pagination'
-    assert_select_in result, 'span.current.page', :text => '1'
+    assert_select_in result, 'span.pagination'
+    assert_select_in result, 'span.pagination li.current span', :text => '1'
     assert_select_in result, 'a[href=?]', "/projects/#{project.identifier}/memberships/autocomplete.js?page=2", :text => '2'
   end
 end

Original file line number	Diff line number	Diff line change
`@@ -41,6 +41,6 @@ def render_principals_for_new_group_users(group, limit=100)`
`41`	`41`	`link_to text, autocomplete_for_user_group_path(group, parameters.merge(:q => params[:q], :format => 'js')), :remote => true`
`42`	`42`	`}`
`43`	`43`
`44`		`- s + content_tag('p', links, :class => 'pagination')`
	`44`	`+ s + content_tag('span', links, :class => 'pagination')`
`45`	`45`	`end`
`46`	`46`	`end`
Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,6 @@ def render_principals_for_new_members(project, limit=100)`
`33`	`33`	`link_to text, autocomplete_project_memberships_path(project, parameters.merge(:q => params[:q], :format => 'js')), :remote => true`
`34`	`34`	`}`
`35`	`35`
`36`		`- s + content_tag('p', links, :class => 'pagination')`
	`36`	`+ s + content_tag('span', links, :class => 'pagination')`
`37`	`37`	`end`
`38`	`38`	`end`