From 0403dbb23c01769693295232eab1c29edb340bf0 Mon Sep 17 00:00:00 2001 From: Daniel Ritz Date: Sat, 23 Jan 2016 18:48:57 +0100 Subject: [PATCH] Improve accessibility for icon-only links Adds a for all icon-only links. The text in the span is automatically set to the :title attribute. --- app/helpers/application_helper.rb | 59 ++++++++++++++++++------- app/helpers/email_addresses_helper.rb | 8 ++-- app/helpers/issues_helper.rb | 8 ++-- app/helpers/journals_helper.rb | 39 ++++++++-------- app/helpers/watchers_helper.rb | 7 ++- app/views/attachments/_links.html.erb | 28 ++++++------ app/views/issues/_attributes.html.erb | 30 ++++++------- app/views/issues/_relations.html.erb | 15 +++---- app/views/my/blocks/_timelog.html.erb | 14 +++--- app/views/news/show.html.erb | 8 ++-- app/views/reports/issue_report.html.erb | 35 ++++++++++++--- app/views/repositories/_related_issues.html.erb | 19 ++++---- app/views/roles/permissions.html.erb | 7 ++- app/views/settings/_repositories.html.erb | 12 +++-- app/views/timelog/_list.html.erb | 16 +++---- test/unit/helpers/application_helper_test.rb | 12 +++-- 16 files changed, 182 insertions(+), 135 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 12dccea..8ace68d 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -37,6 +37,34 @@ module ApplicationHelper User.current.allowed_to?({:controller => controller, :action => action}, @project) end + # Creates a link with only an icon, without visible text. A hidden is added + # using the :title attribute for accessibility + # + # @param [String or Hash] url + # @param [Hash] html_options Options passed to link_to + def icon_link(url, options = {}) + link_to(icon_link_content(options), url, options) + end + + # Like link_to_function for icon only links, similar to icon_link + # + # @param [String] funct The function + # @param [Hash] html_options Options passed to link_to + def icon_link_to_function(funct, options) + link_to_function(icon_link_content(options), options) + end + + # Display an icon-only link if user is authorized + # + # @param [Hash] options Hash params. This will checked by authorize_for to see if the user is authorized + # @param [optional, Hash] html_options Options passed to link_to + # @param [optional, Hash] parameters_for_method_reference Extra parameters for link_to + def icon_link_if_authorized(options = {}, html_options = nil, *parameters_for_method_reference) + if authorize_for(options[:controller] || params[:controller], options[:action]) + link_to(icon_link_content(html_options), options, html_options, *parameters_for_method_reference) + end + end + # Display a link if user is authorized # # @param [String] name Anchor text (passed to link_to) @@ -454,18 +482,14 @@ module ApplicationHelper end def reorder_links(name, url, method = :post) - link_to('', - url.merge({"#{name}[move_to]" => 'highest'}), :method => method, - :title => l(:label_sort_highest), :class => 'icon-only icon-move-top') + - link_to('', - url.merge({"#{name}[move_to]" => 'higher'}), :method => method, - :title => l(:label_sort_higher), :class => 'icon-only icon-move-up') + - link_to('', - url.merge({"#{name}[move_to]" => 'lower'}), :method => method, - :title => l(:label_sort_lower), :class => 'icon-only icon-move-down') + - link_to('', - url.merge({"#{name}[move_to]" => 'lowest'}), :method => method, - :title => l(:label_sort_lowest), :class => 'icon-only icon-move-bottom') + icon_link(url.merge({"#{name}[move_to]" => 'highest'}), :method => method, + :title => l(:label_sort_highest), :class => 'icon-only icon-move-top') + + icon_link(url.merge({"#{name}[move_to]" => 'higher'}), :method => method, + :title => l(:label_sort_higher), :class => 'icon-only icon-move-up') + + icon_link(url.merge({"#{name}[move_to]" => 'lower'}), :method => method, + :title => l(:label_sort_lower), :class => 'icon-only icon-move-down') + + icon_link(url.merge({"#{name}[move_to]" => 'lowest'}), :method => method, + :title => l(:label_sort_lowest), :class => 'icon-only icon-move-bottom') end def breadcrumb(*args) @@ -887,10 +911,10 @@ module ApplicationHelper @current_section += 1 if @current_section > 1 content_tag('div', - link_to('', options[:edit_section_links].merge(:section => @current_section), - :class => 'icon-only icon-edit'), + icon_link(options[:edit_section_links].merge(:section => @current_section), + :class => 'icon-only icon-edit', + :title => l(:button_edit_section)), :class => 'contextual', - :title => l(:button_edit_section), :id => "section-#{@current_section}") + heading.html_safe else heading @@ -1335,6 +1359,11 @@ module ApplicationHelper return self end + def icon_link_content(options = {}) + title = options[:title] + title.nil? ? '' : content_tag(:span, title, :class => 'hidden-for-sighted') + end + def link_to_content_update(text, url_params = {}, html_options = {}) link_to(text, url_params, html_options) end diff --git a/app/helpers/email_addresses_helper.rb b/app/helpers/email_addresses_helper.rb index c30aaa6..9af60fd 100644 --- a/app/helpers/email_addresses_helper.rb +++ b/app/helpers/email_addresses_helper.rb @@ -22,17 +22,17 @@ module EmailAddressesHelper # Returns a link to enable or disable notifications for the address def toggle_email_address_notify_link(address) if address.notify? - link_to '', + icon_link( user_email_address_path(address.user, address, :notify => '0'), :method => :put, :remote => true, :title => l(:label_disable_notifications), - :class => 'icon icon-email' + :class => 'icon icon-email') else - link_to '', + icon_link( user_email_address_path(address.user, address, :notify => '1'), :method => :put, :remote => true, :title => l(:label_enable_notifications), - :class => 'icon icon-email-disabled' + :class => 'icon icon-email-disabled') end end end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 3adc454..898dee8 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -442,10 +442,10 @@ module IssuesHelper # Link to the attachment if it has not been removed value = link_to_attachment(atta, :download => true, :only_path => options[:only_path]) if options[:only_path] != false && atta.is_text? - value += link_to('', - { :controller => 'attachments', :action => 'show', - :id => atta, :filename => atta.filename }, - :class => 'icon icon-magnifier') + value += icon_link({ :controller => 'attachments', :action => 'show', + :id => atta, :filename => atta.filename }, + :class => 'icon icon-magnifier', + :title => l(:button_view)) end else value = content_tag("i", h(value)) if value diff --git a/app/helpers/journals_helper.rb b/app/helpers/journals_helper.rb index 2a0a7d3..abbaa22 100644 --- a/app/helpers/journals_helper.rb +++ b/app/helpers/journals_helper.rb @@ -30,27 +30,24 @@ module JournalsHelper editable = User.current.logged? && (User.current.allowed_to?(:edit_issue_notes, issue.project) || (journal.user == User.current && User.current.allowed_to?(:edit_own_issue_notes, issue.project))) links = [] if !journal.notes.blank? - links << link_to('', - quoted_issue_path(issue, :journal_id => journal), - :remote => true, - :method => 'post', - :title => l(:button_quote), - :class => 'icon-only icon-comment' - ) if options[:reply_links] - links << link_to('', - edit_journal_path(journal), - :remote => true, - :method => 'get', - :title => l(:button_edit), - :class => 'icon-only icon-edit' - ) if editable - links << link_to('', - journal_path(journal, :notes => ""), - :remote => true, - :method => 'put', :data => {:confirm => l(:text_are_you_sure)}, - :title => l(:button_delete), - :class => 'icon-only icon-del' - ) if editable + links << icon_link(quoted_issue_path(issue, :journal_id => journal), + :remote => true, + :method => 'post', + :title => l(:button_quote), + :class => 'icon-only icon-comment' + ) if options[:reply_links] + links << icon_link(edit_journal_path(journal), + :remote => true, + :method => 'get', + :title => l(:button_edit), + :class => 'icon-only icon-edit' + ) if editable + links << icon_link(journal_path(journal, :notes => ""), + :remote => true, + :method => 'put', :data => {:confirm => l(:text_are_you_sure)}, + :title => l(:button_delete), + :class => 'icon-only icon-del' + ) if editable end content << content_tag('div', links.join(' ').html_safe, :class => 'contextual') unless links.empty? content << textilizable(journal, :notes) diff --git a/app/helpers/watchers_helper.rb b/app/helpers/watchers_helper.rb index 9a5de74..f4a5b7c 100644 --- a/app/helpers/watchers_helper.rb +++ b/app/helpers/watchers_helper.rb @@ -58,10 +58,9 @@ module WatchersHelper :object_id => object.id, :user_id => user} s << ' ' - s << link_to('', url, - :remote => true, :method => 'delete', - :class => "delete icon-only icon-del", - :title => l(:button_delete)) + s << icon_link(url, :remote => true, :method => 'delete', + :class => "delete icon-only icon-del", + :title => l(:button_delete)) end content << content_tag('li', s, :class => "user-#{user.id}") end diff --git a/app/views/attachments/_links.html.erb b/app/views/attachments/_links.html.erb index 53324c2..e3cc139 100644 --- a/app/views/attachments/_links.html.erb +++ b/app/views/attachments/_links.html.erb @@ -1,28 +1,26 @@
- <%= link_to('', - container_attachments_edit_path(container), - :title => l(:label_edit_attachments), - :class => 'icon-only icon-edit' - ) if options[:editable] %> + <%= icon_link(container_attachments_edit_path(container), + :title => l(:label_edit_attachments), + :class => 'icon-only icon-edit' + ) if options[:editable] %>
<% for attachment in attachments %>

