consul/grecia
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use a switch control to enable/disable features

Browse Source 
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.
This commit is contained in:
Javi Martín
2021-08-20 21:50:23 +02:00
parent 71aa651f6f
commit fabe97e506
13 changed files with 157 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
app/assets/stylesheets/admin/machine_learning/setting.scss
View File

@@ -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));
}

6
app/assets/stylesheets/admin/settings/featured_settings_form.scss Normal file
View File

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

6
app/assets/stylesheets/admin/settings/featured_settings_table.scss Normal file
View File

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

40
app/assets/stylesheets/mixins/buttons.scss
View File

@@ -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;
}
}
}

13
app/components/admin/machine_learning/setting_component.html.erb
View File

@@ -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| %>
<%= 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 %>
<%= render Admin::Settings::FeaturedSettingsFormComponent.new(setting, tab: "#settings") %>
<dl class="callout success">
<dt><strong><%= t("admin.machine_learning.last_execution") %></strong></dt>

8
app/components/admin/settings/featured_settings_form_component.html.erb
View File

@@ -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.submit(t("admin.settings.index.features.#{feature.enabled? ? "disable" : "enable"}"),
class: "button expanded #{feature.enabled? ? "hollow alert" : "success"}",
data: { confirm: t("admin.actions.confirm") }) %>
<%= f.hidden_field :value, id: dom_id(feature, :value), value: (enabled? ? "" : "active") %>
<%= f.button text, options %>
<% end %>

19
app/components/admin/settings/featured_settings_form_component.rb
View File

@@ -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

25
app/views/admin/settings/_featured_settings_table.html.erb
View File

@@ -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.setting_actions") %></th>
<th><%= t("admin.settings.index.features.enabled") %></th>
</tr>
</thead>
<tbody>
<% features.each do |feature| %>
<tr>
<td class="small-8">
<strong><%= t("settings.#{feature.key}") %></strong>
<td>
<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>

5
config/locales/en/admin.yml
View File

@@ -1298,8 +1298,7 @@ en:
images_and_documents: "Images and documents"
feature_flags: Features
features:
enabled: "Feature enabled"
disabled: "Feature disabled"
enabled: "Enabled"
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:

5
config/locales/es/admin.yml
View File

@@ -1297,8 +1297,7 @@ es:
images_and_documents: "Imágenes y documentos"
feature_flags: Funcionalidades
features:
enabled: "Funcionalidad activada"
disabled: "Funcionalidad desactivada"
enabled: "Activada"
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:

55
spec/components/admin/settings/featured_settings_form_component_spec.rb Normal file
View File

@@ -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

47
spec/system/admin/feature_flags_spec.rb
View File

@@ -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
expect(page).to have_button "Disable"
expect(page).not_to have_button "Enable"
accept_confirm { click_button "Disable" }
end
within("tr", text: "Participatory budgeting") { click_button "Yes" }
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("#edit_setting_#{setting.id}") do
expect(page).to have_button "Enable"
expect(page).not_to have_button "Disable"
accept_confirm { click_button "Enable" }
end
within("tr", text: "Participatory budgeting") { click_button "No" }
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
expect(page).to have_button "Disable"
expect(page).not_to have_button "Enable"
accept_confirm { click_button "Disable" }
end
within("tr", text: "Twitter login") { click_button "Yes" }
expect(page).to have_content "Value updated"
within("#edit_setting_#{setting.id}") do
expect(page).to have_button "Enable"
expect(page).not_to have_button "Disable"
within("tr", text: "Twitter login") do
expect(page).to have_button "No"
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
expect(page).to have_button "Enable"
expect(page).not_to have_button "Disable"
accept_confirm { click_button "Enable" }
end
within("tr", text: "Proposals and budget investments geolocation") { click_button "No" }
expect(page).to have_content "Value updated"
within("#edit_setting_#{setting.id}") do
expect(page).to have_button "Disable"
expect(page).not_to have_button "Enable"
within("tr", text: "Proposals and budget investments geolocation") do
expect(page).to have_button "Yes"
expect(page).not_to have_button "No"
end
end
end

34
spec/system/admin/settings_spec.rb
View File

@@ -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
accept_alert do
find("#edit_setting_#{process_setting.id} .button").click
end
within("tr", text: "Whatever") { click_button "No" }
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
accept_alert do
find("#edit_setting_#{feature_setting.id} .button").click
end
within("tr", text: "Whatever") { click_button "No" }
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
within("tr", text: "Whatever") { click_button "Enable" }
end
expect(page).to have_content "Value updated"
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
accept_alert do
find("#edit_setting_#{setting.id} .button").click
end
within("tr", text: "Skip user verification") { click_button "Yes" }
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
accept_alert do
find("#edit_setting_#{setting.id} .button").click
end
within("tr", text: "Skip user verification") { click_button "No" }
expect(page).to have_content "Value updated"
Setting["feature.user.skip_verification"] = nil
end
end