Fix disabled ToggleSwitch behavior (#1643)

Co-authored-by: camertron <[email protected]>
primer · Nov 29, 2022 · 7e20e14 · 7e20e14
1 parent 83a1415
commit 7e20e14
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 14 deletions.
diff --git a/.changeset/small-lemons-change.md b/.changeset/small-lemons-change.md
@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+Fix disabled ToggleSwitch behavior
diff --git a/app/components/primer/alpha/toggle_switch.ts b/app/components/primer/alpha/toggle_switch.ts
@@ -32,9 +32,13 @@ class ToggleSwitchElement extends HTMLElement {
   }
 
   toggle() {
+    if (this.isDisabled()) {
+      return
+    }
+
     if (this.isRemote()) {
       this.setLoadingState()
-      this.check()
+      this.submitForm()
     } else {
       this.performToggle()
     }
@@ -110,14 +114,14 @@ class ToggleSwitchElement extends HTMLElement {
   }
 
   @debounce(300)
-  private async check() {
+  private async submitForm() {
     const body = new FormData()
 
     if (this.csrf) {
       body.append(this.csrfField, this.csrf)
     }
 
-    body.append('value', this.isOn() ? '1' : '0')
+    body.append('value', this.isOn() ? '0' : '1')
 
     try {
       if (!this.src) throw new Error('invalid src')

diff --git a/demo/app/controllers/toggle_switch_controller.rb b/demo/app/controllers/toggle_switch_controller.rb
@@ -9,27 +9,37 @@ class << self
 
   skip_before_action :verify_authenticity_token
 
+  before_action :reject_non_ajax_request
+  before_action :verify_artificial_authenticity_token
+
   def create
+    # lol this is so not threadsafe
     self.class.last_request = request
 
     sleep 1 unless Rails.env.test?
 
-    # this mimics dotcom behavior
-    if request.headers["HTTP_REQUESTED_WITH"] != "XMLHttpRequest"
-      head :unprocessable_entity
-      return
-    end
-
-    if form_params[:authenticity_token] && form_params[:authenticity_token] != "let_me_in"
-      head :unauthorized
-      return
-    end
-
     head :accepted
   end
 
   private
 
+  # this mimics dotcom behavior
+  def reject_non_ajax_request
+    return if request.headers["HTTP_REQUESTED_WITH"] == "XMLHttpRequest"
+
+    head :unprocessable_entity
+  end
+
+  def verify_artificial_authenticity_token
+    # don't check token if not provided
+    return unless form_params[:authenticity_token]
+
+    # if provided, check token
+    return if form_params[:authenticity_token] == "let_me_in"
+
+    head :unauthorized
+  end
+
   def form_params
     params.permit(:value, :authenticity_token)
   end

diff --git a/test/system/alpha/toggle_switch_test.rb b/test/system/alpha/toggle_switch_test.rb
@@ -17,6 +17,7 @@ def test_click_disabled
 
       refute_selector(".ToggleSwitch--checked")
       find("toggle-switch").click
+      wait_for_spinner
       refute_selector(".ToggleSwitch--checked")
     end
 
@@ -26,9 +27,31 @@ def test_click_checked_disabled
       assert_selector(".ToggleSwitch--checked")
       assert_selector(".ToggleSwitch--disabled")
       find("toggle-switch").click
+      wait_for_spinner
       assert_selector(".ToggleSwitch--checked")
     end
 
+    def test_submits_correct_value_when_off
+      visit_preview(:default)
+
+      refute_selector(".ToggleSwitch--checked")
+      find("toggle-switch").click
+      assert_selector(".ToggleSwitch--checked")
+
+      assert_equal "1", ToggleSwitchController.last_request.params[:value]
+    end
+
+    def test_submits_correct_value_when_on
+      visit_preview(:checked)
+
+      assert_selector(".ToggleSwitch--checked")
+      find("toggle-switch").click
+      wait_for_spinner
+      refute_selector(".ToggleSwitch--checked")
+
+      assert_equal "0", ToggleSwitchController.last_request.params[:value]
+    end
+
     def test_csrf_token
       visit_preview(:with_csrf_token)
 
@@ -54,5 +77,11 @@ def test_fetch_made_with_correct_headers
 
       assert_equal "XMLHttpRequest", ToggleSwitchController.last_request.headers["HTTP_REQUESTED_WITH"]
     end
+
+    private
+
+    def wait_for_spinner
+      refute_selector("[data-target='toggle-switch.loadingSpinner']")
+    end
   end
 end