Skip to content

Commit

Permalink
Fix usages of aria-labelledby
Browse files Browse the repository at this point in the history
Previously the div of attachments used 'aria-labelledby' to
refer to an outer heading element, which is incorrect: the
label should refer to something inside the 'div'. The 'div'
should also set the appropriate 'role="section"' attribute [1],
or we can just replace the 'div' with a 'section' element [2].

This corrects the structure of the attachments partial so
that the sections properly self-describe. Each section now has
an ID, so they continue to be easy to grab hold of in tests.

[1] https://www.w3.org/TR/wai-aria/#region

[2] https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Region_role
  • Loading branch information
Ben Thorner committed Mar 27, 2020
1 parent a097b81 commit bf24c71
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 23 deletions.
13 changes: 5 additions & 8 deletions app/views/content_items/_attachments.html.erb
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
<% attachment_title_id = "#{title.parameterize}-title" %>

<% if legacy_pre_rendered_documents.present? || attachments.any? %>
<%= render 'govuk_publishing_components/components/heading',
id: attachment_title_id,
text: title,
mobile_top_margin: true %>
<section id="<%= title.parameterize %>">
<%= render 'govuk_publishing_components/components/heading',
text: title,
mobile_top_margin: true %>

<div aria-labelledby="<%= attachment_title_id %>">
<% if legacy_pre_rendered_documents.present? %>
<%= render 'govuk_publishing_components/components/govspeak',
content: legacy_pre_rendered_documents.html_safe,
Expand All @@ -19,5 +16,5 @@
</div>
<% end %>
<% end %>
</div>
</section>
<% end %>
12 changes: 6 additions & 6 deletions app/views/content_items/publication.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,16 @@
legacy_pre_rendered_documents: @content_item.documents,
attachments: @content_item.featured_attachments %>

<%= render 'govuk_publishing_components/components/heading',
text: t("publication.details"),
id: "details-title",
mobile_top_margin: true %>

<div aria-labelledby="details-title">
<section id="details">
<%= render 'govuk_publishing_components/components/heading',
text: t("publication.details"),
mobile_top_margin: true %>

<%= render 'govuk_publishing_components/components/govspeak',
content: @content_item.details.html_safe,
direction: page_text_direction %>
</div>
</section>
</div>

<%= render 'components/published-dates', {
Expand Down
12 changes: 6 additions & 6 deletions test/integration/consultation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ class ConsultationTest < ActionDispatch::IntegrationTest
setup_and_visit_content_item("closed_consultation")

assert page.has_text?("Documents")
within '[aria-labelledby="documents-title"]' do
within "#documents" do
assert page.has_text?("Museums Review Terms of Reference")
end

setup_and_visit_content_item("consultation_outcome_with_featured_attachments")

assert page.has_text?("Documents")
within '[aria-labelledby="documents-title"]' do
within "#documents" do
assert page.has_text?("Setting the grade standards of new GCSEs in England – part 2")
end
end
Expand Down Expand Up @@ -99,14 +99,14 @@ class ConsultationTest < ActionDispatch::IntegrationTest
setup_and_visit_content_item("consultation_outcome")

assert page.has_text?("Download the full outcome")
within '[aria-labelledby="download-the-full-outcome-title"]' do
within "#download-the-full-outcome" do
assert page.has_text?("Employee Share Schemes: NIC elections - consulation response")
end

setup_and_visit_content_item("consultation_outcome_with_featured_attachments")

assert page.has_text?("Download the full outcome")
within '[aria-labelledby="download-the-full-outcome-title"]' do
within "#download-the-full-outcome" do
assert page.has_text?("Equalities impact assessment: setting the grade standards of new GCSEs in England – part 2")
end
end
Expand All @@ -115,14 +115,14 @@ class ConsultationTest < ActionDispatch::IntegrationTest
setup_and_visit_content_item("consultation_outcome_with_feedback")

assert page.has_text?("Feedback received")
within '[aria-labelledby="feedback-received-title"]' do
within "#feedback-received" do
assert page.has_text?("Analysis of responses to our consultation on setting the grade standards of new GCSEs in England – part 2")
end

setup_and_visit_content_item("consultation_outcome_with_featured_attachments")

assert page.has_text?("Feedback received")
within '[aria-labelledby="feedback-received-title"]' do
within "#feedback-received" do
assert page.has_text?("Analysis of responses to our consultation on setting the grade standards of new GCSEs in England – part 2")
end
end
Expand Down
6 changes: 3 additions & 3 deletions test/integration/publication_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class PublicationTest < ActionDispatch::IntegrationTest
assert_has_component_title(@content_item["title"])
assert page.has_text?(@content_item["description"])

within '[aria-labelledby="details-title"]' do
within "#details" do
assert page.has_text?("Installation name: Leeming Biogas Facility")
end
end
Expand All @@ -36,12 +36,12 @@ class PublicationTest < ActionDispatch::IntegrationTest

test "renders document attachments (as-is and directly)" do
setup_and_visit_content_item("publication")
within '[aria-labelledby="documents-title"]' do
within "#documents" do
assert page.has_text?("Permit: Veolia ES (UK) Limited")
end

setup_and_visit_content_item("publication-with-featured-attachments")
within '[aria-labelledby="documents-title"]' do
within "#documents" do
assert page.has_text?("Number of ex-regular service personnel now part of FR20")
assert page.has_css?(".gem-c-attachment")
end
Expand Down

0 comments on commit bf24c71

Please sign in to comment.