From: Anton Khorev <tony29@yandex.ru>
Date: Wed, 6 Mar 2024 11:33:35 +0000 (+0300)
Subject: Add titles to changeset element page links
X-Git-Tag: live~204^2~2
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/b072c2935f8f79e16b2b63cc5e01fc34326f7daa?ds=sidebyside

Add titles to changeset element page links
---

diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb
index c8fc8245c..4cd0384cb 100644
--- a/app/helpers/browse_helper.rb
+++ b/app/helpers/browse_helper.rb
@@ -70,14 +70,14 @@ module BrowseHelper
     "nofollow" if object.tags.empty?
   end
 
-  def type_and_paginated_count(type, pages)
+  def type_and_paginated_count(type, pages, selected_page = pages.current_page)
     if pages.page_count == 1
       t ".#{type.pluralize}",
         :count => pages.item_count
     else
       t ".#{type.pluralize}_paginated",
-        :x => pages.current_page.first_item,
-        :y => pages.current_page.last_item,
+        :x => selected_page.first_item,
+        :y => selected_page.last_item,
         :count => pages.item_count
     end
   end
@@ -93,14 +93,14 @@ module BrowseHelper
     link_classes = ["page-link", { "px-1" => width > max_width_for_default_padding }]
 
     tag.ul :class => "pagination pagination-sm mb-1 ms-auto" do
-      pagination_items(pages, {}).each do |body, n|
-        linked = !(n.is_a? String)
+      pagination_items(pages, {}).each do |body, page_or_class|
+        linked = !(page_or_class.is_a? String)
         link = if linked
-                 link_to body, url_for(page_param => n), :class => link_classes
+                 link_to body, url_for(page_param => page_or_class.number), :class => link_classes, :title => yield(page_or_class)
                else
                  tag.span body, :class => link_classes
                end
-        concat tag.li link, :class => ["page-item", { n => !linked }]
+        concat tag.li link, :class => ["page-item", { page_or_class => !linked }]
       end
     end
   end
diff --git a/app/views/changesets/_paging_nav.html.erb b/app/views/changesets/_paging_nav.html.erb
index aa7ee23b9..19ac42f95 100644
--- a/app/views/changesets/_paging_nav.html.erb
+++ b/app/views/changesets/_paging_nav.html.erb
@@ -1,6 +1,6 @@
 <div class="d-flex flex-wrap gap-2">
   <h4 class="fs-5 mb-0"><%= type_and_paginated_count(type, pages) %></h4>
   <% if pages.page_count > 1 %>
-    <%= sidebar_classic_pagination(pages, "#{type}_page") %>
+    <%= sidebar_classic_pagination(pages, "#{type}_page") { |page| type_and_paginated_count(type, pages, page) } %>
   <% end %>
 </div>
diff --git a/lib/classic_pagination/pagination_helper.rb b/lib/classic_pagination/pagination_helper.rb
index 72d16fc98..c450de4e1 100644
--- a/lib/classic_pagination/pagination_helper.rb
+++ b/lib/classic_pagination/pagination_helper.rb
@@ -145,7 +145,7 @@ module ActionView
         items = []
 
         if always_show_anchors && !(wp_first = window_pages[0]).first?
-          items.push [first.number.to_s, first.number]
+          items.push [first.number.to_s, first]
           items.push ["...", "disabled"] if wp_first.number - first.number > 1
         end
 
@@ -153,13 +153,13 @@ module ActionView
           if current_page == page && !link_to_current_page
             items.push [page.number.to_s, "active"]
           else
-            items.push [page.number.to_s, page.number]
+            items.push [page.number.to_s, page]
           end
         end
 
         if always_show_anchors && !(wp_last = window_pages[-1]).last?
           items.push ["...", "disabled"] if last.number - wp_last.number > 1
-          items.push [last.number.to_s, last.number]
+          items.push [last.number.to_s, last]
         end
 
         items
diff --git a/test/system/changeset_elements_test.rb b/test/system/changeset_elements_test.rb
new file mode 100644
index 000000000..22c065933
--- /dev/null
+++ b/test/system/changeset_elements_test.rb
@@ -0,0 +1,41 @@
+require "application_system_test_case"
+
+class ChangesetElementsTest < ApplicationSystemTestCase
+  test "can navigate between element subpages without losing comment input" do
+    element_page_size = 20
+    changeset = create(:changeset, :closed)
+    ways = create_list(:way, element_page_size + 1, :with_history, :changeset => changeset)
+    way_paths = ways.map { |way| way_path(way) }
+    nodes = create_list(:node, element_page_size + 1, :with_history, :changeset => changeset)
+    node_paths = nodes.map { |node| node_path(node) }
+
+    sign_in_as(create(:user))
+    visit changeset_path(changeset)
+
+    within_sidebar do
+      assert_one_missing_link way_paths
+      assert_link "Ways (21-21 of 21)"
+
+      assert_one_missing_link node_paths
+      assert_link "Nodes (21-21 of 21)"
+    end
+  end
+
+  private
+
+  def assert_one_missing_link(hrefs)
+    missing_href = nil
+    hrefs.each do |href|
+      missing = true
+      assert_link :href => href, :minimum => 0, :maximum => 1 do
+        missing = false
+      end
+      if missing
+        assert_nil missing_href, "unexpected extra missing link '#{href}'"
+        missing_href = href
+      end
+    end
+    assert_not_nil missing_href, "expected one link missing but all are present"
+    missing_href
+  end
+end