From d885d449648ff48eb763cef14b6525e41e177fb4 Mon Sep 17 00:00:00 2001
From: Daniel Ritz <daniel.ritz@gmx.ch>
Date: Mon, 16 Nov 2015 21:18:36 +0100
Subject: [PATCH 4/5] Pagination: use <ul> to do pagination for better styling
 capabilities

* The outer p.pagination is changed to span.pagination. This is required
  since lists are not allowed within <p>
* Change all items of the navigation to be <li>. This allows easy styling.
---
 app/helpers/groups_helper.rb              |  2 +-
 app/helpers/members_helper.rb             |  2 +-
 app/views/auth_sources/index.html.erb     |  2 +-
 app/views/boards/show.html.erb            |  2 +-
 app/views/issue_statuses/index.html.erb   |  2 +-
 app/views/issues/index.html.erb           |  2 +-
 app/views/messages/show.html.erb          |  2 +-
 app/views/news/index.html.erb             |  2 +-
 app/views/repositories/revisions.html.erb |  2 +-
 app/views/roles/index.html.erb            |  2 +-
 app/views/search/index.html.erb           |  2 +-
 app/views/timelog/index.html.erb          |  2 +-
 app/views/trackers/index.html.erb         |  2 +-
 app/views/users/index.html.erb            |  2 +-
 lib/redmine/pagination.rb                 | 24 +++++++++++++++---------
 public/stylesheets/application.css        | 11 ++++++++++-
 test/unit/helpers/groups_helper_test.rb   |  4 ++--
 test/unit/helpers/members_helper_test.rb  |  4 ++--
 18 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index 0214d52..0706dcb 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -41,6 +41,6 @@ module GroupsHelper
       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
index cd9e563..57ff4d4 100644
--- a/app/helpers/members_helper.rb
+++ b/app/helpers/members_helper.rb
@@ -33,6 +33,6 @@ module MembersHelper
       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
index fcdbf7d..ecde2bf 100644
--- 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
index 7a599ea..a973431 100644
--- 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
index 3da8c5b..40a5c92 100644
--- 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
index de87c5e..8310dc7 100644
--- 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
index af59845..60c7d5f 100644
--- a/app/views/messages/show.html.erb
+++ b/app/views/messages/show.html.erb
@@ -71,7 +71,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
index 87eb98b..64614c1 100644
--- 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
index a1e5458..c0afb95 100644
--- 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
index 9545d55..6561d11 100644
--- 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
index 35d0d0b..6fceaea 100644
--- 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
index ad1854f..a875e53 100644
--- 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
index f76a4f5..8ba7b7c 100644
--- 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
index 7cd6bb0..c32b46a 100644
--- 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
index 943e8f5..83ded7b 100644
--- a/lib/redmine/pagination.rb
+++ b/lib/redmine/pagination.rb
@@ -172,34 +172,40 @@ module Redmine
         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
index ab6830f..ee8e737 100644
--- a/public/stylesheets/application.css
+++ b/public/stylesheets/application.css
@@ -498,7 +498,16 @@ 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-top:8px; font-size: 90%}
+.pagination ul.pages {
+  margin: 0 5px 0 0;
+  padding: 0;
+  display: inline;
+}
+.pagination ul.pages li {
+  display: inline-block;
+  padding-right: 4px;
+}
 
 #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
index 19a0813..3492a0d 100644
--- a/test/unit/helpers/groups_helper_test.rb
+++ b/test/unit/helpers/groups_helper_test.rb
@@ -35,8 +35,8 @@ class GroupsHelperTest < ActionView::TestCase
     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
index 0c20aa4..61089bd 100644
--- a/test/unit/helpers/members_helper_test.rb
+++ b/test/unit/helpers/members_helper_test.rb
@@ -36,8 +36,8 @@ class MembersHelperTest < ActionView::TestCase
     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
-- 
2.6.2