[SelectPanel] Fix result count in screen reader announcements (#3187)

.changeset/wise-donuts-cheer.md

@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+[SelectPanel] Fix result count in screen reader announcements

app/components/primer/alpha/action_list/item.rb

@@ -302,7 +302,7 @@ def before_render
           if @list.allows_selection?
             @content_arguments[:aria] = merge_aria(
               @content_arguments,
-              { aria: @list.aria_selection_variant == :selected ? {selected: active?} : { checked: active? } }
+              { aria: @list.aria_selection_variant == :selected ? { selected: active? } : { checked: active? } }
             )
           end
 

app/components/primer/alpha/select_panel.html.erb

@@ -47,7 +47,7 @@
       <% end %>
       <%= render Primer::Alpha::Dialog::Body.new(mt: show_filter? ? 0 : 2, p: 0) do %>
         <focus-group direction="vertical" mnemonics retain>
-          <div class="sr-only" aria-live="polite" aria-atomic="true" data-target="select-panel.ariaLiveContainer"></div>
+          <live-region data-target="select-panel.liveRegion"></live-region>
           <%= render(Primer::BaseComponent.new(
             tag: :div,
             data: {

app/components/primer/alpha/select_panel_element.ts

@@ -1,9 +1,10 @@
 import {getAnchoredPosition} from '@primer/behaviors'
 import {controller, target} from '@github/catalyst'
-import {announceFromElement, announce} from '../aria_live'
 import {IncludeFragmentElement} from '@github/include-fragment-element'
 import type {PrimerTextFieldElement} from 'app/lib/primer/forms/primer_text_field'
 import type {AnchorAlignment, AnchorSide} from '@primer/behaviors'
+import type {LiveRegionElement} from '@primer/live-region-element'
+import '@primer/live-region-element'
 import '@oddbird/popover-polyfill'
 
 type SelectVariant = 'none' | 'single' | 'multiple' | null
@@ -69,11 +70,11 @@ export class SelectPanelElement extends HTMLElement {
   @target filterInputTextField: HTMLInputElement
   @target remoteInput: HTMLElement
   @target list: HTMLElement
-  @target ariaLiveContainer: HTMLElement
   @target noResults: HTMLElement
   @target fragmentErrorElement: HTMLElement
   @target bannerErrorElement: HTMLElement
   @target bodySpinner: HTMLElement
+  @target liveRegion: LiveRegionElement
 
   filterFn?: FilterFn
 
@@ -412,7 +413,7 @@ export class SelectPanelElement extends HTMLElement {
     if (this.#loadingAnnouncementTimeoutId) clearTimeout(this.#loadingAnnouncementTimeoutId)
 
     this.#loadingAnnouncementTimeoutId = setTimeout(() => {
-      announce('Loading', {element: this.ariaLiveContainer})
+      this.liveRegion.announce('Loading')
     }, 2000) as unknown as number
 
     this.#loadingDelayTimeoutId = setTimeout(() => {
@@ -564,7 +565,7 @@ export class SelectPanelElement extends HTMLElement {
         const errorElement = this.fragmentErrorElement
         // check if the errorElement is visible in the dom
         if (errorElement && !errorElement.hasAttribute('hidden')) {
-          announceFromElement(errorElement, {element: this.ariaLiveContainer, assertive: true})
+          this.liveRegion.announceFromElement(errorElement, {politeness: 'assertive'})
           return
         }
 
@@ -766,7 +767,7 @@ export class SelectPanelElement extends HTMLElement {
 
     // check if the errorElement is visible in the dom
     if (errorElement && !errorElement.hasAttribute('hidden')) {
-      announceFromElement(errorElement, {element: this.ariaLiveContainer, assertive: true})
+      this.liveRegion.announceFromElement(errorElement, {politeness: 'assertive'})
       return
     }
   }
@@ -778,17 +779,15 @@ export class SelectPanelElement extends HTMLElement {
 
   #maybeAnnounce() {
     if (this.open && this.list) {
-      const items = this.items
+      const items = this.visibleItems
 
       if (items.length > 0) {
         const instructions = 'tab for results'
-        announce(`${items.length} result${items.length === 1 ? '' : 's'} ${instructions}`, {
-          element: this.ariaLiveContainer,
-        })
+        this.liveRegion.announce(`${items.length} result${items.length === 1 ? '' : 's'} ${instructions}`)
       } else {
         const noResultsEl = this.noResults
         if (noResultsEl) {
-          announceFromElement(noResultsEl, {element: this.ariaLiveContainer})
+          this.liveRegion.announceFromElement(noResultsEl)
         }
       }
     }

app/components/primer/primer.ts

@@ -7,7 +7,6 @@ import './anchored_position'
 import './dialog_helper'
 import './focus_group'
 import './scrollable_region'
-import './aria_live'
 import './shared_events'
 import './alpha/modal_dialog'
 import './beta/nav_list'

package.json

@@ -53,6 +53,7 @@
     "@github/relative-time-element": "^4.0.0",
     "@github/remote-input-element": "^0.4.0",
     "@github/tab-container-element": "^3.1.2",
+    "@primer/live-region-element": "^0.7.1",
     "@oddbird/popover-polyfill": "^0.4.0",
     "@primer/behaviors": "^1.3.4"
   },

test/system/alpha/select_panel_test.rb

@@ -100,11 +100,24 @@ def filter_results(query:)
       find("input").fill_in(with: query)
     end
 
-    def assert_announces(message:)
+    def assert_announces(message:, politeness: :polite)
       yield
 
-      assert_selector "[data-target='select-panel.ariaLiveContainer']" do |element|
-        assert_includes element.text, message
+      attempts = 0
+      max_attempts = 3
+
+      begin
+        attempts += 1
+
+        region_text = page.evaluate_script(%Q|
+          document.querySelector("[data-target='select-panel.liveRegion']")?.shadowRoot?.querySelector('##{politeness}')?.textContent ?? ""
+        |)
+
+        assert_includes region_text, message
+      rescue Minitest::Assertion => e
+        raise e if attempts >= max_attempts
+        sleep 1
+        retry
       end
     end
 
@@ -1281,5 +1294,49 @@ def test_remote_fetch_announces_no_results
         end
       end
     end
+
+    def test_announces_correct_result_count
+      visit_preview(:local_fetch)
+
+      assert_announces(message: "4 results") do
+        click_on_invoker_button
+      end
+    end
+
+    def test_announces_correct_result_count_after_filtering
+      visit_preview(:local_fetch)
+
+      click_on_invoker_button
+
+      assert_announces(message: "1 result") do
+        filter_results(query: "2")
+      end
+    end
+
+    def test_announces_error_on_initial_failure
+      visit_preview(:remote_fetch_initial_failure)
+
+      assert_announces(message: "Sorry, something went wrong", politeness: :assertive) do
+        wait_for_items_to_load do
+          click_on_invoker_button
+        end
+      end
+    end
+
+    def test_announces_error_on_filter_failure
+      visit_preview(:remote_fetch_filter_failure)
+
+      wait_for_items_to_load do
+        click_on_invoker_button
+      end
+
+      assert_selector "select-panel ul li"
+
+      assert_announces(message: "Sorry, something went wrong", politeness: :assertive) do
+        wait_for_items_to_load do
+          filter_results(query: "foobar")
+        end
+      end
+    end
   end
 end