<%= link_to_attachment attachment, :class => 'icon icon-attachment', :download => true -%> <% if attachment.is_text? %> - <%= link_to '', - { :controller => 'attachments', :action => 'show', - :id => attachment, :filename => attachment.filename }, - :class => 'icon icon-magnifier', - :title => l(:button_view) %> + <%= icon_link({ :controller => 'attachments', :action => 'show', + :id => attachment, :filename => attachment.filename }, + :class => 'icon icon-magnifier', + :title => l(:button_view)) %> <% end %> <%= " - #{attachment.description}" unless attachment.description.blank? %> (<%= number_to_human_size attachment.filesize %>) <% if options[:deletable] %> - <%= link_to '', attachment_path(attachment), - :data => {:confirm => l(:text_are_you_sure)}, - :method => :delete, - :class => 'delete icon-only icon-del', - :title => l(:button_delete) %> + <%= icon_link(attachment_path(attachment), + :data => {:confirm => l(:text_are_you_sure)}, + :method => :delete, + :class => 'delete icon-only icon-del', + :title => l(:button_delete)) %> <% end %> <% if options[:author] %> <%= attachment.author %>, <%= format_time(attachment.created_on) %> diff --git a/app/views/issues/_attributes.html.erb b/app/views/issues/_attributes.html.erb index f0a9890..bcfb4cb 100644 --- a/app/views/issues/_attributes.html.erb +++ b/app/views/issues/_attributes.html.erb @@ -20,26 +20,24 @@ <% if @issue.safe_attribute?('category_id') && @issue.project.issue_categories.any? %>

