Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mild rejig of the browse pages #487

Closed
wants to merge 28 commits into from
Closed

Mild rejig of the browse pages #487

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
bbf5561
browse page: improve css for h4 headings
danstowell Sep 18, 2013
7371ae9
browse page: collect "download xml" & "edit" links w other right-hand…
danstowell Sep 19, 2013
7dc49f3
browse page: collapse "last edited" info rows into a sentence under t…
danstowell Sep 19, 2013
47f7afe
browse page: float browse-nav to right-hand side
danstowell Sep 19, 2013
c2d2b09
browse page: less wordy headers for changeset content panes
danstowell Sep 19, 2013
bebdbae
browse page: express last-edited time as user-friendly "time ago"
danstowell Sep 19, 2013
93fe5a2
browse page: undo my mistaken reordering of divs
danstowell Sep 21, 2013
c881ef4
browse page: no top-border-bar for first child needed now
danstowell Sep 21, 2013
63e1e77
browse page: better way of specifying "time ago" (for i18n)
danstowell Sep 21, 2013
cd8b516
browse page: more approachable headings
danstowell Sep 21, 2013
2da012d
browse page: ensure neat render (no spurious top bar) when no-tags-no…
danstowell Sep 21, 2013
ae5bdb2
browse page: formatting tweaks for small-screen mode
danstowell Sep 22, 2013
43b3f8a
browse page: printable_name() opt args converted to named hash options
danstowell Sep 28, 2013
6c48876
browse page: full rather than "lego" i18n strings for editsummary
danstowell Sep 28, 2013
64b98e0
browse page: reinstate version metadata on history page
danstowell Sep 28, 2013
45a056f
browse page: remove title translations that are now no-op
danstowell Sep 28, 2013
d0b8718
bugfix: remove superfluous bracket in css
danstowell Oct 8, 2013
6e18e76
browse pages: use common "pager" style across large and small
danstowell Oct 8, 2013
83d787a
browse page: improve line-wrapping of the new edit summary
danstowell Oct 8, 2013
f091f75
update locales to remove three browse.*.*_title keys no longer used
danstowell Oct 8, 2013
ca98e8d
fix previous linebreak commit so it looks good on the history pages too
danstowell Oct 8, 2013
8348c4e
browse page: don't show edit summary stats if redacted
danstowell Oct 8, 2013
3af7c2a
browse page: update tests for redactions
danstowell Oct 8, 2013
b94c1c7
browse page: move changeset's when+who info into heading, matching th…
danstowell Oct 27, 2013
471bc62
browse page: fix misaligned "Comment" heading
danstowell Oct 28, 2013
2e043a3
browse page: historylink inside versioninchangeset (not working for h…
danstowell Nov 3, 2013
19469ed
browse page: history link in version text
danstowell Nov 3, 2013
a634f12
browse page: "View history" title for version link
danstowell Nov 4, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions app/assets/stylesheets/common.css.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1301,8 +1301,11 @@ ul.results-list li { border-bottom: 1px solid #ccc; }
border-top: 1px solid #ccc;
margin-top: $lineheight/2;
padding-top: $lineheight/2;
clear: left;
&:first-child {
margin-top: 0;
border-top: 0;
padding-top: 0;
}
.warning {
background-color: #ffe0cc;
Expand All @@ -1324,9 +1327,10 @@ ul.results-list li { border-bottom: 1px solid #ccc; }
}
h4 {
float: left;
width: 33.3333%;
width: 18%;
display: inline-block;
vertical-align: top;
clear: left;
}
}

Expand Down Expand Up @@ -1940,8 +1944,8 @@ ul.secondary-actions {
margin-bottom: 0;
margin-left: 0;
&.pager {
display: inline-block;
margin-right: 60px;
display: inline;
margin-right: 30px;
}
> li {
display: block;
Expand Down
4 changes: 4 additions & 0 deletions app/assets/stylesheets/large.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
/* Styles specific to large screens */
ul.secondary-actions.pager {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd like to just delete large.css, so can this go in the main stylesheet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the motivation for deleting large.css is for speed of loading?
On small screens, the float I added looks confusing and messy, which is why I moved it to large-only. If I simply put it in common.css and attempt to override it with float:none in small.css, it doesn't override, for me. (I tend to avoid using !important because it feels hacky.) Suggestions welcome about what would be best here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the plan was to get rid of the whole small/large thing in due course and just use media queries in the main file when the site can't be made responsive in other ways, but I might be wrong.

Historically we haven't used large.css that much - in most cases we just put the large screen code in common.css and override it for small screens in small.css.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just drop this change? It seems pretty minor, and on /browse/changesets, it results in the top pager being hidden behind the map.

float: right;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regardless of any other issues with this style rule, you have more close braces then open, which is causing a large number of test failures.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh dear, that's embarrassing, thanks. Will push a brackets fix immediately, before considering the other remaining issues.

3 changes: 3 additions & 0 deletions app/assets/stylesheets/small.css.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ h2, h3, h4 {
#browse_map ul.secondary-actions {
float: right;
font-size: 10px;
margin-left: 5px;
margin-right: 5px;
}

#map {
Expand Down Expand Up @@ -216,6 +218,7 @@ p.search_results_entry {

/* Rules for the browse pages */

.browse_details.common div,
.browse-section.common div{
clear: both;
}
Expand Down
29 changes: 23 additions & 6 deletions app/helpers/browse_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,41 @@ def link_to_page(page, page_param)
return link_to(page, page_param => page)
end

def printable_name(object, version=false)
def printable_name(object, options={})
options = {
:type => false, # :type can be set to a string indicating object type, such as t('browse.way.way')
:version => false
}.merge(options)
if object.id.is_a?(Array)
id = object.id[0]
else
id = object.id
end
name = t 'printable_name.with_id', :id => id.to_s
if version
name = t 'printable_name.with_version', :id => name, :version => object.version.to_s
name = t 'printable_name.with_id', :id => id.to_s, :type => options[:type]
if options[:version]
name = t 'printable_name.with_version', :id => name, :type => options[:type], :version => object.version.to_s
end

# don't look at object tags if redacted, so as to avoid giving
# away redacted version tag information.
unless object.redacted?
nametag = false
if object.tags.include? "name:#{I18n.locale}"
name = t 'printable_name.with_name', :name => object.tags["name:#{I18n.locale}"].to_s, :id => name
nametag = object.tags["name:#{I18n.locale}"].to_s
elsif object.tags.include? 'name'
name = t 'printable_name.with_name', :name => object.tags['name'].to_s, :id => name
nametag = object.tags['name'].to_s
end

if nametag
if options[:type]
name = t 'printable_name.with_name_type', :name => nametag, :type => options[:type], :id => name
else
name = t 'printable_name.with_name', :name => nametag, :id => name
end
else
if options[:type]
name = t 'printable_name.with_type', :type => options[:type], :id => name
end
end
end

Expand Down
8 changes: 4 additions & 4 deletions app/views/browse/_changeset_details.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
<h4><%= t 'browse.changeset_details.has_nodes', :count => @node_pages.item_count %></h4>
<ul>
<% @nodes.each do |node| %>
<li><%= link_to h(printable_name(node, true)), { :action => "node", :id => node.node_id.to_s }, :class => link_class('node', node), :title => link_title(node) %></li>
<li><%= link_to h(printable_name(node, {:version => true})), { :action => "node", :id => node.node_id.to_s }, :class => link_class('node', node), :title => link_title(node) %></li>
<% end %>
</ul>
</div>
Expand All @@ -53,7 +53,7 @@
<h4><%= t 'browse.changeset_details.has_ways', :count => @way_pages.item_count %></h4>
<ul>
<% @ways.each do |way| %>
<li><%= link_to h(printable_name(way, true)), { :action => "way", :id => way.way_id.to_s }, :class => link_class('way', way), :title => link_title(way) %></li>
<li><%= link_to h(printable_name(way, {:version => true})), { :action => "way", :id => way.way_id.to_s }, :class => link_class('way', way), :title => link_title(way) %></li>
<% end %>
<%=
#render :partial => "containing_relation", :collection => changeset_details.containing_relation_members
Expand All @@ -68,11 +68,11 @@
<h4><%= t 'browse.changeset_details.has_relations', :count => @relation_pages.item_count %></h4>
<ul>
<% @relations.each do |relation| %>
<li><%= link_to h(printable_name(relation, true)), { :action => "relation", :id => relation.relation_id.to_s }, :class => link_class('relation', relation), :title => link_title(relation) %></li>
<li><%= link_to h(printable_name(relation, {:version => true})), { :action => "relation", :id => relation.relation_id.to_s }, :class => link_class('relation', relation), :title => link_title(relation) %></li>
<% end %>
</ul>
</div>
<%= render :partial => 'paging_nav', :locals => { :pages => @relation_pages, :page_param => "relation_page" } %>
<% end %>

</div>
</div>
42 changes: 7 additions & 35 deletions app/views/browse/_common_details.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,39 +1,11 @@
<div class='browse-section common'>
<div>
<% if common_details.visible? %>
<h4><%= t 'browse.common_details.edited_at' %></h4>
<% else %>
<h4><%= t 'browse.common_details.deleted_at' %></h4>
<% end %>
<p><%= l common_details.timestamp %></p>
</div>

<% if common_details.changeset.user.data_public? %>
<% if common_details.changeset.tags['comment'].present? or (not common_details.tags.empty?) %>
<div class='browse-section common'>
<div>
<% if common_details.visible? %>
<h4><%= t 'browse.common_details.edited_by' %></h4>
<% else %>
<h4><%= t 'browse.common_details.deleted_by' %></h4>
<%= render :partial => "tag_details", :object => common_details %>
<% if common_details.changeset.tags['comment'].present? %>
<h4><%= t 'browse.common_details.changeset_comment' %></h4>
<p><%= linkify(h(common_details.changeset.tags['comment'])) %></p>
<% end %>
<p><%= link_to h(common_details.changeset.user.display_name), :controller => "user", :action => "view", :display_name => common_details.changeset.user.display_name %></p>
</div>
<% end %>

<div>
<h4><%= t 'browse.common_details.version' %></h4>
<p><%= h(common_details.version) %></p>
</div>

<div>
<h4><%= t 'browse.common_details.in_changeset' %></h4>
<p><%= link_to common_details.changeset_id, :action => :changeset, :id => common_details.changeset_id %></p>
</div>

<% if common_details.changeset.tags['comment'].present? %>
<div>
<h4><%= t 'browse.common_details.changeset_comment' %></h4>
<p><%= linkify(h(common_details.changeset.tags['comment'])) %></p>
</div>
<% end %>
</div>
<%= render :partial => "tag_details", :object => common_details %>
<% end %>
22 changes: 22 additions & 0 deletions app/views/browse/_common_editsummary.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<%
@agohtml = "<abbr title='%s'>%s</abbr>" % [l(common_editsummary.timestamp), t('browse.common_details.ago', :time_in_words_ago => time_ago_in_words(common_editsummary.timestamp))]

if common_editsummary.changeset.user.data_public?
@userhtml = link_to h(common_editsummary.changeset.user.display_name), :controller => "user", :action => "view", :display_name => common_editsummary.changeset.user.display_name
if common_editsummary.visible? %>
<%= raw t 'browse.common_details.edited_ago_by', :time_in_words_ago => @agohtml, :user => @userhtml %>
<% else %>
<%= raw t 'browse.common_details.deleted_ago_by', :time_in_words_ago => @agohtml, :user => @userhtml %>
<% end %>
<% else %>
<% if common_editsummary.visible? %>
<%= raw t 'browse.common_details.edited_ago', :time_in_words_ago => @agohtml %>
<% else %>
<%= raw t 'browse.common_details.deleted_ago', :time_in_words_ago => @agohtml %>
<% end %>
<% end %>

&middot;

<%= raw t 'browse.common_details.version_in_changeset', :version => common_editsummary.version, :changeset => link_to(common_editsummary.changeset_id, :action => :changeset, :id => common_editsummary.changeset_id) %>

2 changes: 1 addition & 1 deletion app/views/browse/_containing_relation.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
else
raw t 'browse.containing_relation.entry_role', :relation_name => linked_name, :relation_role => h(containing_relation.member_role)
end
%></li>
%></li>
35 changes: 35 additions & 0 deletions app/views/browse/_map.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,41 @@
</ul>
<% end %>

<%
if map.instance_of? Changeset
%>
<ul class='secondary-actions clearfix'>
<li><%= link_to(t('browse.changeset.changesetxml'), :controller => "changeset", :action => "read") %></li>
<li><%= link_to(t('browse.changeset.osmchangexml'), :controller => "changeset", :action => "download") %></li>
</ul>
<%
elsif map.instance_of? OldNode
%>
<ul class='secondary-actions clearfix'>
<li><%= link_to(t('browse.node.history.download_xml'), :controller => "old_node", :action => "history") %></li>
</ul>
<%
elsif map.instance_of? OldWay
%>
<ul class='secondary-actions clearfix'>
<li><%= link_to(t('browse.way_history.download_xml'), :controller => "old_way", :action => "history") %></li>
</ul>
<%
elsif map.instance_of? OldRelation
%>
<ul class='secondary-actions clearfix'>
<li><%= link_to(t('browse.relation_history.download_xml'), :controller => "old_relation", :action => "history") %></li>
</ul>
<%
elsif !(map.instance_of? Note)
%>
<ul class='secondary-actions clearfix'>
<li><%= link_to(t('browse.' + map.class.to_s.downcase + '.download_xml'), :controller => map.class.to_s.downcase, :action => "read") %></li>
</ul>
<%
end
%>

<% else %>
<%= t 'browse.map.deleted' %>
<% end %>
Expand Down
4 changes: 2 additions & 2 deletions app/views/browse/_node_details.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="browse_details" id="<%= node_details.version %>">
<div class="browse_details common" id="<%= node_details.version %>">
<% if node_details.redacted? %>
<div class='browse-section'>
<%= t 'browse.redacted.message_html', :type => t('browse.redacted.type.node'), :redaction_link => link_to(t('browse.redacted.redaction', :id => node_details.redaction.id), node_details.redaction), :version => node_details.version %>
Expand All @@ -18,7 +18,7 @@
<h4><%= t 'browse.node_details.part_of' %></h4>
<ul>
<% node_details.ways.each do |way| %>
<li><%= link_to h(printable_name(way)), { :action => "way", :id => way.id.to_s }, :class => link_class('way', way), :title => link_title(way) %></li>
<li><%= link_to h(printable_name(way, {:type => t('browse.way.way')})), { :action => "way", :id => way.id.to_s }, :class => link_class('way', way), :title => link_title(way) %></li>
<% end %>
<%= render :partial => "containing_relation", :collection => node_details.containing_relation_members %>
</ul>
Expand Down
2 changes: 1 addition & 1 deletion app/views/browse/_relation_member.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@
else
raw t'browse.relation_member.entry_role', :type => type_str, :name => linked_name, :role => h(relation_member.member_role)
end
%></li>
%></li>
9 changes: 2 additions & 7 deletions app/views/browse/changeset.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,10 @@

<% content_for :heading do %>
<h2><%= t 'browse.changeset.changeset', :id => @changeset.id %></h2>
<ul class='secondary-actions clearfix'>
<li><%= link_to(t('browse.changeset.changesetxml'), :controller => "changeset", :action => "read") %></li>
<li><%= link_to(t('browse.changeset.osmchangexml'), :controller => "changeset", :action => "download") %></li>
</ul>
<%= render :partial => "navigation" %>
<% end %>

<%= render :partial => "navigation" %>

<% if @changeset.has_valid_bbox? %>
<%= render :partial => "map", :object => @changeset %>
<% end %>
<%= render :partial => "changeset_details", :object => @changeset %>
<%= render :partial => "changeset_details", :object => @changeset %>
21 changes: 9 additions & 12 deletions app/views/browse/node.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,24 @@
<%
@name = printable_name @node
@name = printable_name @node, {:type => t('browse.node.node')}
@title = t('browse.node.node') + ' | ' + @name
%>
<% content_for :head do %>
<%= stylesheet_link_tag 'browse' %>
<% end %>

<% content_for :heading do %>
<h2><%= t'browse.node.node_title', :node_name => @name %></h2>
<ul class='secondary-actions clearfix'>
<li><%= link_to(t('browse.node.download_xml'), :controller => "node", :action => "read") %></li>
<li><%= link_to(t('browse.node.view_history'), :action => "node_history") %></li>
<% if @node.visible -%>
<li><%= link_to(t('browse.node.edit'), :controller => "site", :action => "edit", :lat => @node.lat, :lon => @node.lon, :zoom => 18, :node => @node.id) %></li>
<% end -%>
</ul>
<% end %>
<%= render :partial => "navigation" %>
<h2><%= @name %></h2>
<%= render :partial => "navigation" %>

<%= render :partial => "common_editsummary", :object => @node %>

(<%= link_to(t('browse.node.view_history'), :action => "node_history") %>)

<% end %>
<% if @node.visible -%>
<%= render :partial => "map", :object => @node %>
<% end -%>

<div class='column-1'>
<%= render :partial => "node_details", :object => @node %>
</div>
</div>
8 changes: 6 additions & 2 deletions app/views/browse/node_history.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<%
@name = printable_name @node
@name = printable_name @node, {:type => t('browse.node.node')}
@title = t('browse.node_history.node_history') + ' | ' + @name
%>
<% content_for :head do %>
Expand All @@ -9,7 +9,6 @@
<% content_for :heading do %>
<h2><%= raw t'browse.node_history.node_history_title', :node_name => link_to(h(@name), :action => "node", :id => @node.id) %></h2>
<ul class='secondary-actions clearfix'>
<li><%= link_to(t('browse.node_history.download_xml'), :controller => "old_node", :action => "history") %></li>
<li><%= link_to(t('browse.node_history.view_details'), :action => "node") %></li>
</ul>
<% end %>
Expand All @@ -20,6 +19,11 @@

<div class='column-1'>
<% @node.old_nodes.reverse.each do |node| %>
<div class="browse_details">
<div class='browse-section common'>
<div class='browse-section'> <%= render :partial => "common_editsummary", :object => node %></div>
</div>
</div>
<%= render :partial => "node_details", :object => node %>
<% end %>
</div>
17 changes: 9 additions & 8 deletions app/views/browse/relation.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
<%
@name = printable_name @relation
@name = printable_name @relation, {:type => t('browse.relation.relation')}
@title = t('browse.relation.relation') + ' | ' + @name
%>
<% content_for :head do %>
<%= stylesheet_link_tag 'browse' %>
<% end %>

<% content_for :heading do %>
<h2><%= t'browse.relation.relation_title', :relation_name => @name %></h2>
<ul class='secondary-actions clearfix'>
<li><%= link_to(t('browse.relation.download_xml'), :controller => "relation", :action => "read") %></li>
<li><%= link_to(t('browse.relation.view_history'), :action => "relation_history") %></li>
</ul>
<h2><%= @name %></h2>
<%= render :partial => "navigation" %>

<%= render :partial => "common_editsummary", :object => @relation %>

(<%= link_to(t('browse.relation.view_history'), :action => "relation_history") %>)

<% end %>
<%= render :partial => "navigation" %>
<%= render :partial => "map", :object => @relation %>

<div class='column-1'>
<%= render :partial => "relation_details", :object => @relation %>
</div>
</div>
Loading