From 463112c2eaca29280499476b2591cb1dad67e9b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 21 Aug 2021 00:50:11 +0200 Subject: [PATCH] Use a switch to toggle investment selection Just like it happened with proposals, the button to select/deselect an investment wasn't very intuitive; for example, it wasn't obvious that pressing a button saying "selected" would deselect the investment. So we're using a switch control, like we do to enable/disable features since commit fabe97e50. Note that we're making the text of the switch smaller than in other places because the text in the investments table it is also smaller (we're using `font-size: inherit` for that purpose). That made the button look weird because we were using rems instead of ems for the width of the button, so we're adjusting that as well. Also note we're changing the width of the switch to `6em` instead of `6.25em` (which would be 100px if 1em is 16px). We're doing so because we used 100 for the minimum width because it's a round number, so now we're using another round number. --- .../admin/budget_investments/investments.scss | 4 ++ app/assets/stylesheets/mixins/buttons.scss | 2 +- .../toggle_selection_component.html.erb | 2 +- .../toggle_selection_component.rb | 27 ++++----- .../toggle_selection.js.erb | 6 +- config/locales/en/admin.yml | 1 - config/locales/es/admin.yml | 1 - .../toggle_selection_component_spec.rb | 12 ++-- spec/system/admin/budget_investments_spec.rb | 60 ++++++++++++------- 9 files changed, 68 insertions(+), 47 deletions(-) diff --git a/app/assets/stylesheets/admin/budget_investments/investments.scss b/app/assets/stylesheets/admin/budget_investments/investments.scss index 479c69fe9..5d931e6ff 100644 --- a/app/assets/stylesheets/admin/budget_investments/investments.scss +++ b/app/assets/stylesheets/admin/budget_investments/investments.scss @@ -12,4 +12,8 @@ font-size: $small-font-size; } } + + .toggle-switch [aria-pressed] { + font-size: inherit; + } } diff --git a/app/assets/stylesheets/mixins/buttons.scss b/app/assets/stylesheets/mixins/buttons.scss index ee09362ac..72691b9f1 100644 --- a/app/assets/stylesheets/mixins/buttons.scss +++ b/app/assets/stylesheets/mixins/buttons.scss @@ -85,7 +85,7 @@ @include regular-button; border-radius: $line-height; font-weight: bold; - min-width: rem-calc(100); + min-width: 6em; position: relative; &::after { diff --git a/app/components/admin/budget_investments/toggle_selection_component.html.erb b/app/components/admin/budget_investments/toggle_selection_component.html.erb index ffe677cc3..8981bf633 100644 --- a/app/components/admin/budget_investments/toggle_selection_component.html.erb +++ b/app/components/admin/budget_investments/toggle_selection_component.html.erb @@ -1,5 +1,5 @@ <% if can?(:toggle_selection, investment) %> - <%= button_to text, path, method: :patch, remote: true, class: html_class %> + <%= render Admin::ToggleSwitchComponent.new(action, investment, pressed: selected?, **options) %> <% elsif selected? %> <%= selected_text %> <% end %> diff --git a/app/components/admin/budget_investments/toggle_selection_component.rb b/app/components/admin/budget_investments/toggle_selection_component.rb index 64d662fd5..5e5fa43e4 100644 --- a/app/components/admin/budget_investments/toggle_selection_component.rb +++ b/app/components/admin/budget_investments/toggle_selection_component.rb @@ -9,18 +9,14 @@ class Admin::BudgetInvestments::ToggleSelectionComponent < ApplicationComponent private - def text - if selected? - selected_text - else - t("admin.budget_investments.index.select") - end - end - def selected_text t("admin.budget_investments.index.selected") end + def action + :toggle_selection + end + def path toggle_selection_admin_budget_budget_investment_path( investment.budget, @@ -34,11 +30,14 @@ class Admin::BudgetInvestments::ToggleSelectionComponent < ApplicationComponent ) end - def html_class - if selected? - "button small expanded" - else - "button small hollow expanded" - end + def options + { + "aria-label": label, + path: path + } + end + + def label + t("admin.actions.label", action: t("admin.actions.select"), name: investment.title) end end diff --git a/app/views/admin/budget_investments/toggle_selection.js.erb b/app/views/admin/budget_investments/toggle_selection.js.erb index 6a99f9783..98d258bad 100644 --- a/app/views/admin/budget_investments/toggle_selection.js.erb +++ b/app/views/admin/budget_investments/toggle_selection.js.erb @@ -1,3 +1,3 @@ -$("#<%= dom_id(@investment) %> [data-field='selected']").html( - "<%= j render(Admin::BudgetInvestments::ToggleSelectionComponent.new(@investment)) %>" -); +<%= render "admin/shared/toggle_switch", + record: @investment, + new_content: render(Admin::BudgetInvestments::ToggleSelectionComponent.new(@investment)) %> diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 589127371..433d2d10f 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -269,7 +269,6 @@ en: unfeasible: "Unfeasible" undecided: "Undecided" selected: "Selected" - select: "Select" list: id: ID title: Title diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index 510753097..0e5f3bd0e 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -269,7 +269,6 @@ es: unfeasible: "Inviable" undecided: "Sin decidir" selected: "Seleccionado" - select: "Seleccionar" list: id: ID title: Título diff --git a/spec/components/admin/budget_investments/toggle_selection_component_spec.rb b/spec/components/admin/budget_investments/toggle_selection_component_spec.rb index 954bee04b..aff7da09f 100644 --- a/spec/components/admin/budget_investments/toggle_selection_component_spec.rb +++ b/spec/components/admin/budget_investments/toggle_selection_component_spec.rb @@ -20,8 +20,9 @@ describe Admin::BudgetInvestments::ToggleSelectionComponent, :admin do render_inline Admin::BudgetInvestments::ToggleSelectionComponent.new(valuation_finished_investment) - expect(page).to have_button "Select" - expect(page).not_to have_button "Selected" + expect(page).to have_button count: 1 + expect(page).to have_button exact_text: "No" + expect(page).to have_css "button[aria-pressed='false']" end it "renders a button to deselect selected investments" do @@ -29,8 +30,9 @@ describe Admin::BudgetInvestments::ToggleSelectionComponent, :admin do render_inline Admin::BudgetInvestments::ToggleSelectionComponent.new(selected_investment) - expect(page).to have_button "Selected" - expect(page).not_to have_button "Select" + expect(page).to have_button count: 1 + expect(page).to have_button exact_text: "Yes" + expect(page).to have_css "button[aria-pressed='true']" end end @@ -58,7 +60,7 @@ describe Admin::BudgetInvestments::ToggleSelectionComponent, :admin do render_inline Admin::BudgetInvestments::ToggleSelectionComponent.new(selected_investment) expect(page).to have_content "Selected" - expect(page).not_to have_button "Selected" + expect(page).not_to have_button end end end diff --git a/spec/system/admin/budget_investments_spec.rb b/spec/system/admin/budget_investments_spec.rb index 2df4ca0cf..90fda619c 100644 --- a/spec/system/admin/budget_investments_spec.rb +++ b/spec/system/admin/budget_investments_spec.rb @@ -1412,9 +1412,14 @@ describe "Admin budget investments", :admin do expect(page).to have_content "Unfeasible project" - within("#budget_investment_#{feasible_vf_bi.id}") do - click_button "Select" - expect(page).to have_button "Selected" + within("tr", text: "Feasible, VF project") do + within("[data-field=selected]") do + expect(page).to have_content "No" + + click_button "Select Feasible, VF project" + + expect(page).to have_content "Yes" + end end click_link "Advanced filters" @@ -1424,9 +1429,10 @@ describe "Admin budget investments", :admin do expect(page).not_to have_content "Unfeasible project" - within("#budget_investment_#{feasible_vf_bi.id}") do - expect(page).not_to have_button "Select" - expect(page).to have_button "Selected" + within("tr", text: "Feasible, VF project") do + within("[data-field=selected]") do + expect(page).to have_content "Yes" + end end end @@ -1439,21 +1445,26 @@ describe "Admin budget investments", :admin do expect(page).to have_content("There are 2 investments") - within("#budget_investment_#{selected_bi.id}") do - click_button "Selected" + within("tr", text: "Selected project") do + within("[data-field=selected]") do + expect(page).to have_content "Yes" - expect(page).to have_button "Select" + click_button "Select Selected project" + + expect(page).to have_content "No" + end end - click_button("Filter") - expect(page).not_to have_content(selected_bi.title) - expect(page).to have_content("There is 1 investment") + click_button "Filter" + expect(page).not_to have_content "Selected project" + expect(page).to have_content "There is 1 investment" visit admin_budget_budget_investments_path(budget) - within("#budget_investment_#{selected_bi.id}") do - expect(page).to have_button "Select" - expect(page).not_to have_button "Selected" + within("tr", text: "Selected project") do + within("[data-field=selected]") do + expect(page).to have_content "No" + end end end @@ -1465,10 +1476,12 @@ describe "Admin budget investments", :admin do visit admin_budget_budget_investments_path(budget) - within("#budget_investment_#{selected_bi.id}") do - click_button "Selected" + within("tr", text: "Selected project") do + within("[data-field=selected]") do + click_button "Select Selected project" - expect(page).to have_button "Select" + expect(page).to have_content "No" + end end click_link("Next") @@ -1806,11 +1819,16 @@ describe "Admin budget investments", :admin do within("#js-columns-selector-wrapper") { uncheck "Title" } within("#budget_investment_#{investment.id}") do - click_button "Selected" + within("[data-field=selected]") do + expect(page).to have_content "Yes" - expect(page).to have_button "Select" - expect(page).not_to have_content "Don't display me, please!" + click_button "Select Don't display me, please!" + + expect(page).to have_content "No" + end end + + expect(page).not_to have_content "Don't display me, please!" end scenario "When restoring the page from browser history renders columns selectors only once" do