Toggle switch a11y (#1967)

Co-authored-by: Jon Rohan <rohan@github.com>

.changeset/eight-bears-grow.md

@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+Toggle switch accessibility fixes

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

@@ -1,14 +1,14 @@
 <%= render(Primer::BaseComponent.new(tag: "toggle-switch", **@system_arguments)) do %>
   <span class="ToggleSwitch-statusIcon">
-    <%= render(Primer::Beta::Octicon.new(size: :small, color: :danger, icon: :alert, hidden: "true", data: { target: "toggle-switch.errorIcon" })) %>
+    <%= render(Primer::Beta::Octicon.new(size: :small, color: :danger, icon: :"alert-fill", hidden: "true", data: { target: "toggle-switch.errorIcon" })) %>
     <%= render(Primer::Beta::Spinner.new(size: :small, hidden: "true", data: { target: "toggle-switch.loadingSpinner" })) %>
   </span>
   <%= render(Primer::Beta::Text.new(aria: { hidden: true }, classes: "ToggleSwitch-status")) do %>
     <%= render(Primer::Box.new(classes: "ToggleSwitch-statusOn").with_content("On")) %>
     <%= render(Primer::Box.new(classes: "ToggleSwitch-statusOff").with_content("Off")) %>
   <% end %>
 
-  <%= render(Primer::BaseComponent.new(tag: :button, classes: "ToggleSwitch-track", role: "switch", data: { target: "toggle-switch.switch", action: "click:toggle-switch#toggle" }, aria: { checked: on?, disabled: disabled?, label: "Switch" })) do %>
+  <%= render(Primer::BaseComponent.new(tag: :button, classes: "ToggleSwitch-track", disabled: disabled?, data: { target: "toggle-switch.switch", action: "click:toggle-switch#toggle" }, **@aria_arguments)) do %>
     <%= render(Primer::Box.new(classes: "ToggleSwitch-icons", aria: { hidden: true })) do %>
       <%= render(Primer::Box.new(classes: "ToggleSwitch-lineIcon")) do %>
         <%= render(Primer::BaseComponent.new(

app/components/primer/alpha/toggle_switch.pcss

@@ -70,17 +70,17 @@
   }
 }
 
-.ToggleSwitch-track[aria-checked='true'][aria-disabled='true'] {
+.ToggleSwitch-track[aria-pressed='true'][disabled] {
   background-color: var(--color-switch-track-disabled-bg);
   color: var(--color-switch-track-checked-disabled-fg);
   border-color: transparent;
 }
 
-.ToggleSwitch-track[aria-checked='true'] {
+.ToggleSwitch-track[aria-pressed='true'] {
   background-color: var(--color-switch-track-checked-bg);
   border-color: var(--color-switch-track-checked-border);
 
-  &:not([aria-disabled='true']) {
+  &:not([disabled]) {
     &:hover {
       background-color: var(--color-switch-track-checked-hover-bg);
     }
@@ -105,7 +105,7 @@
   }
 }
 
-.ToggleSwitch-track[aria-disabled='true'] {
+.ToggleSwitch-track[disabled] {
   cursor: not-allowed;
   background-color: var(--color-switch-track-disabled-bg);
   border-color: transparent;

app/components/primer/alpha/toggle_switch.rb

@@ -66,6 +66,13 @@ def initialize(src: nil, csrf_token: nil, checked: false, enabled: true, size: S
           SIZE_MAPPINGS[@size]
         )
 
+        @aria_arguments = {
+          aria: merge_aria(
+            @system_arguments,
+            aria: { pressed: on? }
+          )
+        }
+
         @system_arguments[:src] = @src if @src
 
         return unless @src && @csrf_token

app/components/primer/alpha/toggle_switch.ts

@@ -31,25 +31,43 @@ class ToggleSwitchElement extends HTMLElement {
     return this.src != null
   }
 
-  toggle() {
+  @debounce(300)
+  async toggle() {
     if (this.isDisabled()) {
       return
     }
 
-    if (this.isRemote()) {
-      this.setLoadingState()
-      this.submitForm()
-    } else {
+    if (!this.isRemote()) {
       this.performToggle()
+      return
     }
+
+    // toggle immediately to tell screen readers the switch was clicked
+    this.performToggle()
+    this.setLoadingState()
+
+    try {
+      await this.submitForm()
+    } catch (error) {
+      if (error instanceof Error) {
+        // because we toggle immediately when the switch is clicked, toggle back to the
+        // old state on failure
+        this.setErrorState(error.message || 'An error occurred, please try again.')
+        this.performToggle()
+      }
+
+      return
+    }
+
+    this.setSuccessState()
   }
 
   turnOn(): void {
     if (this.isDisabled()) {
       return
     }
 
-    this.switch.setAttribute('aria-checked', 'true')
+    this.switch.setAttribute('aria-pressed', 'true')
     this.classList.add('ToggleSwitch--checked')
   }
 
@@ -58,28 +76,28 @@ class ToggleSwitchElement extends HTMLElement {
       return
     }
 
-    this.switch.setAttribute('aria-checked', 'false')
+    this.switch.setAttribute('aria-pressed', 'false')
     this.classList.remove('ToggleSwitch--checked')
   }
 
   isOn(): boolean {
-    return this.switch.getAttribute('aria-checked') === 'true'
+    return this.switch.getAttribute('aria-pressed') === 'true'
   }
 
   isOff(): boolean {
     return !this.isOn()
   }
 
   isDisabled(): boolean {
-    return this.switch.getAttribute('aria-disabled') === 'true'
+    return this.switch.getAttribute('disabled') != null
   }
 
   disable(): void {
-    this.switch.setAttribute('aria-disabled', 'true')
+    this.switch.setAttribute('disabled', 'disabled')
   }
 
   enable(): void {
-    this.switch.setAttribute('aria-disabled', 'false')
+    this.switch.removeAttribute('disabled')
   }
 
   private performToggle(): void {
@@ -91,9 +109,11 @@ class ToggleSwitchElement extends HTMLElement {
   }
 
   private setLoadingState(): void {
-    this.disable()
     this.errorIcon.setAttribute('hidden', 'hidden')
     this.loadingSpinner.removeAttribute('hidden')
+
+    const event = new CustomEvent('toggleSwitchLoading', {bubbles: true})
+    this.dispatchEvent(event)
   }
 
   private setSuccessState(): void {
@@ -116,47 +136,36 @@ class ToggleSwitchElement extends HTMLElement {
     }
 
     this.loadingSpinner.setAttribute('hidden', 'hidden')
-    this.enable()
   }
 
-  @debounce(300)
   private async submitForm() {
     const body = new FormData()
 
     if (this.csrf) {
       body.append(this.csrfField, this.csrf)
     }
 
-    body.append('value', this.isOn() ? '0' : '1')
+    body.append('value', this.isOn() ? '1' : '0')
 
-    try {
-      if (!this.src) throw new Error('invalid src')
-
-      let response
-
-      try {
-        response = await fetch(this.src, {
-          credentials: 'same-origin',
-          method: 'POST',
-          headers: {
-            'Requested-With': 'XMLHttpRequest'
-          },
-          body
-        })
-      } catch (error) {
-        throw new Error('A network error occurred, please try again.')
-      }
+    if (!this.src) throw new Error('invalid src')
 
-      if (response.ok) {
-        this.setSuccessState()
-        this.performToggle()
-      } else {
-        throw new Error(await response.text())
-      }
+    let response
+
+    try {
+      response = await fetch(this.src, {
+        credentials: 'same-origin',
+        method: 'POST',
+        headers: {
+          'Requested-With': 'XMLHttpRequest'
+        },
+        body
+      })
     } catch (error) {
-      if (error instanceof Error) {
-        this.setErrorState(error.message || 'An error occurred, please try again.')
-      }
+      throw new Error('A network error occurred, please try again.')
+    }
+
+    if (!response.ok) {
+      throw new Error(await response.text())
     }
   }
 }

lib/primer/forms/dsl/input.rb

@@ -47,7 +47,7 @@ class Input
 
         include Primer::ClassNameHelper
 
-        attr_reader :builder, :form, :input_arguments, :label_arguments, :caption, :validation_message, :ids, :form_control
+        attr_reader :builder, :form, :input_arguments, :label_arguments, :caption, :validation_message, :ids, :form_control, :base_id
 
         alias form_control? form_control
 
@@ -107,11 +107,11 @@ def initialize(builder:, form:, **system_arguments)
 
           @input_arguments[:invalid] = "true" if invalid?
 
-          base_id = SecureRandom.uuid
+          @base_id = SecureRandom.uuid
 
           @ids = {}.tap do |id_map|
-            id_map[:validation] = "validation-#{base_id}" if invalid?
-            id_map[:caption] = "caption-#{base_id}" if caption? || caption_template?
+            id_map[:validation] = "validation-#{@base_id}"
+            id_map[:caption] = "caption-#{@base_id}" if caption? || caption_template?
           end
 
           add_input_aria(:required, true) if required?
@@ -264,10 +264,6 @@ def input?
           true
         end
 
-        def need_validation_element?
-          invalid?
-        end
-
         def validation_arguments
           {
             class: "FormControl-inlineValidation",

lib/primer/forms/dsl/text_field_input.rb

@@ -57,10 +57,6 @@ def leading_visual?
           !!@leading_visual
         end
 
-        def need_validation_element?
-          super || auto_check_src.present?
-        end
-
         def validation_arguments
           if auto_check_src.present?
             super.merge(

lib/primer/forms/dsl/toggle_switch_input.rb

@@ -29,6 +29,10 @@ def to_component
         def type
           :toggle_switch
         end
+
+        def validation_arguments
+          super.merge(role: "alert")
+        end
       end
     end
   end

lib/primer/forms/form_control.html.erb

@@ -9,11 +9,9 @@
       <% end %>
     <% end %>
     <%= content %>
-    <% if @input.need_validation_element? %>
-      <%= content_tag(:div, **@input.validation_arguments) do %>
-        <%= render(Primer::Beta::Octicon.new(icon: :"alert-fill", size: :xsmall, aria: { hidden: true })) %>
-        <%= content_tag(:span, @input.validation_messages.first, **@input.validation_message_arguments) %>
-      <% end %>
+    <%= content_tag(:div, **@input.validation_arguments) do %>
+      <%= render(Primer::Beta::Octicon.new(icon: :"alert-fill", size: :xsmall, aria: { hidden: true })) %>
+      <%= content_tag(:span, @input.invalid? ? @input.validation_messages.first : "", **@input.validation_message_arguments) %>
     <% end %>
     <%= render(Caption.new(input: @input)) %>
   <% end %>

lib/primer/forms/primer_base_component_wrapper.html.erb

@@ -0,0 +1,3 @@
+<%= render(Primer::BaseComponent.new(classes: @classes, **@system_arguments)) do %>
+  <%= content %>
+<% end %>

lib/primer/forms/primer_base_component_wrapper.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require "primer/class_name_helper"
+
+module Primer
+  module Forms
+    # Wraps Primer::BaseComponent.
+    class PrimerBaseComponentWrapper < BaseComponent
+      include Primer::ClassNameHelper
+
+      def initialize(**system_arguments)
+        @system_arguments = system_arguments
+
+        # Extract class and classes so they can be passed to Primer::BaseComponent
+        # as classes:. The class: argument is expected by Rails, but Primer expects
+        # classes:, reminiscent of HashWithIndifferentAccess shenanigans.
+        @classes = class_names(
+          system_arguments.delete(:classes),
+          system_arguments.delete(:class)
+        )
+      end
+    end
+  end
+end

lib/primer/forms/toggle_switch.html.erb

@@ -1,6 +1,6 @@
-<%= content_tag("toggle-switch-input", **@input.input_arguments) do %>
+<%= content_tag("toggle-switch-input", class: "FormControl-toggleSwitchInput", hidden: @input.hidden?) do %>
   <span style="flex-grow: 1">
-    <%= builder.label(@input.name, **@input.label_arguments) do %>
+    <%= render(Primer::Forms::PrimerBaseComponentWrapper.new(tag: :span, **@input.label_arguments)) do %>
       <%= @input.label %>
     <% end %>
 
@@ -18,5 +18,5 @@
       }
     )
   %>
-  <%= render(Primer::Alpha::ToggleSwitch.new(src: @input.src, csrf: csrf)) %>
+  <%= render(Primer::Alpha::ToggleSwitch.new(src: @input.src, csrf: csrf, **@input.input_arguments)) %>
 <% end %>

lib/primer/forms/toggle_switch.rb

@@ -9,8 +9,12 @@ class ToggleSwitch < BaseComponent
       def initialize(input:)
         @input = input
         @input.add_label_classes("FormControl-label")
-        @input.add_input_classes("FormControl-toggleSwitchInput")
-        @input.input_arguments[:hidden] = "hidden" if @input.hidden?
+        @input.label_arguments[:id] = label_id
+        @input.add_input_aria(:labelledby, label_id)
+      end
+
+      def label_id
+        @label_id ||= "label-#{@input.base_id}"
       end
     end
   end

lib/primer/forms/toggle_switch_input.ts

@@ -1,3 +1,5 @@
+/* eslint-disable custom-elements/expose-class-on-global */
+
 import {controller, target} from '@github/catalyst'
 
 @controller
@@ -7,12 +9,17 @@ export class ToggleSwitchInputElement extends HTMLElement {
 
   connectedCallback() {
     this.addEventListener('toggleSwitchError', (event: Event) => {
-      this.validationMessageElement.innerText = (event as CustomEvent).detail
+      this.validationMessageElement.textContent = (event as CustomEvent).detail
       this.validationElement.removeAttribute('hidden')
     })
 
     this.addEventListener('toggleSwitchSuccess', () => {
-      this.validationMessageElement.innerText = ''
+      this.validationMessageElement.textContent = ''
+      this.validationElement.setAttribute('hidden', 'hidden')
+    })
+
+    this.addEventListener('toggleSwitchLoading', () => {
+      this.validationMessageElement.textContent = ''
       this.validationElement.setAttribute('hidden', 'hidden')
     })
   }