Skip to content

Commit

Permalink
Refactor markup to provide more semantic meaning
Browse files Browse the repository at this point in the history
Why these changes are being introduced:

It was suggested in a recent meeting with DAS that the best
way to make a page navigable to screen reader users is by
using semantic HTML and ARIA roles.

Relevant ticket(s):

* [GDT-279](https://mitlibraries.atlassian.net/browse/GDT-279)

How this addresses that need:

This attempts to make better use of semantic HTML elements and ARIA
roles throughout our views.

One notable change is the addition of the `application` layout from
our theme. The layout has been added in a separate commit to clarify
what changes were made.

Customization of this layout is needed because the it adds a `main`
tag that surrounds all content between the nav and footer. While this
corresponds to the main content for some apps, it is less useful in
TIMDEX UI.

Side effects of this change:

* This should be considered a first iteration, as we have not
undergone a11y review yet. I tried to use a light touch due to my
lack of familiarity with ARIA, but we may learn that additional
context would be useful.
* We may decide in the future to make the changes to the `application`
layout upstream, in the theme gem. This would require making corresponding
changes in all of our Rails apps, so it is out of scope of this ticket.
* The full record sidebar is an `aside` in non-GDT apps, but in GDT it
includes the access link. To accommodate this, we've given it a
`role=region` with an `aria-label` mentioning the access link.
Long-term, however, we should consider moving this button outside of
the sidebar, so it has more prominence in the markup.
  • Loading branch information
jazairi committed Apr 26, 2024
1 parent f51876f commit 90b30b9
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 26 deletions.
4 changes: 2 additions & 2 deletions app/views/basic_search/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<%= content_for(:title, index_page_title) %>

<div class="space-wrap">
<main class="space-wrap">
<%= render partial: "shared/site_title" %>
<%= render partial: "search/form" %>
<%= render partial: "static/about" if ENV.fetch('ABOUT_APP', nil) %>
</div>
</main>
9 changes: 3 additions & 6 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,11 @@

<div class="wrap-outer-content layout-band">
<div class="wrap-content">
<main id="content-main" class="content-main" role="main">
<%= render partial: "layouts/flash" %>
<%= render partial: "layouts/flash" %>
<%= yield %>
<%= yield %>
<%= render partial: "layouts/site_footer" %>
</main>
<!-- close content-main -->
<%= render partial: "layouts/site_footer" %>
</div>
</div>

Expand Down
4 changes: 2 additions & 2 deletions app/views/record/_record.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div id="full-record" class="gridband layout-3q1q wrap-full-record">
<div class="col3q box-content region full-record" data-region="Full record">
<main class="col3q box-content region full-record" data-region="Full record">
<h2 class="record-title">
<span class="sr">Title: </span>
<% if @record['title'].present? %>
Expand Down Expand Up @@ -177,7 +177,7 @@
<%= render('sidebar') %>
</div>
</main>
<% if params[:debug].present? %>
<%= debug(@record) %>
Expand Down
6 changes: 4 additions & 2 deletions app/views/record/_record_empty.html.erb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
<h1>Record not found</h1>
<main id="full-record" class="box-content region full-record">
<h2>Record not found</h2>

<p>No record was returned by our systems. </p>
<p>No record was returned by our systems. </p>
</main>
4 changes: 2 additions & 2 deletions app/views/record/_record_geo.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div id="full-record" class="gridband layout-3q1q wrap-full-record">
<div class="col3q box-content region full-record" data-region="Full record">
<main class="col3q box-content region full-record" data-region="Full record">
<h2 class="record-title">
<span class="sr">Title: </span>
<% if @record['title'].present? %>
Expand Down Expand Up @@ -48,7 +48,7 @@
<% end %>
<%= render partial: 'access_button', locals: { display: 'view-md' } %>
</div>
</div>
</main>

<%= render('sidebar') %>

Expand Down
12 changes: 10 additions & 2 deletions app/views/record/_sidebar.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
<div class="col1q-r sidebar">
<% if Flipflop.enabled?(:gdt) %>
<div role="region" aria-label="Access link and additional information" class="col1q-r sidebar">
<% else %>
<aside class="col1q-r sidebar">
<% end %>
<% if doi(@record) %>
<% data_url = "/doi?doi=#{URI.encode_www_form_component(doi(@record))}&slim=true" %>

Expand All @@ -13,4 +17,8 @@
<% end %>
<%= render partial: 'shared/ask', locals: { display: '' } %>
</div>
<% if Flipflop.enabled?(:gdt) %>
</div>
<% else %>
</aside>
<% end %>
2 changes: 1 addition & 1 deletion app/views/search/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ if params[:geodistance] == "true"
end
%>

<form id="basic-search" class="form-horizontal basic-search" action="<%= results_path %>" method="get">
<form id="basic-search" class="form-horizontal basic-search" action="<%= results_path %>" method="get" role="search">
<div class="form-group">
<input id="basic-search-main" type="search"
class="field field-text basic-search-input <%= "required" if search_required %>" name="q" title="<%= label %>"
Expand Down
6 changes: 3 additions & 3 deletions app/views/search/_search_summary.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<% return unless applied_filters(@enhanced_query).any? %>

<div class="filter-summary">
<aside class="filter-summary">
<div class="list-filter-summary">
<h2 class="hd-filter-summary hd-5">Applied filters: </h2>
<ul class="list-inline">
Expand All @@ -24,6 +24,6 @@
<div class="clear-filters">
<a class="btn button-primary"
href="<%= results_path(remove_all_filters(@enhanced_query)) %>">Clear all filters</a>
</div>
</div>
<% end %>
</div>
</aside>
4 changes: 2 additions & 2 deletions app/views/search/results.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
</aside>
<% end %>

<div id="results" class="col3q wrap-results">
<main id="results" class="col3q wrap-results">
<% if @results.present? %>
<h2 class="hd-3 results-context"><%= results_summary(@pagination[:hits]) %> returned</h2>
<ol class="results-list" start="<%= @pagination[:start] %>">
Expand All @@ -51,7 +51,7 @@
<p class="hd-2">No results found for your search</p>
</div>
<% end %>
</div>
</main>
<%= render partial: 'shared/ask', locals: { display: 'aside' } if @results.blank? %>
<% if @results.present? %>
Expand Down
4 changes: 2 additions & 2 deletions app/views/shared/_ask.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<% if display == 'view-aside' %>
<div class="col1qr-sidebar">
<aside class="col1qr-sidebar">
<% end %>
<div class="bit ask-us <%= display %>">
<h3 class="title">Need help?</h3>
Expand All @@ -10,5 +10,5 @@
<% end %>
</div>
<% if display == 'view-aside' %>
</div>
</aside>
<% end %>
2 changes: 1 addition & 1 deletion app/views/shared/_error.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<% return unless error['message'].present? %>

<div class="alert alert-banner error">
<div class="alert alert-banner error" role="alert">
<%= error['message'] %><br>
<%= error&.dig('extensions')&.dig('problems')&.pluck("explanation")&.join %>
</div>
2 changes: 1 addition & 1 deletion test/controllers/record_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class RecordControllerTest < ActionDispatch::IntegrationTest
match_requests_on: %i[method uri body]) do
get "/record/#{needle_id}"
message = 'Record not found'
assert_select '#content-main', /(.*)#{message}(.*)/
assert_select 'main', /(.*)#{message}(.*)/
end
end

Expand Down

0 comments on commit 90b30b9

Please sign in to comment.