Use a switch control to enable/disable features

We were using buttons with the "Enable" and "Disable" texts to
enable/disable settings. However, when machine learning settings were
introduced in commit 4d27bbeba, a switch control was introduced to
enable/disable them.

In order to keep the interface consistent, we're now using switch
controls in other sections where settings are enabled/disabled. We can
even use the same code in the machine learning settings as well.

We're also removing the confirmation dialog to enable/disable a setting,
since the dialog is really annoying when changing several settings and
this action can be undone immediately. The only setting which might need
a confirmation is the "Skip user verification" one; we might add it in
the future. Removing the confirmation here doesn't make things worse,
though; the "Are you sure?" confirmation dialog was also pretty useless
and users would most likely blindly accept it.

Note Capybara doesn't support finding a button by its `aria-labelledby`
atrribute. Ideally we'd write `click_button "Participatory budgeting"`
instead of `click_button "Yes"`, since from the user's point of view the
"Yes" or "No" texts aren't button labels but indicators of the status of
the setting. This makes the code a little brittle since tests would pass
even if the element referenced by `aria-labelledby` didn't exist.

app/assets/stylesheets/admin/machine_learning/setting.scss

@@ -9,46 +9,6 @@
     }
   }
 
-  [aria-pressed] {
-    @include regular-button;
-    border-radius: $line-height;
-    font-weight: bold;
-    min-width: rem-calc(100);
-    position: relative;
-
-    &::after {
-      background: $white;
-      border-radius: 100%;
-      content: "";
-      display: block;
-      height: 1.75em;
-      position: absolute;
-      transform: translateY(-50%);
-      top: 50%;
-      width: 1.75em;
-    }
-
-    &[aria-pressed=true] {
-      background: $primary-color;
-      padding-right: 2.5em;
-      text-align: left;
-
-      &::after {
-        right: 0.5em;
-      }
-    }
-
-    &[aria-pressed=false] {
-      background: $dark-gray;
-      padding-left: 2.5em;
-      text-align: right;
-
-      &::after {
-        left: 0.5em;
-      }
-    }
-  }
-
   dl {
     @include callout-size(map-get($callout-sizes, small));
   }

app/assets/stylesheets/admin/settings/featured_settings_form.scss

@@ -0,0 +1,6 @@
+.admin .featured-settings-form {
+
+  [aria-pressed] {
+    @include switch;
+  }
+}

app/assets/stylesheets/admin/settings/featured_settings_table.scss

@@ -0,0 +1,6 @@
+.admin .featured-settings-table {
+
+  td {
+    max-width: $global-width / 3;
+  }
+}

app/assets/stylesheets/mixins/buttons.scss

@@ -40,3 +40,43 @@
     text-decoration: underline;
   }
 }
+
+@mixin switch {
+  @include regular-button;
+  border-radius: $line-height;
+  font-weight: bold;
+  min-width: rem-calc(100);
+  position: relative;
+
+  &::after {
+    background: $white;
+    border-radius: 100%;
+    content: "";
+    display: block;
+    height: 1.75em;
+    position: absolute;
+    transform: translateY(-50%);
+    top: 50%;
+    width: 1.75em;
+  }
+
+  &[aria-pressed=true] {
+    background: $primary-color;
+    padding-right: 2.5em;
+    text-align: left;
+
+    &::after {
+      right: 0.5em;
+    }
+  }
+
+  &[aria-pressed=false] {
+    background: $dark-gray;
+    padding-left: 2.5em;
+    text-align: right;
+
+    &::after {
+      left: 0.5em;
+    }
+  }
+}

app/components/admin/machine_learning/setting_component.html.erb

@@ -1,19 +1,12 @@
 <div class="card machine-learning-setting" id="<%= dom_id(setting) %>">
   <div class="card-divider">
-    <h3 id="machine_learning_<%= kind %>"><%= t("admin.machine_learning.#{kind}") %></h3>
+    <h3 id="<%= dom_id(setting, :title) %>"><%= t("admin.machine_learning.#{kind}") %></h3>
   </div>
   <div class="card-section">
-    <p id="machine_learning_<%= kind %>_description"><%= t("admin.machine_learning.#{kind}_description") %></p>
+    <p id="<%= dom_id(setting, :description) %>"><%= t("admin.machine_learning.#{kind}_description") %></p>
 
     <% if ml_info.present? %>
