diff --git a/app/helpers/investigations_helper.rb b/app/helpers/investigations_helper.rb index ffa66162d7..2959565e48 100644 --- a/app/helpers/investigations_helper.rb +++ b/app/helpers/investigations_helper.rb @@ -568,6 +568,17 @@ def non_search_cases_page_names %w[team_cases your_cases assigned_cases].freeze end + def calculate_row_index(investigation_counter, row_number) + # Each investigation has 3 rows (title, meta, status) + # So for investigation 0, rows are 1,2,3 + # For investigation 1, rows are 4,5,6 etc. + (investigation_counter * 3) + row_number + end + + def investigation_owner(investigation) + sanitize(investigation.owner_display_name_for(viewer: current_user)) + end + private def search_result_values(_search_terms, number_of_results) diff --git a/app/views/investigations/_full_table_body.html.erb b/app/views/investigations/_full_table_body.html.erb index 8dd937db0f..969a144e16 100644 --- a/app/views/investigations/_full_table_body.html.erb +++ b/app/views/investigations/_full_table_body.html.erb @@ -1,54 +1,58 @@ - - - Notification title - - - <%= link_to sanitize(investigation.title), investigation_path(investigation), class: "govuk-link govuk-link--no-visited-state" %> + + + + Notification number: <%= investigation.pretty_id %>. + <%= non_search_cases_page_names.include?(@page_name) ? "Created: #{investigation.created_at.to_formatted_s(:govuk)}" : "Owner: #{investigation_owner(investigation)}" %>. + <%= @page_name == "assigned_cases" ? "Assigner: #{sanitize(investigation.owner_team&.name)}" : "Hazard type: #{sanitize(investigation.hazard_type.presence || "Not provided")}" %>. + Product count: <%= pluralize investigation.products.count, "product" %>. + <%= sorted_by == SortByHelper::SORT_BY_CREATED_AT ? "Created: #{time_ago_in_words(investigation.created_at)} ago" : "Updated: #{date_or_recent_time_ago investigation.updated_at}" %>. + + <%= link_to sanitize(investigation.title), investigation_path(investigation), class: "govuk-link govuk-link--no-visited-state", "aria-describedby": "desc-#{investigation.id}" %> - - + + Metadata - + <%= investigation.pretty_id %> <% if non_search_cases_page_names.include? @page_name %> - + <%= investigation.created_at.to_formatted_s(:govuk) %> <% else %> - - <%= sanitize(investigation.owner_display_name_for(viewer: current_user)) %> + + <%= investigation_owner(investigation) %> <% end %> <% if @page_name == "assigned_cases" %> - + <%= sanitize(investigation.owner_team&.name) %> <% else %> - + <%= sanitize(investigation.hazard_type.presence || "Not provided") %> <% end %> - + <%= pluralize investigation.products.count, "product" %> <% if sorted_by == SortByHelper::SORT_BY_CREATED_AT %> - + <%= "#{time_ago_in_words(investigation.created_at)} ago" %> <% else %> - + <%= date_or_recent_time_ago investigation.updated_at %> <% end %> - <%= render "investigations/status_table_row", investigation: investigation %> + <%= render "investigations/status_table_row", investigation: investigation, investigation_counter: investigation_counter %> diff --git a/app/views/investigations/_restricted_table_body.html.erb b/app/views/investigations/_restricted_table_body.html.erb index fffe66c693..0ac89abef2 100644 --- a/app/views/investigations/_restricted_table_body.html.erb +++ b/app/views/investigations/_restricted_table_body.html.erb @@ -1,22 +1,25 @@ - - - Notification title - - + + + + Notification number: <%= investigation.pretty_id %>. + <%= non_search_cases_page_names.include?(page_name) ? "Created: #{investigation.created_at.to_formatted_s(:govuk)}" : "Owner: #{investigation_owner(investigation)}" %>. + <%= page_name == "assigned_cases" ? "Assigner: #{sanitize(investigation.owner_team&.name)}" : "Hazard type: #{sanitize(investigation.hazard_type.presence || "Not provided")}" %>. + Product count: <%= pluralize investigation.products.count, "product" %>. + <%= "#{sanitize(investigation.case_type&.capitalize)} restricted" %> - - + + Metadata - + <%= investigation.pretty_id %> - +   - <%= render "investigations/status_table_row", investigation: investigation %> + <%= render "investigations/status_table_row", investigation: investigation, investigation_counter: investigation_counter %> diff --git a/app/views/investigations/_status_table_row.html.erb b/app/views/investigations/_status_table_row.html.erb index 2f0411b76c..556c943290 100644 --- a/app/views/investigations/_status_table_row.html.erb +++ b/app/views/investigations/_status_table_row.html.erb @@ -21,7 +21,7 @@ <% end %> <% end %> - + Type diff --git a/app/views/investigations/_table_body.html.erb b/app/views/investigations/_table_body.html.erb index f5f8505174..cc8b5cd7df 100644 --- a/app/views/investigations/_table_body.html.erb +++ b/app/views/investigations/_table_body.html.erb @@ -1,5 +1,5 @@ <% if policy(investigation).view_non_protected_details? %> - <%= render "investigations/full_table_body", investigation: investigation.decorate, sorted_by: sorted_by, page_name: page_name %> + <%= render "investigations/full_table_body", investigation: investigation.decorate, sorted_by: sorted_by, page_name: page_name, investigation_counter: investigation_counter %> <% else %> - <%= render "investigations/restricted_table_body", investigation: investigation.decorate, page_name: page_name %> + <%= render "investigations/restricted_table_body", investigation: investigation.decorate, page_name: page_name, investigation_counter: investigation_counter %> <% end %> diff --git a/app/views/notifications/create/search_for_or_add_a_business.html.erb b/app/views/notifications/create/search_for_or_add_a_business.html.erb index a2b941cc05..8dbea7edee 100644 --- a/app/views/notifications/create/search_for_or_add_a_business.html.erb +++ b/app/views/notifications/create/search_for_or_add_a_business.html.erb @@ -65,14 +65,14 @@ <% if @records.any? %>

