Merge remote-tracking branch 'upstream/pull/4565'

This commit is contained in:
Tom Hughes 2024-08-28 18:59:09 +01:00
commit 5f2a5cadcb
8 changed files with 132 additions and 56 deletions

View 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;
}

View 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

View 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

View file

@ -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 %>

View 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>

View file

@ -90,42 +90,15 @@
<% 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>

View 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

View file

@ -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