<%= f.select :category_id, (@issue.project.issue_categories.collect {|c| [c.name, c.id]}), :include_blank => true, :required => @issue.required_attribute?('category_id') %> -<%= link_to('', - new_project_issue_category_path(@issue.project), - :remote => true, - :method => 'get', - :title => l(:label_issue_category_new), - :tabindex => 200, - :class => 'icon-only icon-add' - ) if User.current.allowed_to?(:manage_categories, @issue.project) %>

+<%= icon_link(new_project_issue_category_path(@issue.project), + :remote => true, + :method => 'get', + :title => l(:label_issue_category_new), + :tabindex => 200, + :class => 'icon-only icon-add' + ) if User.current.allowed_to?(:manage_categories, @issue.project) %>

<% end %> <% if @issue.safe_attribute?('fixed_version_id') && @issue.assignable_versions.any? %>

<%= f.select :fixed_version_id, version_options_for_select(@issue.assignable_versions, @issue.fixed_version), :include_blank => true, :required => @issue.required_attribute?('fixed_version_id') %> -<%= link_to('', - new_project_version_path(@issue.project), - :remote => true, - :method => 'get', - :title => l(:label_version_new), - :tabindex => 200, - :class => 'icon-only icon-add' - ) if User.current.allowed_to?(:manage_versions, @issue.project) %> +<%= icon_link(new_project_version_path(@issue.project), + :remote => true, + :method => 'get', + :title => l(:label_version_new), + :tabindex => 200, + :class => 'icon-only icon-add' + ) if User.current.allowed_to?(:manage_versions, @issue.project) %>