-      <%= form_for(setting, url: admin_setting_path(setting), method: :put) do |f| %>
+      <%= render Admin::Settings::FeaturedSettingsFormComponent.new(setting, tab: "#settings") %>
-        <%= f.hidden_field :tab, value: "#settings", id: "setting_tab_#{kind}" %>
-        <%= f.hidden_field :value, value: (setting.enabled? ? "" : "active"), id: "setting_value_#{kind}" %>
-        <%= f.button(t("shared.#{setting.enabled? ? "yes" : "no"}"),
-                     "aria-labelledby": "machine_learning_#{kind}",
-                     "aria-describedby": "machine_learning_#{kind}_description",
-                     "aria-pressed": setting.enabled?) %>
-      <% end %>
 
       <dl class="callout success">
         <dt><strong><%= t("admin.machine_learning.last_execution") %></strong></dt>

app/components/admin/settings/featured_settings_form_component.html.erb

@@ -1,7 +1,5 @@
-<%= form_for([:admin, feature]) do |f| %>
+<%= form_for([:admin, feature], html: { class: "featured-settings-form" }) do |f| %>
   <%= f.hidden_field :tab, id: dom_id(feature, :tab), value: tab if tab %>
-  <%= f.hidden_field :value, id: dom_id(feature, :value), value: (feature.enabled? ? "" : "active") %>
+  <%= f.hidden_field :value, id: dom_id(feature, :value), value: (enabled? ? "" : "active") %>
-  <%= f.submit(t("admin.settings.index.features.#{feature.enabled? ? "disable" : "enable"}"),
+  <%= f.button text, options %>
-               class: "button expanded #{feature.enabled? ? "hollow alert" : "success"}",
-               data: { confirm: t("admin.actions.confirm") }) %>
 <% end %>

app/components/admin/settings/featured_settings_form_component.rb

@@ -1,8 +1,27 @@
 class Admin::Settings::FeaturedSettingsFormComponent < ApplicationComponent
   attr_reader :feature, :tab
+  delegate :enabled?, to: :feature
 
   def initialize(feature, tab: nil)
     @feature = feature
     @tab = tab
   end
+
+  private
+
+    def text
+      if enabled?
+        t("shared.yes")
+      else
+        t("shared.no")
+      end
+    end
+
+    def options
+      {
+        "aria-labelledby": dom_id(feature, :title),
+        "aria-describedby": dom_id(feature, :description),
+        "aria-pressed": enabled?
+      }
+    end
 end

app/views/admin/settings/_featured_settings_table.html.erb

@@ -1,37 +1,22 @@
-<table>
+<table class="featured-settings-table">
   <thead>
     <tr>
       <th><%= t("admin.settings.setting") %></th>
-      <th><%= t("admin.settings.setting_status") %></th>
+      <th><%= t("admin.settings.index.features.enabled") %></th>
-      <th><%= t("admin.settings.setting_actions") %></th>
     </tr>
   </thead>
   <tbody>
     <% features.each do |feature| %>
       <tr>
-        <td class="small-8">
+        <td>
-          <strong><%= t("settings.#{feature.key}") %></strong>
+          <strong id="<%= dom_id(feature, :title) %>"><%= t("settings.#{feature.key}") %></strong>
           <br>
-          <span class="small">
+          <span class="small" id="<%= dom_id(feature, :description) %>">
             <%= t("settings.#{feature.key}_description", default: t("admin.settings.no_description")) %>
           </span>
         </td>
 
         <td>
-          <% if feature.enabled? %>
-            <span class="enabled">
-              <strong>
-                <%= t("admin.settings.index.features.enabled") %>
-              </strong>
-            </span>
-          <% else %>
-            <span class="disabled">
-              <%= t("admin.settings.index.features.disabled") %>
-            </span>
-          <% end %>
-        </td>
-
-        <td class="text-right">
           <%= render Admin::Settings::FeaturedSettingsFormComponent.new(feature, tab: tab) %>
         </td>
       </tr>

config/locales/en/admin.yml

@@ -1298,8 +1298,7 @@ en:
         images_and_documents: "Images and documents"
         feature_flags: Features
         features:
-          enabled: "Feature enabled"
+          enabled: "Enabled"
-          disabled: "Feature disabled"
           enable: "Enable"
           disable: "Disable"
         map:
