From 2b4b2f344206538ef5731e7fb5017a86c0eeec96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 17 Aug 2021 22:29:11 +0200 Subject: [PATCH] Use aria-label in admin table actions This way screen reader users will know which record they're going to access when focusing on a link to a certain action. Otherwise they'd hear something like "Edit, link", and they wouldn't know which record they'll end up editing if they follow the link. --- app/components/admin/action_component.rb | 19 ++++- .../admin/table_actions_component.rb | 2 +- app/models/application_record.rb | 1 + app/models/concerns/human_name.rb | 9 ++ app/models/dashboard/administrator_task.rb | 4 + app/models/local_census_record.rb | 4 + app/models/poll/booth_assignment.rb | 2 + app/models/poll/shift.rb | 4 + config/locales/en/admin.yml | 1 + config/locales/es/admin.yml | 1 + .../components/admin/action_component_spec.rb | 54 ++++++++++++ .../admin/table_actions_component_spec.rb | 8 +- spec/models/human_name_spec.rb | 85 +++++++++++++++++++ 13 files changed, 189 insertions(+), 5 deletions(-) create mode 100644 app/models/concerns/human_name.rb create mode 100644 spec/models/human_name_spec.rb diff --git a/app/components/admin/action_component.rb b/app/components/admin/action_component.rb index ab499d8fb..08c5916b1 100644 --- a/app/components/admin/action_component.rb +++ b/app/components/admin/action_component.rb @@ -21,14 +21,23 @@ class Admin::ActionComponent < ApplicationComponent def html_options { class: html_class, + "aria-label": label, data: { confirm: confirmation_text } - }.merge(options.except(:confirm, :path, :text)) + }.merge(options.except(:"aria-label", :confirm, :path, :text)) end def html_class "#{action.to_s.gsub("_", "-")}-link" end + def label + if options[:"aria-label"] == true + t("admin.actions.label", action: text, name: record_name) + else + options[:"aria-label"] + end + end + def confirmation_text if options[:confirm] == true t("admin.actions.confirm") @@ -37,6 +46,14 @@ class Admin::ActionComponent < ApplicationComponent end end + def record_name + if record.respond_to?(:human_name) + record.human_name + else + record.to_s.humanize + end + end + def default_path if %i[answers configure destroy preview show].include?(action.to_sym) namespaced_polymorphic_path(namespace, record) diff --git a/app/components/admin/table_actions_component.rb b/app/components/admin/table_actions_component.rb index 9a159e8ab..6df23a8d4 100644 --- a/app/components/admin/table_actions_component.rb +++ b/app/components/admin/table_actions_component.rb @@ -7,7 +7,7 @@ class Admin::TableActionsComponent < ApplicationComponent end def action(action_name, **args) - render Admin::ActionComponent.new(action_name, record, **args) + render Admin::ActionComponent.new(action_name, record, "aria-label": true, **args) end private diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 50235a9ca..ebc177b0d 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -1,4 +1,5 @@ class ApplicationRecord < ActiveRecord::Base + include HumanName self.abstract_class = true def self.sample(count = 1) diff --git a/app/models/concerns/human_name.rb b/app/models/concerns/human_name.rb new file mode 100644 index 000000000..a94396b9a --- /dev/null +++ b/app/models/concerns/human_name.rb @@ -0,0 +1,9 @@ +module HumanName + def human_name + %i[title name subject].each do |method| + return send(method) if respond_to?(method) + end + + raise "Must implement a method defining a human name" + end +end diff --git a/app/models/dashboard/administrator_task.rb b/app/models/dashboard/administrator_task.rb index 8fd78a28a..e0091d0a4 100644 --- a/app/models/dashboard/administrator_task.rb +++ b/app/models/dashboard/administrator_task.rb @@ -8,4 +8,8 @@ class Dashboard::AdministratorTask < ApplicationRecord scope :pending, -> { where(executed_at: nil) } scope :done, -> { where.not(executed_at: nil) } + + def title + "#{source.proposal.title} #{source.action.title}" + end end diff --git a/app/models/local_census_record.rb b/app/models/local_census_record.rb index a812a4144..a9cea9058 100644 --- a/app/models/local_census_record.rb +++ b/app/models/local_census_record.rb @@ -10,6 +10,10 @@ class LocalCensusRecord < ApplicationRecord scope :search, ->(terms) { where("document_number ILIKE ?", "%#{terms}%") } + def title + "#{ApplicationController.helpers.humanize_document_type(document_type)} #{document_number}" + end + private def sanitize diff --git a/app/models/poll/booth_assignment.rb b/app/models/poll/booth_assignment.rb index e525b6ed7..e3529e06a 100644 --- a/app/models/poll/booth_assignment.rb +++ b/app/models/poll/booth_assignment.rb @@ -3,6 +3,8 @@ class Poll belongs_to :booth belongs_to :poll + delegate :name, to: :booth + before_destroy :destroy_poll_shifts, only: :destroy has_many :officer_assignments, dependent: :destroy diff --git a/app/models/poll/shift.rb b/app/models/poll/shift.rb index 5fc8ca9b9..080e6170c 100644 --- a/app/models/poll/shift.rb +++ b/app/models/poll/shift.rb @@ -16,6 +16,10 @@ class Poll after_create :create_officer_assignments before_destroy :destroy_officer_assignments + def title + "#{I18n.t("admin.poll_shifts.#{task}")} #{officer_name} #{I18n.l(date.to_date, format: :long)}" + end + def persist_data self.officer_name = officer.name self.officer_email = officer.email diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 6e29759b7..bd7dac056 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -8,6 +8,7 @@ en: confirm_hide: Confirm moderation hide: Hide hide_author: Hide author + label: "%{action} %{name}" restore: Restore mark_featured: Featured unmark_featured: Unmark featured diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index f27a7c35d..36605d887 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -8,6 +8,7 @@ es: confirm_hide: Confirmar moderación hide: Ocultar hide_author: Bloquear al autor + label: "%{action} %{name}" restore: Volver a mostrar mark_featured: Destacar unmark_featured: Quitar destacado diff --git a/spec/components/admin/action_component_spec.rb b/spec/components/admin/action_component_spec.rb index 901571f73..521796671 100644 --- a/spec/components/admin/action_component_spec.rb +++ b/spec/components/admin/action_component_spec.rb @@ -6,4 +6,58 @@ describe Admin::ActionComponent do expect(page).to have_css "a.edit-link" end + + describe "aria-label attribute" do + it "is not rendered by default" do + record = double(human_name: "Stay home") + + render_inline Admin::ActionComponent.new(:edit, record, path: "/") + + expect(page).to have_link count: 1 + expect(page).not_to have_css "[aria-label]" + end + + it "is not rendered when aria-label is nil" do + render_inline Admin::ActionComponent.new(:edit, double, path: "/", "aria-label": nil) + + expect(page).to have_link count: 1 + expect(page).not_to have_css "[aria-label]" + end + + it "renders with the given value" do + render_inline Admin::ActionComponent.new(:edit, double, path: "/", "aria-label": "Modify") + + expect(page).to have_link count: 1 + expect(page).to have_css "[aria-label='Modify']" + end + + context "when aria-label is true" do + it "includes the action and the human_name of the record" do + record = double(human_name: "Stay home") + + render_inline Admin::ActionComponent.new(:edit, record, path: "/", "aria-label": true) + + expect(page).to have_link count: 1 + expect(page).to have_css "a[aria-label='Edit Stay home']", exact_text: "Edit" + end + + it "uses the to_s method when there's no human_name" do + record = double(to_s: "do_not_go_out") + + render_inline Admin::ActionComponent.new(:edit, record, path: "/", "aria-label": true) + + expect(page).to have_link count: 1 + expect(page).to have_css "a[aria-label='Edit Do not go out']", exact_text: "Edit" + end + + it "uses the human_name when there are both human_name and to_s" do + record = double(human_name: "Stay home", to_s: "do_not_go_out") + + render_inline Admin::ActionComponent.new(:edit, record, path: "/", "aria-label": true) + + expect(page).to have_link count: 1 + expect(page).to have_css "a[aria-label='Edit Stay home']", exact_text: "Edit" + end + end + end end diff --git a/spec/components/admin/table_actions_component_spec.rb b/spec/components/admin/table_actions_component_spec.rb index 89d20a7d6..3c710f3cc 100644 --- a/spec/components/admin/table_actions_component_spec.rb +++ b/spec/components/admin/table_actions_component_spec.rb @@ -1,14 +1,16 @@ require "rails_helper" describe Admin::TableActionsComponent, controller: Admin::BaseController do - let(:record) { create(:banner) } + let(:record) { create(:banner, title: "Important!") } it "renders links to edit and destroy a record by default" do render_inline Admin::TableActionsComponent.new(record) expect(page).to have_css "a", count: 2 - expect(page).to have_css "a[href*='edit']", text: "Edit" - expect(page).to have_css "a[data-method='delete']", text: "Delete" + expect(page).to have_css "a[href*='edit']", exact_text: "Edit" + expect(page).to have_css "a[aria-label='Edit Important!']", exact_text: "Edit" + expect(page).to have_css "a[data-method='delete']", exact_text: "Delete" + expect(page).to have_css "a[aria-label='Delete Important!']", exact_text: "Delete" end context "actions parameter is passed" do diff --git a/spec/models/human_name_spec.rb b/spec/models/human_name_spec.rb new file mode 100644 index 000000000..6fb2b54b5 --- /dev/null +++ b/spec/models/human_name_spec.rb @@ -0,0 +1,85 @@ +require "rails_helper" + +describe HumanName do + describe "#human_name" do + it "uses the title when available" do + model = Class.new do + include HumanName + + def title + "I am fire" + end + end + + expect(model.new.human_name).to eq "I am fire" + end + + it "uses the name when available" do + model = Class.new do + include HumanName + + def name + "Be like water" + end + end + + expect(model.new.human_name).to eq "Be like water" + end + + it "uses the subject when available" do + model = Class.new do + include HumanName + + def subject + "20% off on fire and water" + end + end + + expect(model.new.human_name).to eq "20% off on fire and water" + end + + it "prioritizes title over name and subject" do + model = Class.new do + include HumanName + + def title + "I am fire" + end + + def name + "Be like water" + end + + def subject + "20% off on fire and water" + end + end + + expect(model.new.human_name).to eq "I am fire" + end + + it "prioritizes name over subject" do + model = Class.new do + include HumanName + + def name + "Be like water" + end + + def subject + "20% off on fire and water" + end + end + + expect(model.new.human_name).to eq "Be like water" + end + + it "raises an exception when no methods are defined" do + model = Class.new do + include HumanName + end + + expect { model.new.human_name }.to raise_error RuntimeError + end + end +end