<% end %>
diff --git a/app/views/issues/_relations.html.erb b/app/views/issues/_relations.html.erb index b79e744..0fb21df 100644 --- a/app/views/issues/_relations.html.erb +++ b/app/views/issues/_relations.html.erb @@ -19,14 +19,13 @@ <%= other_issue.status.name %> <%= format_date(other_issue.start_date) %> <%= format_date(other_issue.due_date) %> - <%= link_to('', - relation_path(relation), - :remote => true, - :method => :delete, - :data => {:confirm => l(:text_are_you_sure)}, - :title => l(:label_relation_delete), - :class => 'icon-only icon-link-break' - ) if User.current.allowed_to?(:manage_issue_relations, @project) %> + <%= icon_link(relation_path(relation), + :remote => true, + :method => :delete, + :data => {:confirm => l(:text_are_you_sure)}, + :title => l(:label_relation_delete), + :class => 'icon-only icon-link-break' + ) if User.current.allowed_to?(:manage_issue_relations, @project) %> <% end %> diff --git a/app/views/my/blocks/_timelog.html.erb b/app/views/my/blocks/_timelog.html.erb index 76b974e..597b343 100644 --- a/app/views/my/blocks/_timelog.html.erb +++ b/app/views/my/blocks/_timelog.html.erb @@ -42,13 +42,13 @@ entries_by_day = entries.group_by(&:spent_on) <%= html_hours("%.2f" % entry.hours) %> <% if entry.editable_by?(@user) -%> - <%= link_to '', {:controller => 'timelog', :action => 'edit', :id => entry}, - :title => l(:button_edit), - :class => 'icon-only icon-edit' %> - <%= link_to '', {:controller => 'timelog', :action => 'destroy', :id => entry}, - :data => {:confirm => l(:text_are_you_sure)}, :method => :delete, - :title => l(:button_delete), - :class => 'icon-only icon-del' %> + <%= icon_link({:controller => 'timelog', :action => 'edit', :id => entry}, + :title => l(:button_edit), + :class => 'icon-only icon-edit') %> + <%= icon_link({:controller => 'timelog', :action => 'destroy', :id => entry}, + :data => {:confirm => l(:text_are_you_sure)}, :method => :delete, + :title => l(:button_delete), + :class => 'icon-only icon-del') %> <% end -%> diff --git a/app/views/news/show.html.erb b/app/views/news/show.html.erb index 0390964..eb63d3e 100644 --- a/app/views/news/show.html.erb +++ b/app/views/news/show.html.erb @@ -36,10 +36,10 @@ <% @comments.each do |comment| %> <% next if comment.new_record? %>
- <%= link_to_if_authorized '', {:controller => 'comments', :action => 'destroy', :id => @news, :comment_id => comment}, - :data => {:confirm => l(:text_are_you_sure)}, :method => :delete, - :title => l(:button_delete), - :class => 'icon-only icon-del' %> + <%= icon_link_if_authorized({:controller => 'comments', :action => 'destroy', :id => @news, :comment_id => comment}, + :data => {:confirm => l(:text_are_you_sure)}, :method => :delete, + :title => l(:button_delete), + :class => 'icon-only icon-del') %>

<%= avatar(comment.author, :size => "24") %><%= authoring comment.created_on, comment.author %>

diff --git a/app/views/reports/issue_report.html.erb b/app/views/reports/issue_report.html.erb index 1cb3ccd..f1ea515 100644 --- a/app/views/reports/issue_report.html.erb +++ b/app/views/reports/issue_report.html.erb @@ -3,25 +3,37 @@

