Skip to content

Commit

Permalink
Fix form required attributes redundancy and keyword input description
Browse files Browse the repository at this point in the history
Why these changes are being introduced:

DAS has informed us that it is bad practice to use both `required`
and `aria-required`. They also suggested that we add an
`aria-describedby` to the keyword input that links to the `p`
tag below the site title.

Relevant ticket(s):

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

How this addresses that need:

This removes the redundant `aria-required` attributes and makes
the suggested labeling change for the keyword input.

Side effects of this change:

The labeling piece only affects GDT, as non-GDT apps don't currently
use the `site_title` partial. This feels acceptable, because the
keyword input is more clearly labeled in other TIMDEX UI applications.
  • Loading branch information
jazairi committed May 29, 2024
1 parent ab1dfb6 commit b02ad64
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 19 deletions.
10 changes: 0 additions & 10 deletions app/javascript/search_form.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ function disableAdvanced() {
if (geobox_label.classList.contains('closed') && geodistance_label.classList.contains('closed')) {
keyword_field.toggleAttribute('required');
keyword_field.classList.toggle('required');
keyword_field.setAttribute('aria-required', true);
keyword_field.setAttribute('placeholder', 'Enter your search');
}
[...details_panel.getElementsByClassName('field')].forEach(
Expand All @@ -17,7 +16,6 @@ function enableAdvanced() {
if (geobox_label.classList.contains('closed') && geodistance_label.classList.contains('closed')) {
keyword_field.toggleAttribute('required');
keyword_field.classList.toggle('required');
keyword_field.setAttribute('aria-required', false);
keyword_field.setAttribute('placeholder', 'Keyword anywhere');
}
advanced_label.classList = 'open';
Expand All @@ -27,15 +25,13 @@ function disableGeobox() {
if (advanced_label.classList.contains('closed') && geodistance_label.classList.contains('closed')) {
keyword_field.toggleAttribute('required');
keyword_field.classList.toggle('required');
keyword_field.setAttribute('aria-required', true);
keyword_field.setAttribute('placeholder', 'Enter your search');
}
geobox_field.setAttribute('value', '');
[...geobox_details_panel.getElementsByClassName('field')].forEach(function(field) {
field.value = '';
field.classList.toggle('required');
field.toggleAttribute('required');
field.setAttribute('aria-required', false);
});
geobox_label.classList = 'closed';
};
Expand All @@ -44,15 +40,13 @@ function enableGeobox() {
if (advanced_label.classList.contains('closed') && geodistance_label.classList.contains('closed')) {
keyword_field.toggleAttribute('required');
keyword_field.classList.toggle('required');
keyword_field.setAttribute('aria-required', false);
keyword_field.setAttribute('placeholder', 'Keyword anywhere');
}
geobox_field.setAttribute('value', 'true');
[...geobox_details_panel.getElementsByClassName('field')].forEach(function(field) {
field.value = '';
field.classList.toggle('required');
field.toggleAttribute('required');
field.setAttribute('aria-required', true);
});
geobox_label.classList = 'open';
};
Expand All @@ -61,15 +55,13 @@ function disableGeodistance() {
if (advanced_label.classList.contains('closed') && geobox_label.classList.contains('closed')) {
keyword_field.toggleAttribute('required');
keyword_field.classList.toggle('required');
keyword_field.setAttribute('aria-required', true);
keyword_field.setAttribute('placeholder', 'Enter your search');
}
geodistance_field.setAttribute('value', '');
[...geodistance_details_panel.getElementsByClassName('field')].forEach(function(field) {
field.value = '';
field.classList.toggle('required');
field.toggleAttribute('required');
field.setAttribute('aria-required', false);
});
geodistance_label.classList = 'closed';
};
Expand All @@ -78,15 +70,13 @@ function enableGeodistance() {
if (advanced_label.classList.contains('closed') && geobox_label.classList.contains('closed')) {
keyword_field.toggleAttribute('required');
keyword_field.classList.toggle('required');
keyword_field.setAttribute('aria-required', false);
keyword_field.setAttribute('placeholder', 'Keyword anywhere');
}
geodistance_field.setAttribute('value', 'true');
[...geodistance_details_panel.getElementsByClassName('field')].forEach(function(field) {
field.value = '';
field.classList.toggle('required');
field.toggleAttribute('required');
field.setAttribute('aria-required', true);
});
geodistance_label.classList = 'open';
};
Expand Down
17 changes: 9 additions & 8 deletions app/views/search/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ end
<input id="basic-search-main" type="search"
class="field field-text basic-search-input <%= "required" if search_required %>" name="q"
title="Keyword anywhere" placeholder="Enter your search"
value="<%= params[:q] %>" <%= 'required="required" aria-required="true"' if search_required %>>
value="<%= params[:q] %>" <%= 'required' if search_required %>
<%= 'aria-describedby="site-desc"' if Flipflop.enabled?(:gdt) %>>

<% if Flipflop.enabled?(:gdt) %>
<details id="geobox-search-panel" <%= "open" if params[:geobox] == "true" %>>
Expand All @@ -47,7 +48,7 @@ end
<input type="number" step="0.000001" min="-180.0" max="180.0"
class="field field-text <%= "required" if geobox_required %>"
id="geobox-minLongitude" name="geoboxMinLongitude" value="<%= params[:geoboxMinLongitude] %>"
<%= 'required="required" aria-required="true"' if geobox_required %>
<%= 'required' if geobox_required %>
aria-describedby="minLong-desc">
<span class="geo-desc" id="minLong-desc">
A decimal between -180.0 and 180.0 (Ex: -73.507239)
Expand All @@ -59,7 +60,7 @@ end
<input type="number" step="0.000001" min="-90.0" max="90.0"
class="field field-text <%= "required" if geobox_required %>"
id="geobox-minLatitude" name="geoboxMinLatitude" value="<%= params[:geoboxMinLatitude] %>"
<%= 'required="required" aria-required="true"' if geobox_required %>
<%= 'required' if geobox_required %>
aria-describedby="minLat-desc">
<span class="geo-desc" id="minLat-desc">
A decimal between -90.0 and 90.0 (Ex: 41.239083)
Expand All @@ -71,7 +72,7 @@ end
<input type="number" step="0.000001" min="-180.0" max="180.0"
class="field field-text <%= "required" if geobox_required %>"
id="geobox-maxLongitude" name="geoboxMaxLongitude" value="<%= params[:geoboxMaxLongitude] %>"
<%= 'required="required" aria-required="true"' if geobox_required %>
<%= 'required' if geobox_required %>
aria-describedby="maxLong-desc">
<span class="geo-desc" id="maxLong-desc">
A decimal between -180.0 and 180.0 (Ex: -69.928713)
Expand All @@ -83,7 +84,7 @@ end
<input type="number" step="0.000001" min="-90.0" max="90.0"
class="field field-text <%= "required" if geobox_required %>"
id="geobox-maxLatitude" name="geoboxMaxLatitude" value="<%= params[:geoboxMaxLatitude] %>"
<%= 'required="required" aria-required="true"' if geobox_required %>
<%= 'required' if geobox_required %>
aria-describedby="maxLat-desc">
<span class="geo-desc" id="maxLat-desc">
A decimal between -90.0 and 90.0 (Ex: 42.886759)
Expand All @@ -108,7 +109,7 @@ end
class="field field-text <%= "required" if geodistance_required %>"
id="geodistance-latitude" name="geodistanceLatitude"
value="<%= params[:geodistanceLatitude] %>" aria-describedby="lat-desc"
<%= 'required="required" aria-required="true"' if geodistance_required %>
<%= 'required' if geodistance_required %>
aria-describedby="lat-desc">
<span class="geo-desc" id="lat-desc">
A decimal between -90.0 and 90.0 (Ex: 42.279594)
Expand All @@ -121,7 +122,7 @@ end
class="field field-text <%= "required" if geodistance_required %>"
id="geodistance-longitude" name="geodistanceLongitude"
value="<%= params[:geodistanceLongitude] %>" aria-describedby="long-desc"
<%= 'required="required" aria-required="true"' if geodistance_required %>
<%= 'required' if geodistance_required %>
aria-describedby="long-desc">
<span class="geo-desc" id="long-desc">
A decimal between -180.0 and 180.0 (Ex: -83.732124)
Expand All @@ -133,7 +134,7 @@ end
<input type="text" class="field field-text <%= "required" if geodistance_required %>"
id="geodistance-distance" name="geodistanceDistance"
value="<%= params[:geodistanceDistance] %>" aria-describedby="distance-desc"
<%= 'required="required" aria-required="true"' if geodistance_required %>
<%= 'required' if geodistance_required %>
aria-describedby="distance-desc">
<span class="geo-desc" id="distance-desc">
Distance is in meters by default; add other units if preferred (Ex: '100km' or '50mi')
Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/_site_title.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<% if Flipflop.enabled?(:gdt) %>
<h2>Search for geospatial/GIS data</h2>
<p>Find GIS data held at MIT and other institutions</p>
<p id="site-desc">Find GIS data held at MIT and other institutions</p>
<% else %>
<h2>Search the MIT Libraries</h2>
<% end %>

0 comments on commit b02ad64

Please sign in to comment.