@@ -1322,9 +1321,7 @@ en:
       remote_census_request_name: Request Data
       remote_census_response_name: Response Data
       setting: Feature
-      setting_actions: Actions
       setting_name: Setting
-      setting_status: Status
       setting_value: Value
       no_description: "No description"
     shared:

config/locales/es/admin.yml

@@ -1297,8 +1297,7 @@ es:
         images_and_documents: "Imágenes y documentos"
         feature_flags: Funcionalidades
         features:
-          enabled: "Funcionalidad activada"
+          enabled: "Activada"
-          disabled: "Funcionalidad desactivada"
           enable: "Activar"
           disable: "Desactivar"
         map:
@@ -1321,9 +1320,7 @@ es:
       remote_census_request_name: Datos Petición
       remote_census_response_name: Datos Respuesta
       setting: Funcionalidad
-      setting_actions: Acciones
       setting_name: Configuración
-      setting_status: Estado
       setting_value: Valor
       no_description: "Sin descripción"
     shared:

spec/components/admin/settings/featured_settings_form_component_spec.rb

@@ -0,0 +1,55 @@
+require "rails_helper"
+
+describe Admin::Settings::FeaturedSettingsFormComponent do
+  let(:setting) { create(:setting, key: "feature.goodness") }
+  let(:component) { Admin::Settings::FeaturedSettingsFormComponent.new(setting) }
+
+  it "includes an aria-labelledby attribute" do
+    render_inline component
+
+    expect(page).to have_button count: 1
+    expect(page).to have_css "button[aria-labelledby='title_setting_#{setting.id}']"
+  end
+
+  it "includes an aria-describedby attribute" do
+    render_inline component
+
+    expect(page).to have_css "button[aria-describedby='description_setting_#{setting.id}']"
+  end
+
+  describe "aria-pressed attribute" do
+    it "is true when the setting is enabled" do
+      setting.update!(value: "active")
+
+      render_inline component
+
+      expect(page).to have_css "button[aria-pressed='true']"
+    end
+
+    it "is false when the setting is disabled" do
+      setting.update!(value: "")
+
+      render_inline component
+
+      expect(page).to have_css "button[aria-pressed='false']"
+    end
+  end
+
+  describe "button text" do
+    it "reflects the status when the setting is enabled" do
+      setting.update!(value: "active")
+
+      render_inline component
+
+      expect(page).to have_button "Yes"
+    end
+
+    it "reflects the status when the setting is disabled" do
+      setting.update!(value: "")
+
+      render_inline component
+
+      expect(page).to have_button "No"
+    end
+  end
+end

spec/system/admin/feature_flags_spec.rb

@@ -15,18 +15,12 @@ describe "Admin feature flags", :admin do
   end
 
   scenario "Disable a participatory process", :show_exceptions do
-    setting = Setting.find_by(key: "process.budgets")
     budget = create(:budget)
 
     visit admin_settings_path
     within("#settings-tabs") { click_link "Participation processes" }
 
-    within("#edit_setting_#{setting.id}") do
+    within("tr", text: "Participatory budgeting") { click_button "Yes" }
-      expect(page).to have_button "Disable"
-      expect(page).not_to have_button "Enable"
-
-      accept_confirm { click_button "Disable" }
-    end
 
     expect(page).to have_content "Value updated"
 
@@ -46,7 +40,6 @@ describe "Admin feature flags", :admin do
 
   scenario "Enable a disabled participatory process" do
     Setting["process.budgets"] = nil
-    setting = Setting.find_by(key: "process.budgets")
 
     visit admin_root_path
 
@@ -56,13 +49,7 @@ describe "Admin feature flags", :admin do
 
     visit admin_settings_path
     within("#settings-tabs") { click_link "Participation processes" }
-
+    within("tr", text: "Participatory budgeting") { click_button "No" }
-    within("#edit_setting_#{setting.id}") do
-      expect(page).to have_button "Enable"
-      expect(page).not_to have_button "Disable"
-
-      accept_confirm { click_button "Enable" }
-    end
 
     expect(page).to have_content "Value updated"
 
@@ -72,44 +59,30 @@ describe "Admin feature flags", :admin do
   end
 
   scenario "Disable a feature" do