<%=l(:field_tracker)%>  - <%= link_to '', project_issues_report_details_path(@project, :detail => 'tracker'), :class => 'icon-only icon-zoom-in' %> + <%= icon_link project_issues_report_details_path(@project, :detail => 'tracker'), + :class => 'icon-only icon-zoom-in', + :title => l(:label_zoom_in) + %>

<%= render :partial => 'simple', :locals => { :data => @issues_by_tracker, :field_name => "tracker_id", :rows => @trackers } %>

<%=l(:field_priority)%>  - <%= link_to '', project_issues_report_details_path(@project, :detail => 'priority'), :class => 'icon-only icon-zoom-in' %> + <%= icon_link project_issues_report_details_path(@project, :detail => 'priority'), + :class => 'icon-only icon-zoom-in', + :title => l(:label_zoom_in) + %>

<%= render :partial => 'simple', :locals => { :data => @issues_by_priority, :field_name => "priority_id", :rows => @priorities } %>

<%=l(:field_assigned_to)%>  - <%= link_to '', project_issues_report_details_path(@project, :detail => 'assigned_to'), :class => 'icon-only icon-zoom-in' %> + <%= icon_link project_issues_report_details_path(@project, :detail => 'assigned_to'), + :class => 'icon-only icon-zoom-in', + :title => l(:label_zoom_in) + %>

<%= render :partial => 'simple', :locals => { :data => @issues_by_assigned_to, :field_name => "assigned_to_id", :rows => @assignees } %>

<%=l(:field_author)%>  - <%= link_to '', project_issues_report_details_path(@project, :detail => 'author'), :class => 'icon-only icon-zoom-in' %> + <%= icon_link project_issues_report_details_path(@project, :detail => 'author'), + :class => 'icon-only icon-zoom-in', + :title => l(:label_zoom_in) + %>

<%= render :partial => 'simple', :locals => { :data => @issues_by_author, :field_name => "author_id", :rows => @authors } %>
@@ -31,21 +43,30 @@

<%=l(:field_version)%>  - <%= link_to '', project_issues_report_details_path(@project, :detail => 'version'), :class => 'icon-only icon-zoom-in' %> + <%= icon_link project_issues_report_details_path(@project, :detail => 'version'), + :class => 'icon-only icon-zoom-in', + :title => l(:label_zoom_in) + %>

<%= render :partial => 'simple', :locals => { :data => @issues_by_version, :field_name => "fixed_version_id", :rows => @versions } %>
<% if @project.children.any? %>

<%=l(:field_subproject)%>  - <%= link_to '', project_issues_report_details_path(@project, :detail => 'subproject'), :class => 'icon-only icon-zoom-in' %> + <%= icon_link project_issues_report_details_path(@project, :detail => 'subproject'), + :class => 'icon-only icon-zoom-in', + :title => l(:label_zoom_in) + %>

<%= render :partial => 'simple', :locals => { :data => @issues_by_subproject, :field_name => "project_id", :rows => @subprojects } %>
<% end %>

<%=l(:field_category)%>  - <%= link_to '', project_issues_report_details_path(@project, :detail => 'category'), :class => 'icon-only icon-zoom-in' %> + <%= icon_link project_issues_report_details_path(@project, :detail => 'category'), + :class => 'icon-only icon-zoom-in', + :title => l(:label_zoom_in) + %>

<%= render :partial => 'simple', :locals => { :data => @issues_by_category, :field_name => "category_id", :rows => @categories } %>
diff --git a/app/views/repositories/_related_issues.html.erb b/app/views/repositories/_related_issues.html.erb index ea45391..f39618a 100644 --- a/app/views/repositories/_related_issues.html.erb +++ b/app/views/repositories/_related_issues.html.erb @@ -11,16 +11,15 @@
    <% @changeset.issues.visible.each do |issue| %>
  • "><%= link_to_issue issue %> - <%= link_to('', - {:controller => 'repositories', :action => 'remove_related_issue', - :id => @project, :repository_id => @repository.identifier_param, - :rev => @changeset.identifier, :issue_id => issue}, - :remote => true, - :method => :delete, - :data => {:confirm => l(:text_are_you_sure)}, - :title => l(:label_relation_delete), - :class => 'icon-only icon-link-break' - ) if manage_allowed %> + <%= icon_link({:controller => 'repositories', :action => 'remove_related_issue', + :id => @project, :repository_id => @repository.identifier_param, + :rev => @changeset.identifier, :issue_id => issue}, + :remote => true, + :method => :delete, + :data => {:confirm => l(:text_are_you_sure)}, + :title => l(:label_relation_delete), + :class => 'icon-only icon-link-break' + ) if manage_allowed %>
    • <% end %>
