From: Tom Hughes Date: Wed, 28 Aug 2024 17:59:09 +0000 (+0100) Subject: Merge remote-tracking branch 'upstream/pull/4565' X-Git-Tag: live~474 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/5f2a5cadcb693b34b81a9232b8879c3b4f12843d?hp=7fc83c880ab8ac6e3f6a8778ee7bc65a066670e3 Merge remote-tracking branch 'upstream/pull/4565' --- diff --git a/app/assets/javascripts/index.js b/app/assets/javascripts/index.js index 5c7b2d26e..4f3c414f5 100644 --- a/app/assets/javascripts/index.js +++ b/app/assets/javascripts/index.js @@ -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; } diff --git a/app/controllers/changesets_controller.rb b/app/controllers/changesets_controller.rb index a5ddaf364..ba04b9a17 100644 --- a/app/controllers/changesets_controller.rb +++ b/app/controllers/changesets_controller.rb @@ -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 diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb index c8fc8245c..0ea10e219 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, **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 index 000000000..fd5dd8a26 --- /dev/null +++ b/app/views/changesets/_elements.html.erb @@ -0,0 +1,12 @@ +<%= turbo_frame_tag "changeset_#{type.pluralize}" do %> + <%= render :partial => "paging_nav", :locals => { :type => type, :pages => pages } %> + +<% end %> diff --git a/app/views/changesets/_paging_nav.html.erb b/app/views/changesets/_paging_nav.html.erb index aa7ee23b9..fbdf1d507 100644 --- a/app/views/changesets/_paging_nav.html.erb +++ b/app/views/changesets/_paging_nav.html.erb @@ -1,6 +1,11 @@

<%= type_and_paginated_count(type, pages) %>

<% 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 %>
diff --git a/app/views/changesets/show.html.erb b/app/views/changesets/show.html.erb index 915b0ef35..13f40ce50 100644 --- a/app/views/changesets/show.html.erb +++ b/app/views/changesets/show.html.erb @@ -90,42 +90,15 @@ <% end %> <% unless @ways.empty? %> - <%= render :partial => "paging_nav", :locals => { :type => "way", :pages => @way_pages } %> - + <%= 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 } %> - + <%= 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 } %> - + <%= render :partial => "elements", :locals => { :type => "node", :elements => @nodes, :pages => @node_pages } %> <% end %> 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..9e65f5be2 --- /dev/null +++ b/test/system/changeset_elements_test.rb @@ -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