From b099a3843d0cc40ce2c8fe2a100ffa99f43fd744 Mon Sep 17 00:00:00 2001
From: David Wheatley <hi@davwheat.dev>
Date: Mon, 20 Sep 2021 16:06:15 +0100
Subject: [PATCH] [A11Y] Accessibility improvements for the Search component
 (#3017)

* Remove deprecated code

* Accessibility improvements for Search component
---
 .../core/js/src/forum/components/Search.tsx   | 35 +++++++++++-------
 framework/core/less/common/Search.less        | 11 ++++++
 .../less/common/mixins/accessibility.less     | 36 +++++++++++++++++++
 framework/core/locale/core.yml                |  2 ++
 4 files changed, 71 insertions(+), 13 deletions(-)

diff --git a/framework/core/js/src/forum/components/Search.tsx b/framework/core/js/src/forum/components/Search.tsx
index e6355a956..b4793eab4 100644
--- a/framework/core/js/src/forum/components/Search.tsx
+++ b/framework/core/js/src/forum/components/Search.tsx
@@ -103,14 +103,18 @@ export default class Search<T extends SearchAttrs = SearchAttrs> extends Compone
 
     const searchLabel = extractText(app.translator.trans('core.forum.header.search_placeholder'));
 
+    const isActive = !!currentSearch;
+    const shouldShowResults = !!(!this.loadingSources && this.state.getValue() && this.hasFocus);
+    const shouldShowClearButton = !!(!this.loadingSources && this.state.getValue());
+
     return (
       <div
         role="search"
-        className={classList({
-          Search: true,
+        aria-label={app.translator.trans('core.forum.header.search_role_label')}
+        className={classList('Search', {
           open: this.state.getValue() && this.hasFocus,
           focused: this.hasFocus,
-          active: !!currentSearch,
+          active: isActive,
           loading: !!this.loadingSources,
         })}
       >
@@ -125,18 +129,23 @@ export default class Search<T extends SearchAttrs = SearchAttrs> extends Compone
             onfocus={() => (this.hasFocus = true)}
             onblur={() => (this.hasFocus = false)}
           />
-          {this.loadingSources ? (
-            <LoadingIndicator size="small" display="inline" containerClassName="Button Button--icon Button--link" />
-          ) : currentSearch ? (
-            <button className="Search-clear Button Button--icon Button--link" onclick={this.clear.bind(this)}>
+          {!!this.loadingSources && <LoadingIndicator size="small" display="inline" containerClassName="Button Button--icon Button--link" />}
+          {shouldShowClearButton && (
+            <button
+              className="Search-clear Button Button--icon Button--link"
+              onclick={this.clear.bind(this)}
+              aria-label={app.translator.trans('core.forum.header.search_clear_button_accessible_label')}
+            >
               {icon('fas fa-times-circle')}
             </button>
-          ) : (
-            ''
           )}
         </div>
-        <ul className="Dropdown-menu Search-results">
-          {this.state.getValue() && this.hasFocus ? this.sources.map((source) => source.view(this.state.getValue())) : ''}
+        <ul
+          className="Dropdown-menu Search-results"
+          aria-hidden={!shouldShowResults || undefined}
+          aria-live={shouldShowResults ? 'polite' : undefined}
+        >
+          {shouldShowResults && this.sources.map((source) => source.view(this.state.getValue()))}
         </ul>
       </div>
     );
@@ -174,7 +183,7 @@ export default class Search<T extends SearchAttrs = SearchAttrs> extends Compone
 
     this.$('.Search-results')
       .on('mousedown', (e) => e.preventDefault())
-      .on('click', () => this.$('input').blur())
+      .on('click', () => this.$('input').trigger('blur'))
 
       // Whenever the mouse is hovered over a search result, highlight it.
       .on('mouseenter', '> li:not(.Dropdown-header)', function () {
@@ -223,7 +232,7 @@ export default class Search<T extends SearchAttrs = SearchAttrs> extends Compone
       .on('focus', function () {
         $(this)
           .one('mouseup', (e) => e.preventDefault())
-          .select();
+          .trigger('select');
       });
 
     this.updateMaxHeightHandler = this.updateMaxHeight.bind(this);
diff --git a/framework/core/less/common/Search.less b/framework/core/less/common/Search.less
index bef6114a3..42bf633ca 100644
--- a/framework/core/less/common/Search.less
+++ b/framework/core/less/common/Search.less
@@ -1,5 +1,16 @@
 .Search {
   position: relative;
+
+  &-clear {
+    // It looks very weird due to the padding given to the button..
+    &:focus {
+      outline: none;
+    }
+
+    // ...so we display the ring around the icon inside the button, with an offset
+    .add-keyboard-focus-ring-nearby("> *");
+    .add-keyboard-focus-ring-nearby-offset("> *", 4px);
+  }
 }
 @media @tablet-up {
   .Search {
diff --git a/framework/core/less/common/mixins/accessibility.less b/framework/core/less/common/mixins/accessibility.less
index db261852d..c678f780b 100644
--- a/framework/core/less/common/mixins/accessibility.less
+++ b/framework/core/less/common/mixins/accessibility.less
@@ -130,3 +130,39 @@
     .offset();
   }
 }
+
+/** 
+ * This mixin allows support for a custom element nearby the focused one
+ * to have a focus style applied to it
+ *
+ * For example...
+ * 
+ *? button { .add-keyboard-focus-ring-nearby("+ .myOtherElement") }
+ * becomes
+ *? button:-moz-focusring + .myOtherElement { <styles> }
+ *? button:focus-within + .myOtherElement { <styles> }
+ */
+.add-keyboard-focus-ring-nearby-offset(@nearbySelector, @offset) {
+  @realNearbySelector: ~"@{nearbySelector}";
+
+  .offset() {
+    outline-offset: @offset;
+  }
+
+  // We need to declare these separately, otherwise
+  // browsers will ignore `:focus-visible` as they
+  // don't understand `:-moz-focusring`
+
+  // These are the keyboard-only versions of :focus
+  &:-moz-focusring {
+    @{realNearbySelector} {
+      .offset();
+    }
+  }
+
+  &:focus-visible {
+    @{realNearbySelector} {
+      .offset();
+    }
+  }
+}
diff --git a/framework/core/locale/core.yml b/framework/core/locale/core.yml
index b8276fbba..191cd5364 100644
--- a/framework/core/locale/core.yml
+++ b/framework/core/locale/core.yml
@@ -348,7 +348,9 @@ core:
       log_in_link: => core.ref.log_in
       log_out_button: => core.ref.log_out
       profile_button: Profile
+      search_clear_button_accessible_label: Clear search query
       search_placeholder: Search Forum
+      search_role_label: Search Forum
       session_dropdown_accessible_label: Toggle session options dropdown menu
       settings_button: => core.ref.settings
       sign_up_link: => core.ref.sign_up