diff --git a/app/views/roles/permissions.html.erb b/app/views/roles/permissions.html.erb index b403cff..d20dc94 100644 --- a/app/views/roles/permissions.html.erb +++ b/app/views/roles/permissions.html.erb @@ -9,10 +9,9 @@ <%=l(:label_permissions)%> <% @roles.each do |role| %> - <%= link_to_function('', - "toggleCheckboxesBySelector('input.role-#{role.id}')", - :title => "#{l(:button_check_all)}/#{l(:button_uncheck_all)}", - :class => 'icon-only icon-checked') %> + <%= icon_link_to_function("toggleCheckboxesBySelector('input.role-#{role.id}')", + :title => "#{l(:button_check_all)}/#{l(:button_uncheck_all)}", + :class => 'icon-only icon-checked') %> <%= content_tag(role.builtin? ? 'em' : 'span', role.name) %> <% end %> diff --git a/app/views/settings/_repositories.html.erb b/app/views/settings/_repositories.html.erb index bb7a237..56b7d70 100644 --- a/app/views/settings/_repositories.html.erb +++ b/app/views/settings/_repositories.html.erb @@ -119,8 +119,10 @@ ) %> - <%= link_to('', '#', - :class => 'delete-commit-keywords icon-only icon-del') %> + <%= icon_link('#', + :class => 'delete-commit-keywords icon-only icon-del', + :title => l(:button_delete) + ) %> <% end %> @@ -130,8 +132,10 @@ - <%= link_to('', '#', - :class => 'add-commit-keywords icon-only icon-add') %> + <%= icon_link('#', + :class => 'add-commit-keywords icon-only icon-add', + :title => l(:button_add) + ) %> diff --git a/app/views/timelog/_list.html.erb b/app/views/timelog/_list.html.erb index df497d4..50bd0c4 100644 --- a/app/views/timelog/_list.html.erb +++ b/app/views/timelog/_list.html.erb @@ -21,14 +21,14 @@ <%= raw @query.inline_columns.map {|column| "#{column_content(column, entry)}"}.join %> <% if entry.editable_by?(User.current) -%> - <%= link_to '', edit_time_entry_path(entry), - :title => l(:button_edit), - :class => 'icon icon-edit' %> - <%= link_to '', time_entry_path(entry), - :data => {:confirm => l(:text_are_you_sure)}, - :method => :delete, - :title => l(:button_delete), - :class => 'icon-only icon-del' %> + <%= icon_link edit_time_entry_path(entry), + :title => l(:button_edit), + :class => 'icon icon-edit' %> + <%= icon_link time_entry_path(entry), + :data => {:confirm => l(:text_are_you_sure)}, + :method => :delete, + :title => l(:button_delete), + :class => 'icon-only icon-del' %> <% end -%> diff --git a/test/unit/helpers/application_helper_test.rb b/test/unit/helpers/application_helper_test.rb index 6c0d7ab..7d00daa 100644 --- a/test/unit/helpers/application_helper_test.rb +++ b/test/unit/helpers/application_helper_test.rb @@ -1242,15 +1242,19 @@ RAW result = textilizable(raw, :edit_section_links => {:controller => 'wiki', :action => 'edit', :project_id => '1', :id => 'Test'}).gsub("\n", "") # heading that contains inline code - assert_match Regexp.new('
' + - '
' + + assert_match Regexp.new('' + '' + '

Subtitle with inline code

'), result # last heading - assert_match Regexp.new('
' + - '
' + + assert_match Regexp.new('' + '' + '

Subtitle after pre tag

'), result -- 2.6.3