When hiding a Spinner, also hide its screen reader text (#3021)

Author: camertron

.changeset/neat-pumpkins-appear.md

@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+When hiding a `Spinner`, also hide its screen reader text

app/components/primer/beta/spinner.html.erb

@@ -1,7 +1,9 @@
-<%= render Primer::BaseComponent.new(**@system_arguments) do %>
-  <circle cx="8" cy="8" r="7" stroke="currentColor" stroke-opacity="0.25" stroke-width="2" vector-effect="non-scaling-stroke" fill="none" />
-  <path d="M15 8a7.002 7.002 0 00-7-7" stroke="currentColor" stroke-width="2" stroke-linecap="round" vector-effect="non-scaling-stroke" />
-<% end %>
-<% if no_aria_label? %>
-  <span class="spinner-screenreader-text sr-only"><%= @sr_text %></span>
+<%= render(Primer::BaseComponent.new(tag: :span, hidden: @hidden, data: { target: @target })) do %>
+  <%= render Primer::BaseComponent.new(**@system_arguments) do %>
+    <circle cx="8" cy="8" r="7" stroke="currentColor" stroke-opacity="0.25" stroke-width="2" vector-effect="non-scaling-stroke" fill="none" />
+    <path d="M15 8a7.002 7.002 0 00-7-7" stroke="currentColor" stroke-width="2" stroke-linecap="round" vector-effect="non-scaling-stroke" />
+  <% end %>
+  <% if no_aria_label? %>
+    <span class="sr-only"><%= @sr_text %></span>
+  <% end %>
 <% end %>

app/components/primer/beta/spinner.rb

@@ -40,6 +40,9 @@ def initialize(size: DEFAULT_SIZE, style: DEFAULT_STYLE, sr_text: DEFAULT_SR_TEX
         else
           @system_arguments[:role] = "img"
         end
+
+        @target = extract_data(:target, @system_arguments)
+        @hidden = @system_arguments.delete(:hidden)
       end
 
       def no_aria_label?

app/lib/primer/attributes_helper.rb

@@ -14,6 +14,16 @@ def data(val, system_arguments)
       system_arguments[:"data-#{val}"] || system_arguments.dig(:data, val.to_sym)
     end
 
+    def extract_data(val, system_arguments)
+      if system_arguments.include?(:"data-#{val}")
+        system_arguments.delete(:"data-#{val}")
+      else
+        if system_arguments.include?(:data) && system_arguments[:data][val.to_sym]
+          system_arguments[:data].delete(val.to_sym)
+        end
+      end
+    end
+
     # Merges hashes that contain "aria-*" keys and nested aria: hashes. Removes keys from
     # each hash and returns them in the new hash.
     #

lib/primer/forms/primer_text_field.ts

@@ -11,7 +11,6 @@ declare global {
   }
 }
 
-const SCREENREADER_TEXT_CLASSNAME = 'spinner-screenreader-text'
 @controller
 export class PrimerTextFieldElement extends HTMLElement {
   @target inputElement: HTMLInputElement
@@ -99,13 +98,11 @@ export class PrimerTextFieldElement extends HTMLElement {
 
   showLeadingSpinner(): void {
     this.leadingSpinner?.removeAttribute('hidden')
-    this.leadingSpinner?.querySelector(SCREENREADER_TEXT_CLASSNAME)?.removeAttribute('hidden')
     this.leadingVisual?.setAttribute('hidden', '')
   }
 
   hideLeadingSpinner(): void {
     this.leadingSpinner?.setAttribute('hidden', '')
-    this.leadingSpinner?.querySelector(SCREENREADER_TEXT_CLASSNAME)?.setAttribute('hidden', '')
     this.leadingVisual?.removeAttribute('hidden')
   }
 }

test/components/alpha/text_field_test.rb

@@ -92,7 +92,7 @@ def test_renders_a_spinner
       Primer::Alpha::TextField.new(**@default_params, leading_visual: { icon: :search }, leading_spinner: true)
     )
 
-    assert_selector "svg[data-target='primer-text-field.leadingSpinner']", visible: :hidden
+    assert_selector "[data-target='primer-text-field.leadingSpinner']", visible: :hidden
   end
 
   def test_enforces_leading_visual_when_spinner_requested

test/system/alpha/text_field_test.rb

@@ -80,5 +80,23 @@ def test_show_and_hide_leading_spinner
       assert_selector "[data-target='primer-text-field.leadingVisual']"
       refute_selector "[data-target='primer-text-field.leadingSpinner']"
     end
+
+    def test_shows_and_hides_screenreader_text
+      visit_preview(:playground, leading_spinner: true)
+
+      evaluate_multiline_script(<<~JS)
+        const textField = document.querySelector('primer-text-field')
+        textField.showLeadingSpinner()
+      JS
+
+      assert_selector "[data-target='primer-text-field.leadingSpinner'] .sr-only", text: "Loading"
+
+      evaluate_multiline_script(<<~JS)
+        const textField = document.querySelector('primer-text-field')
+        textField.hideLeadingSpinner()
+      JS
+
+      refute_selector "[data-target='primer-text-field.leadingSpinner'] .sr-only"
+    end
   end
 end