<% if @records_count == 1 %>There is currently 1 business.<% else %>There are currently <%= @records_count %> businesses.<% end %>

<%= - govuk_table do |table| + govuk_table(html_attributes: { role: "table" }) do |table| table.with_head do |head| head.with_row do |row| - row.with_cell(text: "Trading name") - row.with_cell(text: "Registered or Legal name") - row.with_cell(text: "Companies House number") - row.with_cell(text: "Address") - row.with_cell(text: "Select business".html_safe) + row.with_cell(text: "Trading name", header: true, html_attributes: { scope: "col" }) + row.with_cell(text: "Registered or Legal name", header: true, html_attributes: { scope: "col" }) + row.with_cell(text: "Companies House number", header: true, html_attributes: { scope: "col" }) + row.with_cell(text: "Address", header: true, html_attributes: { scope: "col" }) + row.with_cell(text: "Select business".html_safe, header: true, html_attributes: { scope: "col" }) end end @@ -86,8 +86,18 @@ end.join("
") end - select_button = form_with url: "#{wizard_path}?business_id=#{record.id}", method: :put, builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| - f.govuk_submit "Select", name: "draft", value: true, secondary: true + select_button = if @existing_business_ids.include?(record.id) + "" + else + " + Trading name: #{sanitize(record.trading_name)}. + Registered or Legal name: #{sanitize(record.legal_name)}. + Companies House number: #{sanitize(record.company_number)}. + Address: #{sanitize(addresses)}. + ".html_safe + + form_with(url: "#{wizard_path}?business_id=#{record.id}", method: :put, builder: GOVUKDesignSystemFormBuilder::FormBuilder) do |f| + f.govuk_submit "Select", name: "draft", value: true, secondary: true, "aria-describedby": "desc-#{record.id}" + end end body.with_row do |row| @@ -95,11 +105,7 @@ row.with_cell(text: sanitize(record.legal_name)) row.with_cell(text: sanitize(record.company_number)) row.with_cell(text: sanitize(addresses).html_safe) - if @existing_business_ids.include?(record.id) - row.with_cell(text: "") - else - row.with_cell(text: select_button) - end + row.with_cell(text: select_button.html_safe) end end end diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb index 30ab78aefb..db2d6b0126 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -30,25 +30,25 @@   - Notification number + Notification number <% if non_search_cases_page_names.include? @page_name %> - Notification created + Notification created <% else %> - Notification owner + Notification owner <% end %> <% if @page_name == "assigned_cases" %> - Assigner + Assigner <% else %> - Hazard type + Hazard type <% end %> - Product count + Product count <% if query_params[:sort_by] == SortByHelper::SORT_BY_CREATED_AT && non_search_cases_page_names.exclude?(@page_name) %> - Created + Created <% else %> - Updated + Updated <% end %> @@ -63,21 +63,23 @@   - Notification number - Notification <%= non_search_cases_page_names.include?(@page_name) ? "created" : "owner" %> + Notification number + "> + Notification <%= non_search_cases_page_names.include?(@page_name) ? "created" : "owner" %> + <% if @page_name == "assigned_cases" %> - Assigner + Assigner <% else %> - Hazard type + Hazard type <% end %> - Product count + Product count <% if query_params[:sort_by] == SortByHelper::SORT_BY_CREATED_AT && non_search_cases_page_names.exclude?(@page_name) %> - Created + Created <% else %> - Updated + Updated <% end %> diff --git a/app/views/notifications/your_notifications.html.erb b/app/views/notifications/your_notifications.html.erb index cd4c150377..a195a8f951 100644 --- a/app/views/notifications/your_notifications.html.erb +++ b/app/views/notifications/your_notifications.html.erb @@ -12,7 +12,7 @@ <% if @draft_notifications.any? %> <%= - govuk_table do |table| + govuk_table(html_attributes: { role: "table" }) do |table| if Flipper.enabled?(:submit_notification_reminder) table.with_caption(size: "m") do safe_join([ @@ -32,22 +32,44 @@ table.with_head do |head| head.with_row do |row| - row.with_cell(text: "Product name(s)") - row.with_cell(text: "Notification title") - row.with_cell(text: "Last updated") - row.with_cell(text: "Status") - row.with_cell(text: "Make changes/delete notification".html_safe) + row.with_cell(text: "Product name(s)", html_attributes: { scope: "col" }) + row.with_cell(text: "Notification title", html_attributes: { scope: "col" }) + row.with_cell(text: "Last updated", html_attributes: { scope: "col" }) + row.with_cell(text: "Status", html_attributes: { scope: "col" }) + row.with_cell(text: "Actions".html_safe, html_attributes: { scope: "col" }) end end - @draft_notifications.each do |notification| + @draft_notifications.each_with_index do |notification, index| table.with_body do |body| body.with_row do |row| - row.with_cell(header: true, text: sanitize(notification.investigation_products.decorate.map(&:product).map(&:name_with_brand).presence&.join("
") || "Not provided", tags: %w(br)).html_safe) - row.with_cell(text: sanitize(notification.user_title.presence || "Not provided")) - row.with_cell(text: notification.updated_at.to_formatted_s(:govuk)) - row.with_cell(text: govuk_tag(text: "Draft", colour: "grey").html_safe) - row.with_cell(text: sanitize("Make changes
Delete", tags: %w(a hr), attributes: %w(href class)).html_safe) + product_value = notification.investigation_products.decorate.map(&:product).map(&:name_with_brand).presence&.join(", ") || "Not provided" + title_value = notification.user_title.presence || "Not provided" + updated_value = notification.updated_at.to_formatted_s(:govuk) + status_value = "Draft" + + row.with_cell( + header: true, + text: sanitize(notification.investigation_products.decorate.map(&:product).map(&:name_with_brand).presence&.join("
") || "Not provided", tags: %w(br)).html_safe + ) + row.with_cell(text: sanitize(title_value)) + row.with_cell(text: updated_value) + row.with_cell(text: govuk_tag(text: status_value, colour: "grey").html_safe) + row.with_cell( + text: sanitize( + " + Product name(s): #{product_value}. + Notification title: #{title_value}. + Last updated: #{updated_value}. + Status: #{status_value}. + + Make changes +
+ Delete", + tags: %w(a hr span), + attributes: %w(href class id aria-describedby) + ).html_safe + ) end end end @@ -58,27 +80,47 @@ <% if @submitted_notifications.any? %> <%= - govuk_table do |table| + govuk_table(html_attributes: { role: "table" }) do |table| table.with_caption(size: "m", text: "Submitted notifications") table.with_head do |head| head.with_row do |row| - row.with_cell(text: "Product name(s)") - row.with_cell(text: "Notification title") - row.with_cell(text: "Last updated") - row.with_cell(text: "Status") - row.with_cell(text: "Update notification".html_safe) + row.with_cell(text: "Product name(s)", html_attributes: { scope: "col" }) + row.with_cell(text: "Notification title", html_attributes: { scope: "col" }) + row.with_cell(text: "Last updated", html_attributes: { scope: "col" }) + row.with_cell(text: "Status", html_attributes: { scope: "col" }) + row.with_cell(text: "Actions".html_safe, html_attributes: { scope: "col" }) end end - @submitted_notifications.each do |notification| + @submitted_notifications.each_with_index do |notification, index| table.with_body do |body| body.with_row do |row| - row.with_cell(header: true, text: sanitize(notification.investigation_products.decorate.map(&:product).map(&:name_with_brand).join("
"), tags: %w(br)).html_safe) - row.with_cell(text: sanitize(notification.user_title)) - row.with_cell(text: notification.updated_at.to_formatted_s(:govuk)) - row.with_cell(text: "#{govuk_tag(text: 'Submitted', colour: 'green').html_safe}

#{notification.submitted_at? ?notification.submitted_at.to_formatted_s(:govuk) : "Date not provided"}".html_safe) - row.with_cell(text: sanitize("Update notification", tags: %w(a), attributes: %w(href class)).html_safe) + product_value = notification.investigation_products.decorate.map(&:product).map(&:name_with_brand).join(", ") + title_value = notification.user_title + updated_value = notification.updated_at.to_formatted_s(:govuk) + status_value = "Submitted #{notification.submitted_at? ? notification.submitted_at.to_formatted_s(:govuk) : "Date not provided"}" + + row.with_cell( + header: true, + text: sanitize(notification.investigation_products.decorate.map(&:product).map(&:name_with_brand).join("
"), tags: %w(br)).html_safe + ) + row.with_cell(text: sanitize(title_value)) + row.with_cell(text: updated_value) + row.with_cell(text: "#{govuk_tag(text: 'Submitted', colour: 'green').html_safe}

#{notification.submitted_at? ? notification.submitted_at.to_formatted_s(:govuk) : "Date not provided"}".html_safe) + row.with_cell( + text: sanitize( + " + Product name(s): #{product_value}. + Notification title: #{title_value}. + Last updated: #{updated_value}. + Status: #{status_value}. + + Update notification", + tags: %w(a span), + attributes: %w(href class id aria-describedby) + ).html_safe + ) end end end diff --git a/spec/features/table_accessibility_spec.rb b/spec/features/table_accessibility_spec.rb new file mode 100644 index 0000000000..4aaae7bba0 --- /dev/null +++ b/spec/features/table_accessibility_spec.rb @@ -0,0 +1,43 @@ +require "rails_helper" + +RSpec.describe "Table accessibility" do + let(:user) { create(:user, :activated, has_viewed_introduction: true) } + let(:investigation) { create(:notification, creator: user) } + let(:mail_message) { instance_double(ActionMailer::MessageDelivery, deliver_later: true) } + + before do + allow(NotifyMailer).to receive(:notification_created).and_return(mail_message) + investigation # Create the investigation + sign_in(user) + end + + describe "notifications tables" do + it "has proper accessibility attributes" do + visit "/notifications/your-notifications" + + # Check table class + expect(page).to have_css("table.govuk-table") + + # Check header cells have scope + within "table" do + expect(page).to have_css("th[scope='col']") + end + + # Check hidden descriptions + expect(page).to have_css("caption.govuk-visually-hidden", text: "Notifications data: 5 columns with each notification described across rows within each table body.", visible: :all) + + # Check links have aria-describedby + expect(page).to have_css("a[aria-describedby]") + end + + it "has proper row indices" do + visit "/notifications/team-notifications" + + within "tbody" do + expect(page).to have_css("tr[aria-rowindex='1']") + expect(page).to have_css("tr[aria-rowindex='2']") + expect(page).to have_css("tr[aria-rowindex='3']") + end + end + end +end diff --git a/spec/helpers/investigations_helper_spec.rb b/spec/helpers/investigations_helper_spec.rb new file mode 100644 index 0000000000..806a3f4fc8 --- /dev/null +++ b/spec/helpers/investigations_helper_spec.rb @@ -0,0 +1,31 @@ +require "rails_helper" + +RSpec.describe InvestigationsHelper do + describe "#calculate_row_index" do + context "with different investigation counters and row numbers" do + it "calculates row index for first row" do + expect(helper.calculate_row_index(1, 1)).to eq(4) + end + + it "calculates row index for second row" do + expect(helper.calculate_row_index(1, 2)).to eq(5) + end + + it "calculates row index for third row" do + expect(helper.calculate_row_index(1, 3)).to eq(6) + end + + it "calculates row index for first row of second investigation" do + expect(helper.calculate_row_index(2, 1)).to eq(7) + end + + it "calculates row index for second row of second investigation" do + expect(helper.calculate_row_index(2, 2)).to eq(8) + end + + it "calculates row index for third row of second investigation" do + expect(helper.calculate_row_index(2, 3)).to eq(9) + end + end + end +end