]> git.openstreetmap.org Git - rails.git/commitdiff
Merge remote-tracking branch 'upstream/pull/4565'
authorTom Hughes <tom@compton.nu>
Wed, 28 Aug 2024 17:59:09 +0000 (18:59 +0100)
committerTom Hughes <tom@compton.nu>
Wed, 28 Aug 2024 17:59:09 +0000 (18:59 +0100)
app/assets/javascripts/index.js
app/controllers/changesets_controller.rb
app/helpers/browse_helper.rb
app/views/changesets/_elements.html.erb [new file with mode: 0644]
app/views/changesets/_paging_nav.html.erb
app/views/changesets/show.html.erb
lib/classic_pagination/pagination_helper.rb
test/system/changeset_elements_test.rb [new file with mode: 0644]

index 5c7b2d26e9f1e0fd6eb2d1c53a2bd61ce0481584..4f3c414f5b85b207b399f4ed874bee7649e1ea92 100644 (file)
@@ -396,7 +396,7 @@ $(document).ready(function () {
   OSM.router.load();
 
   $(document).on("click", "a", function (e) {
-    if (e.isDefaultPrevented() || e.isPropagationStopped()) {
+    if (e.isDefaultPrevented() || e.isPropagationStopped() || $(e.target).data("turbo")) {
       return;
     }
 
index a5ddaf364fe44ae48dafeaa5f0e9761830e2ee98..ba04b9a1771a5fa5501f2b3359140c221952a67d 100644 (file)
@@ -75,22 +75,33 @@ class ChangesetsController < ApplicationController
   end
 
   def show
-    @type = "changeset"
     @changeset = Changeset.find(params[:id])
-    @comments = if current_user&.moderator?
-                  @changeset.comments.unscope(:where => :visible).includes(:author)
-                else
-                  @changeset.comments.includes(:author)
-                end
-    @node_pages, @nodes = paginate(:old_nodes, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "node_page")
-    @way_pages, @ways = paginate(:old_ways, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "way_page")
-    @relation_pages, @relations = paginate(:old_relations, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "relation_page")
-    if @changeset.user.active? && @changeset.user.data_public?
-      changesets = conditions_nonempty(@changeset.user.changesets)
-      @next_by_user = changesets.where("id > ?", @changeset.id).reorder(:id => :asc).first
-      @prev_by_user = changesets.where(:id => ...@changeset.id).reorder(:id => :desc).first
+    case turbo_frame_request_id
+    when "changeset_nodes"
+      @node_pages, @nodes = paginate(:old_nodes, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "node_page")
+      render :partial => "elements", :locals => { :type => "node", :elements => @nodes, :pages => @node_pages }
+    when "changeset_ways"
+      @way_pages, @ways = paginate(:old_ways, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "way_page")
+      render :partial => "elements", :locals => { :type => "way", :elements => @ways, :pages => @way_pages }
+    when "changeset_relations"
+      @relation_pages, @relations = paginate(:old_relations, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "relation_page")
+      render :partial => "elements", :locals => { :type => "relation", :elements => @relations, :pages => @relation_pages }
+    else
+      @comments = if current_user&.moderator?
+                    @changeset.comments.unscope(:where => :visible).includes(:author)
+                  else
+                    @changeset.comments.includes(:author)
+                  end
+      @node_pages, @nodes = paginate(:old_nodes, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "node_page")
+      @way_pages, @ways = paginate(:old_ways, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "way_page")
+      @relation_pages, @relations = paginate(:old_relations, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "relation_page")
+      if @changeset.user.active? && @changeset.user.data_public?
+        changesets = conditions_nonempty(@changeset.user.changesets)
+        @next_by_user = changesets.where("id > ?", @changeset.id).reorder(:id => :asc).first
+        @prev_by_user = changesets.where(:id => ...@changeset.id).reorder(:id => :desc).first
+      end
+      render :layout => map_layout
     end
-    render :layout => map_layout
   rescue ActiveRecord::RecordNotFound
     render :template => "browse/not_found", :status => :not_found, :layout => map_layout
   end
index c8fc8245c5e29e61bd5363c0bbde3aff7b31e3b0..0ea10e219080812e79a94e07d415fdfdb4bfff07 100644 (file)
@@ -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, **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/_elements.html.erb b/app/views/changesets/_elements.html.erb
new file mode 100644 (file)
index 0000000..fd5dd8a
--- /dev/null
@@ -0,0 +1,12 @@
+<%= turbo_frame_tag "changeset_#{type.pluralize}" do %>
+  <%= render :partial => "paging_nav", :locals => { :type => type, :pages => pages } %>
+  <ul class="list-unstyled" data-turbo="false">
+    <% elements.each do |element| %>
+      <%= element_list_item type, element do
+            t "printable_name.current_and_old_links_html",
+              :current_link => link_to(printable_element_name(element), :controller => type.pluralize, :action => :show, :id => element.id[0]),
+              :old_link => link_to(printable_element_version(element), :controller => "old_#{type.pluralize}", :action => :show, :id => element.id[0], :version => element.version)
+          end %>
+    <% end %>
+  </ul>
+<% end %>
index aa7ee23b9372342b0b9b04664051beda3a4225ef..fbdf1d50773c1e1d728e1877c982efe3f5a8801c 100644 (file)
@@ -1,6 +1,11 @@
 <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") do |page|
+          {
+            :title => type_and_paginated_count(type, pages, page),
+            :data => { :turbo => "true" }
+          }
+        end %>
   <% end %>
 </div>
index 915b0ef352531c5bbbb749e27fb5299f597df6e4..13f40ce500d016dee39537330586f908b8287828 100644 (file)
   <% end %>
 
   <% unless @ways.empty? %>
-    <%= render :partial => "paging_nav", :locals => { :type => "way", :pages => @way_pages } %>
-    <ul class="list-unstyled">
-      <% @ways.each do |way| %>
-        <%= element_list_item "way", way do
-              t "printable_name.current_and_old_links_html",
-                :current_link => link_to(printable_element_name(way), way_path(way.way_id)),
-                :old_link => link_to(printable_element_version(way), old_way_path(way.way_id, way.version))
-            end %>
-      <% end %>
-    </ul>
+    <%= render :partial => "elements", :locals => { :type => "way", :elements => @ways, :pages => @way_pages } %>
   <% end %>
 
   <% unless @relations.empty? %>
-    <%= render :partial => "paging_nav", :locals => { :type => "relation", :pages => @relation_pages } %>
-    <ul class="list-unstyled">
-      <% @relations.each do |relation| %>
-        <%= element_list_item "relation", relation do
-              t "printable_name.current_and_old_links_html",
-                :current_link => link_to(printable_element_name(relation), relation_path(relation.relation_id)),
-                :old_link => link_to(printable_element_version(relation), old_relation_path(relation.relation_id, relation.version))
-            end %>
-      <% end %>
-    </ul>
+    <%= render :partial => "elements", :locals => { :type => "relation", :elements => @relations, :pages => @relation_pages } %>
   <% end %>
 
   <% unless @nodes.empty? %>
-    <%= render :partial => "paging_nav", :locals => { :type => "node", :pages => @node_pages } %>
-    <ul class="list-unstyled">
-      <% @nodes.each do |node| %>
-        <%= element_list_item "node", node do
-              t "printable_name.current_and_old_links_html",
-                :current_link => link_to(printable_element_name(node), node_path(node.node_id), { :rel => link_follow(node) }),
-                :old_link => link_to(printable_element_version(node), old_node_path(node.node_id, node.version), { :rel => link_follow(node) })
-            end %>
-      <% end %>
-    </ul>
+    <%= render :partial => "elements", :locals => { :type => "node", :elements => @nodes, :pages => @node_pages } %>
   <% end %>
 </div>
 
index 72d16fc983cdbfd80b28201f1ce9b239c1b3b939..c450de4e149b6cff38bb3ab3a787789dd3ccc725 100644 (file)
@@ -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 (file)
index 0000000..9e65f5b
--- /dev/null
@@ -0,0 +1,75 @@
+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
+      next_page_way_path = assert_one_missing_link way_paths
+      assert_no_link "Ways (1-20 of 21)"
+      assert_link "Ways (21-21 of 21)"
+
+      assert_one_missing_link node_paths
+      assert_no_link "Nodes (1-20 of 21)"
+      assert_link "Nodes (21-21 of 21)"
+
+      fill_in "text", :with => "Comment text we don't want to lose"
+
+      click_on "Ways (21-21 of 21)"
+
+      assert_one_present_link way_paths, next_page_way_path
+      assert_link "Ways (1-20 of 21)"
+      assert_no_link "Ways (21-21 of 21)"
+
+      next_page_node_path = assert_one_missing_link node_paths
+      assert_no_link "Nodes (1-20 of 21)"
+      assert_link "Nodes (21-21 of 21)"
+
+      assert_field "text", :with => "Comment text we don't want to lose"
+
+      click_on "Nodes (21-21 of 21)"
+
+      assert_one_present_link way_paths, next_page_way_path
+      assert_link "Ways (1-20 of 21)"
+      assert_no_link "Ways (21-21 of 21)"
+
+      assert_one_present_link node_paths, next_page_node_path
+      assert_link "Nodes (1-20 of 21)"
+      assert_no_link "Nodes (21-21 of 21)"
+
+      assert_field "text", :with => "Comment text we don't want to lose"
+    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
+
+  def assert_one_present_link(hrefs, present_href)
+    hrefs.each do |href|
+      assert_link :href => href, :count => (href == present_href ? 1 : 0)
+    end
+  end
+end