From d148d5d1207cc2d24b28654473226fcce3f534e0 Mon Sep 17 00:00:00 2001
From: jazairi <16103405+jazairi@users.noreply.github.com>
Date: Wed, 29 May 2024 17:01:09 -0400
Subject: [PATCH] Fix form required attributes redundancy and keyword input
 description

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.
---
 app/javascript/search_form.js         | 10 ----------
 app/views/search/_form.html.erb       | 17 +++++++++--------
 app/views/shared/_site_title.html.erb |  2 +-
 3 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/app/javascript/search_form.js b/app/javascript/search_form.js
index 431e9ef7..acbf6453 100644
--- a/app/javascript/search_form.js
+++ b/app/javascript/search_form.js
@@ -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(
@@ -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';
@@ -27,7 +25,6 @@ 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', '');
@@ -35,7 +32,6 @@ function disableGeobox() {
     field.value = '';
     field.classList.toggle('required');
     field.toggleAttribute('required');
-    field.setAttribute('aria-required', false);
   });
   geobox_label.classList = 'closed';
 };
@@ -44,7 +40,6 @@ 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');
@@ -52,7 +47,6 @@ function enableGeobox() {
     field.value = '';
     field.classList.toggle('required');
     field.toggleAttribute('required');
-    field.setAttribute('aria-required', true);
   });
   geobox_label.classList = 'open';
 };
@@ -61,7 +55,6 @@ 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', '');
@@ -69,7 +62,6 @@ function disableGeodistance() {
     field.value = '';
     field.classList.toggle('required');
     field.toggleAttribute('required');
-    field.setAttribute('aria-required', false);
   });
   geodistance_label.classList = 'closed';
 };
@@ -78,7 +70,6 @@ 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');
@@ -86,7 +77,6 @@ function enableGeodistance() {
     field.value = '';
     field.classList.toggle('required');
     field.toggleAttribute('required');
-    field.setAttribute('aria-required', true);
   });
   geodistance_label.classList = 'open';
 };
diff --git a/app/views/search/_form.html.erb b/app/views/search/_form.html.erb
index dd5260fc..f4ae9ff7 100644
--- a/app/views/search/_form.html.erb
+++ b/app/views/search/_form.html.erb
@@ -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" %>>
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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')
diff --git a/app/views/shared/_site_title.html.erb b/app/views/shared/_site_title.html.erb
index 750f130d..65fb825e 100644
--- a/app/views/shared/_site_title.html.erb
+++ b/app/views/shared/_site_title.html.erb
@@ -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 %>