-    setting = Setting.find_by(key: "feature.twitter_login")
-
     visit admin_settings_path
     click_link "Features"
 
-    within("#edit_setting_#{setting.id}") do
+    within("tr", text: "Twitter login") { click_button "Yes" }
-      expect(page).to have_button "Disable"
-      expect(page).not_to have_button "Enable"
-
-      accept_confirm { click_button "Disable" }
-    end
 
     expect(page).to have_content "Value updated"
 
-    within("#edit_setting_#{setting.id}") do
+    within("tr", text: "Twitter login") do
-      expect(page).to have_button "Enable"
+      expect(page).to have_button "No"
-      expect(page).not_to have_button "Disable"
+      expect(page).not_to have_button "Yes"
     end
   end
 
   scenario "Enable a disabled feature" do
-    setting = Setting.find_by(key: "feature.map")
-
     visit admin_settings_path
     click_link "Features"
 
-    within("#edit_setting_#{setting.id}") do
+    within("tr", text: "Proposals and budget investments geolocation") { click_button "No" }
-      expect(page).to have_button "Enable"
-      expect(page).not_to have_button "Disable"
-
-      accept_confirm { click_button "Enable" }
-    end
 
     expect(page).to have_content "Value updated"
 
-    within("#edit_setting_#{setting.id}") do
+    within("tr", text: "Proposals and budget investments geolocation") do
-      expect(page).to have_button "Disable"
+      expect(page).to have_button "Yes"
-      expect(page).not_to have_button "Enable"
+      expect(page).not_to have_button "No"
     end
   end
 end

spec/system/admin/settings_spec.rb

@@ -240,28 +240,22 @@ describe "Admin settings", :admin do
     end
 
     scenario "On #tab-participation-processes" do
-      process_setting = Setting.create!(key: "process.whatever")
+      Setting.create!(key: "process.whatever")
 
       visit admin_settings_path
       find("#participation-processes-tab").click
-
+      within("tr", text: "Whatever") { click_button "No" }
-      accept_alert do
-        find("#edit_setting_#{process_setting.id} .button").click
-      end
 
       expect(page).to have_current_path(admin_settings_path)
       expect(page).to have_css("div#tab-participation-processes.is-active")
     end
 
     scenario "On #tab-feature-flags" do
-      feature_setting = Setting.create!(key: "feature.whatever")
+      Setting.create!(key: "feature.whatever")
 
       visit admin_settings_path
       find("#features-tab").click
-
+      within("tr", text: "Whatever") { click_button "No" }
-      accept_alert do
-        find("#edit_setting_#{feature_setting.id} .button").click
-      end
 
       expect(page).to have_current_path(admin_settings_path)
       expect(page).to have_css("div#tab-feature-flags.is-active")
@@ -274,11 +268,9 @@ describe "Admin settings", :admin do
 
       visit admin_settings_path
       click_link "SDG configuration"
+      within("tr", text: "Whatever") { click_button "No" }
 
-      accept_alert do
+      expect(page).to have_content "Value updated"
-        within("tr", text: "Whatever") { click_button "Enable" }
-      end
-
       expect(page).to have_current_path(admin_settings_path)
       expect(page).to have_css("h2", exact_text: "SDG configuration")
     end
@@ -287,32 +279,22 @@ describe "Admin settings", :admin do
   describe "Skip verification" do
     scenario "deactivate skip verification" do
       Setting["feature.user.skip_verification"] = "true"
-      setting = Setting.find_by(key: "feature.user.skip_verification")
 
       visit admin_settings_path
       find("#features-tab").click
-
+      within("tr", text: "Skip user verification") { click_button "Yes" }
-      accept_alert do
-        find("#edit_setting_#{setting.id} .button").click
-      end
 
       expect(page).to have_content "Value updated"
     end
 
     scenario "activate skip verification" do
       Setting["feature.user.skip_verification"] = nil
-      setting = Setting.find_by(key: "feature.user.skip_verification")
 
       visit admin_settings_path
       find("#features-tab").click
-
+      within("tr", text: "Skip user verification") { click_button "No" }
-      accept_alert do
-        find("#edit_setting_#{setting.id} .button").click
-      end
 
       expect(page).to have_content "Value updated"
-
-      Setting["feature.user.skip_verification"] = nil
     end
   end