From ead5eac67fbedbdb413f5fab9e5a466ad99d567f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 28 Aug 2021 14:00:45 +0200 Subject: [PATCH] Update settings using an AJAX requests Having to wait for a whole page refresh after updating each setting was painful when modifying several settings. Even though the navigation is updated immediately to reflect which sections have been enabled/disabled, there's one gotcha. Changing the "SDG" setting will not update the user menu (which contains a link to SDG content) nor the "SDG Configuration" tab; refreshing the page will be necessary to check these changes. The same happens with the map and remote census tabs. So in these cases we're making an exception and sending the form. We might find a better solution in the future. For this reason, we aren't using the `switch` ARIA role. Some users might not expect a switch control to refresh the page, just like they usually don't expect checkboxes to refresh the page. Furthermore, screen reader support for the `switch` role seems to be inconsistent. For instance, NVDA with Chrome announces the control as a checkbox instead of a switch. Note AJAX is only used for feature settings. Other settings are still updated with regular HTTP requests. Since we're now using AJAX requests, we have to make sure to add an expectation in the homepage tests in order to make sure the request has finished before starting a new one. --- .../featured_settings_form_component.html.erb | 3 +- .../featured_settings_form_component.rb | 5 +++ app/controllers/admin/settings_controller.rb | 6 ++- app/views/admin/settings/update.js.erb | 11 +++++ spec/system/admin/feature_flags_spec.rb | 25 +++++------ spec/system/admin/homepage/homepage_spec.rb | 15 ++++++- spec/system/admin/settings_spec.rb | 41 ++++++++++++++++--- 7 files changed, 84 insertions(+), 22 deletions(-) create mode 100644 app/views/admin/settings/update.js.erb diff --git a/app/components/admin/settings/featured_settings_form_component.html.erb b/app/components/admin/settings/featured_settings_form_component.html.erb index bb5ae5096..e1b8840c9 100644 --- a/app/components/admin/settings/featured_settings_form_component.html.erb +++ b/app/components/admin/settings/featured_settings_form_component.html.erb @@ -1,5 +1,6 @@ -<%= form_for([:admin, feature], html: { class: "featured-settings-form" }) do |f| %> +<%= form_for([:admin, feature], remote: remote?, html: { class: "featured-settings-form" }) do |f| %> <%= f.hidden_field :tab, id: dom_id(feature, :tab), value: tab if tab %> + <%= f.hidden_field :describedby, id: dom_id(feature, :describedby), value: describedby if describedby %> <%= f.hidden_field :value, id: dom_id(feature, :value), value: (enabled? ? "" : "active") %> <%= f.button text, options %> <% end %> diff --git a/app/components/admin/settings/featured_settings_form_component.rb b/app/components/admin/settings/featured_settings_form_component.rb index b785f8716..a4c19154f 100644 --- a/app/components/admin/settings/featured_settings_form_component.rb +++ b/app/components/admin/settings/featured_settings_form_component.rb @@ -21,9 +21,14 @@ class Admin::Settings::FeaturedSettingsFormComponent < ApplicationComponent def options { + data: { disable_with: text }, "aria-labelledby": dom_id(feature, :title), "aria-describedby": (dom_id(feature, :description) if describedby?), "aria-pressed": enabled? } end + + def remote? + !%w[feature.map feature.remote_census feature.sdg].include?(feature.key) + end end diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb index cdc2278ba..e225d6557 100644 --- a/app/controllers/admin/settings_controller.rb +++ b/app/controllers/admin/settings_controller.rb @@ -16,7 +16,11 @@ class Admin::SettingsController < Admin::BaseController def update @setting = Setting.find(params[:id]) @setting.update!(settings_params) - redirect_to request_referer, notice: t("admin.settings.flash.updated") + + respond_to do |format| + format.html { redirect_to request_referer, notice: t("admin.settings.flash.updated") } + format.js + end end def update_map diff --git a/app/views/admin/settings/update.js.erb b/app/views/admin/settings/update.js.erb new file mode 100644 index 000000000..503ef165d --- /dev/null +++ b/app/views/admin/settings/update.js.erb @@ -0,0 +1,11 @@ +var form = $("<%= j render Admin::Settings::FeaturedSettingsFormComponent.new( + @setting, + tab: params[:setting][:tab], + describedby: params[:setting][:describedby] +) %>"); + +$("#" + form.attr("id")).html(form.html()).find("[type='submit']").focus(); + +<% if @setting.type == "feature" || @setting.type == "process" %> + $("#side_menu").html("<%= j render Admin::MenuComponent.new %>").foundation(); +<% end %> diff --git a/spec/system/admin/feature_flags_spec.rb b/spec/system/admin/feature_flags_spec.rb index 28e439e32..397a056b8 100644 --- a/spec/system/admin/feature_flags_spec.rb +++ b/spec/system/admin/feature_flags_spec.rb @@ -20,9 +20,11 @@ describe "Admin feature flags", :admin do visit admin_settings_path within("#settings-tabs") { click_link "Participation processes" } - within("tr", text: "Participatory budgeting") { click_button "Yes" } + within("tr", text: "Participatory budgeting") do + click_button "Yes" - expect(page).to have_content "Value updated" + expect(page).to have_button "No" + end within("#side_menu") do expect(page).not_to have_link "Participatory budgets" @@ -49,9 +51,12 @@ 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" } - expect(page).to have_content "Value updated" + within("tr", text: "Participatory budgeting") do + click_button "No" + + expect(page).to have_button "Yes" + end within("#side_menu") do expect(page).to have_link "Participatory budgets" @@ -62,11 +67,9 @@ describe "Admin feature flags", :admin do visit admin_settings_path click_link "Features" - within("tr", text: "Twitter login") { click_button "Yes" } - - expect(page).to have_content "Value updated" - within("tr", text: "Twitter login") do + click_button "Yes" + expect(page).to have_button "No" expect(page).not_to have_button "Yes" end @@ -76,11 +79,9 @@ describe "Admin feature flags", :admin do visit admin_settings_path click_link "Features" - within("tr", text: "Proposals and budget investments geolocation") { click_button "No" } - - expect(page).to have_content "Value updated" - within("tr", text: "Proposals and budget investments geolocation") do + click_button "No" + expect(page).to have_button "Yes" expect(page).not_to have_button "No" end diff --git a/spec/system/admin/homepage/homepage_spec.rb b/spec/system/admin/homepage/homepage_spec.rb index f7f89a233..314e68247 100644 --- a/spec/system/admin/homepage/homepage_spec.rb +++ b/spec/system/admin/homepage/homepage_spec.rb @@ -34,6 +34,8 @@ describe "Homepage", :admin do within("#widget_feed_#{proposals_feed.id}") do select "1", from: "widget_feed_limit" click_button "No" + + expect(page).to have_button "Yes" end visit root_path @@ -53,6 +55,8 @@ describe "Homepage", :admin do within("#widget_feed_#{debates_feed.id}") do select "2", from: "widget_feed_limit" click_button "No" + + expect(page).to have_button "Yes" end visit root_path @@ -74,11 +78,15 @@ describe "Homepage", :admin do within("#widget_feed_#{proposals_feed.id}") do select "3", from: "widget_feed_limit" click_button "No" + + expect(page).to have_button "Yes" end within("#widget_feed_#{debates_feed.id}") do select "3", from: "widget_feed_limit" click_button "No" + + expect(page).to have_button "Yes" end visit root_path @@ -101,6 +109,8 @@ describe "Homepage", :admin do within("#widget_feed_#{processes_feed.id}") do select "3", from: "widget_feed_limit" click_button "No" + + expect(page).to have_button "Yes" end visit root_path @@ -153,11 +163,12 @@ describe "Homepage", :admin do create(:proposal, tag_list: "Sport") visit admin_homepage_path + within("#edit_setting_#{user_recommendations.id}") do click_button "No" - end - expect(page).to have_content "Value updated" + expect(page).to have_button "Yes" + end login_as(user) visit root_path diff --git a/spec/system/admin/settings_spec.rb b/spec/system/admin/settings_spec.rb index 4b29ec014..74c90a760 100644 --- a/spec/system/admin/settings_spec.rb +++ b/spec/system/admin/settings_spec.rb @@ -268,9 +268,13 @@ describe "Admin settings", :admin do visit admin_settings_path click_link "SDG configuration" - within("tr", text: "Whatever") { click_button "No" } - expect(page).to have_content "Value updated" + within("tr", text: "Whatever") do + click_button "No" + + expect(page).to have_button "Yes" + end + expect(page).to have_current_path(admin_settings_path) expect(page).to have_css("h2", exact_text: "SDG configuration") end @@ -282,9 +286,12 @@ describe "Admin settings", :admin do visit admin_settings_path find("#features-tab").click - within("tr", text: "Skip user verification") { click_button "Yes" } - expect(page).to have_content "Value updated" + within("tr", text: "Skip user verification") do + click_button "Yes" + + expect(page).to have_button "No" + end end scenario "activate skip verification" do @@ -292,9 +299,12 @@ describe "Admin settings", :admin do visit admin_settings_path find("#features-tab").click - within("tr", text: "Skip user verification") { click_button "No" } - expect(page).to have_content "Value updated" + within("tr", text: "Skip user verification") do + click_button "No" + + expect(page).to have_button "Yes" + end end end @@ -320,5 +330,24 @@ describe "Admin settings", :admin do "Sustainable Development Goals you must " \ 'enable "SDG" on "Features" tab.' end + + scenario "is enabled right after enabling the feature" do + Setting["feature.sdg"] = false + login_as(create(:administrator).user) + + visit admin_settings_path + + click_link "Features" + + within("tr", text: "SDG") do + click_button "No" + + expect(page).to have_button "Yes" + end + + click_link "SDG configuration" + + expect(page).to have_css "h2", exact_text: "SDG configuration" + end end end