From f1105140ae0eb339e958a2a1805abe7ed3a63870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 29 Sep 2023 21:36:38 +0200 Subject: [PATCH 1/6] Remove duplicate
tags in management views There can only be one
tag in a document, and we've already got a
tag in the management layout. --- .../budgets/investments/index.html.erb | 48 ++++++++------- .../budgets/investments/print.html.erb | 58 +++++++++---------- app/views/management/proposals/index.html.erb | 34 +++++------ app/views/management/proposals/print.html.erb | 44 +++++++------- 4 files changed, 88 insertions(+), 96 deletions(-) diff --git a/app/views/management/budgets/investments/index.html.erb b/app/views/management/budgets/investments/index.html.erb index dd64ebeaf..4dfc94848 100644 --- a/app/views/management/budgets/investments/index.html.erb +++ b/app/views/management/budgets/investments/index.html.erb @@ -1,30 +1,28 @@ -
- - <%= render "admin/shared/budget_investment_search", url: management_budget_investments_path(@budget) %> - + + <%= render "admin/shared/budget_investment_search", url: management_budget_investments_path(@budget) %> + -
-
+
+
-
- <%= tag.h2 t("management.budget_investments.filters.unfeasible") if params[:unfeasible].present? %> - <%= tag.h2 t("management.budget_investments.filters.heading", heading: @heading.name) if @heading.present? %> - <% if params[:search].present? %> -

- <%= page_entries_info @investments %> - <%= t("management.budget_investments.search_results", count: @investments.size, search_term: params[:search]) %> -

- <% end %> -
- - <% @investments.each do |investment| %> - <%= render "/budgets/investments/investment", - investment: investment, - investment_ids: @investment_ids, - ballot: @ballot %> +
+ <%= tag.h2 t("management.budget_investments.filters.unfeasible") if params[:unfeasible].present? %> + <%= tag.h2 t("management.budget_investments.filters.heading", heading: @heading.name) if @heading.present? %> + <% if params[:search].present? %> +

+ <%= page_entries_info @investments %> + <%= t("management.budget_investments.search_results", count: @investments.size, search_term: params[:search]) %> +

<% end %> - - <%= paginate @investments %>
+ + <% @investments.each do |investment| %> + <%= render "/budgets/investments/investment", + investment: investment, + investment_ids: @investment_ids, + ballot: @ballot %> + <% end %> + + <%= paginate @investments %>
-
+ diff --git a/app/views/management/budgets/investments/print.html.erb b/app/views/management/budgets/investments/print.html.erb index 9eb52c9af..4b5de991d 100644 --- a/app/views/management/budgets/investments/print.html.erb +++ b/app/views/management/budgets/investments/print.html.erb @@ -1,37 +1,35 @@ -
- - <%= render "admin/shared/budget_investment_search", url: print_management_budget_investments_path(@budget) %> - + + <%= render "admin/shared/budget_investment_search", url: print_management_budget_investments_path(@budget) %> + -
-
+
+
- + -
- <% if params[:unfeasible].present? %> -

<%= t("management.budget_investments.filters.unfeasible") %>

- <% end %> - <% if @heading.present? %> -

<%= t("management.budget_investments.filters.heading", heading_name: @heading.name) %>

- <% end %> -

<%= t("management.budget_investments.search_results", count: @investments.count, search_term: params[:search]) %>

-
- - <% @investments.each do |investment| %> - <%= render "/budgets/investments/investment", - investment: investment, - investment_ids: @investment_ids, - ballot: @ballot %> +
+ <% if params[:unfeasible].present? %> +

<%= t("management.budget_investments.filters.unfeasible") %>

<% end %> + <% if @heading.present? %> +

<%= t("management.budget_investments.filters.heading", heading_name: @heading.name) %>

+ <% end %> +

<%= t("management.budget_investments.search_results", count: @investments.count, search_term: params[:search]) %>

+
-
-

<%= t("management.print.budget_investments_info") %>

-
+ <% @investments.each do |investment| %> + <%= render "/budgets/investments/investment", + investment: investment, + investment_ids: @investment_ids, + ballot: @ballot %> + <% end %> + +
+

<%= t("management.print.budget_investments_info") %>

-
+ diff --git a/app/views/management/proposals/index.html.erb b/app/views/management/proposals/index.html.erb index da89c1dbf..3a47eb1c1 100644 --- a/app/views/management/proposals/index.html.erb +++ b/app/views/management/proposals/index.html.erb @@ -1,24 +1,22 @@ -
-

<%= t("management.proposals.index.title") %>

+

<%= t("management.proposals.index.title") %>

- <%= render Admin::SearchComponent.new(label: t("admin.shared.search.label.proposals")) %> +<%= render Admin::SearchComponent.new(label: t("admin.shared.search.label.proposals")) %> -
-
+
+
-
- <% if @search_terms %> -

- <%= page_entries_info @proposals %> - <%= sanitize(t("proposals.index.search_results", - count: @proposals.size, - search_term: strip_tags(@search_terms))) %> -

- <% end %> +
+ <% if @search_terms %> +

+ <%= page_entries_info @proposals %> + <%= sanitize(t("proposals.index.search_results", + count: @proposals.size, + search_term: strip_tags(@search_terms))) %> +

+ <% end %> - <%= render @proposals %> - <%= paginate @proposals %> -
+ <%= render @proposals %> + <%= paginate @proposals %>
-
+ diff --git a/app/views/management/proposals/print.html.erb b/app/views/management/proposals/print.html.erb index 6dc20b639..ccf909753 100644 --- a/app/views/management/proposals/print.html.erb +++ b/app/views/management/proposals/print.html.erb @@ -1,29 +1,27 @@ -
-
-
- - <%= t("management.proposals.print.print_button") %> - +
+
+ + <%= t("management.proposals.print.print_button") %> + - <%= image_tag "header_print_proposals.jpg", class: "for-print-only" %> + <%= image_tag "header_print_proposals.jpg", class: "for-print-only" %> -
- <%= l Date.current, format: :long %> +
+ <%= l Date.current, format: :long %> -

- <%= t("proposals.index.select_order_long") %> - <%= t("management.print.proposals_title") %> -

-
- <%= render "shared/order_links", i18n_namespace: "proposals.index" %> -
-
- - <%= render @proposals %> - -
-

<%= t("management.print.proposals_info") %>

+

+ <%= t("proposals.index.select_order_long") %> + <%= t("management.print.proposals_title") %> +

+
+ <%= render "shared/order_links", i18n_namespace: "proposals.index" %>
+ + <%= render @proposals %> + +
+

<%= t("management.print.proposals_info") %>

+
-
+ From 9c037a484eda9dbfebe621d21699614e37b7c049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 29 Sep 2023 22:01:47 +0200 Subject: [PATCH 2/6] Make proposals map test more robust We were testing what happens when clicking on a geozone without HTML coordinates, which won't happen in a real browser. So we're now defining the HTML coordinates and clicking on the area in the test, which is what real people will do. We also avoid having two consecutive `visit` calls, which will interfere with the way we plan to test the presence of the
tag after every `visit`. Note that, the test didn't work with the HTML coordinates defined in the `with_html_coordinates` trait, with Capybara showing the following error: ``` Selenium::WebDriver::Error::ElementClickInterceptedError: element click intercepted: Element California is not clickable at point (413, 456). Other element would receive the click: ``` The cause of this error was the strange shape of the polygon, which was greatly concave and and so the middle of its area wasn't part of it. We're changing the polygon so it's now convex and when Capybara clicks on its middle point everything will work as expected. --- spec/factories/administration.rb | 2 +- spec/system/proposals_spec.rb | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/spec/factories/administration.rb b/spec/factories/administration.rb index 064f06392..1855b1069 100644 --- a/spec/factories/administration.rb +++ b/spec/factories/administration.rb @@ -15,7 +15,7 @@ FactoryBot.define do end trait :with_html_coordinates do - html_map_coordinates { "30,139,45,153,77,148,107,165" } + html_map_coordinates { "30,139,45,153,77,148,107,125" } end trait :with_geojson do diff --git a/spec/system/proposals_spec.rb b/spec/system/proposals_spec.rb index ff45521ee..6df959d81 100644 --- a/spec/system/proposals_spec.rb +++ b/spec/system/proposals_spec.rb @@ -1353,8 +1353,8 @@ describe "Proposals" do context "Filter" do context "By geozone" do - let(:california) { Geozone.create(name: "California") } - let(:new_york) { Geozone.create(name: "New York") } + let(:california) { create(:geozone, :with_html_coordinates, name: "California") } + let(:new_york) { create(:geozone, name: "New York") } before do create(:proposal, geozone: california, title: "Bigger sequoias") @@ -1362,14 +1362,11 @@ describe "Proposals" do create(:proposal, geozone: new_york, title: "Sully monument") end - scenario "From map", :no_js do + scenario "From map" do visit proposals_path click_link "map" - within("#html_map") do - url = find("area[title='California']")[:href] - visit url - end + within("#html_map") { find("area[title='California']").click } within("#proposals") do expect(page).to have_css(".proposal", count: 2) From 2b962f2789918ebf5aa200d3cd5174a4acd6b509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 29 Sep 2023 22:11:48 +0200 Subject: [PATCH 3/6] Use a
tag on every page Many pages had this tag, but many other didn't, which made navigation inconsistent for people using screen readers. Note that there are slight changes in two pages: * The homepage now includes the banner and the content of the `shared/header` element inside the
tag * The budgets index now includes the banner inside the
tag I see both potential advantages and disadvantages of this approach, since banners aren't necessarily related to the main content of a page but on the other hand they aren't the same across pages and people using screen readers might accidentally skip them if they jump to the
tag. So I'm choosing the option that is easier to implement. Note we're adding a `public-content` class to the
element in the application layout. This might be redundat because the element could already be accessed through the `.public main` selector, but this is consistent with the `admin-content` class used in the admin section, and without it the
element would sometimes have an empty class attribute and we'd have to use `if content_for?(:main_class)` or `tag.main` which IMHO makes the code less consistent. The Capybara::DSL monkey-patch is only done on the `visit` method because it's the only reliable one. Other methods like `click_link` generate AJAX requests, so `expect(page).to have_css "main", count: 1` might be executed before the AJAX request is finished, meaning it wouldn't properly test anything. --- .../admin/budgets/show_component.html.erb | 38 ++-- .../headings/index_component.html.erb | 22 +- .../budgets/groups/index_component.html.erb | 8 +- .../investments/new_component.html.erb | 10 +- app/components/debates/new_component.html.erb | 32 +-- .../moderation/users/index_component.html.erb | 84 ++++---- .../proposals/new_component.html.erb | 30 +-- .../sdg/goals/index_component.html.erb | 41 ++-- .../sdg/goals/show_component.html.erb | 65 +++--- .../subscriptions/edit_component.html.erb | 22 +- app/views/budgets/ballot/show.html.erb | 6 +- app/views/budgets/groups/show.html.erb | 48 ++--- app/views/budgets/index.html.erb | 35 ++- app/views/budgets/investments/index.html.erb | 126 ++++++----- app/views/debates/index.html.erb | 123 ++++++----- app/views/layouts/admin.html.erb | 2 +- app/views/layouts/application.html.erb | 6 +- app/views/layouts/dashboard.html.erb | 4 +- app/views/layouts/devise.html.erb | 4 +- app/views/proposals/index.html.erb | 203 +++++++++--------- app/views/proposals/summary.html.erb | 82 ++++--- app/views/topics/edit.html.erb | 10 +- app/views/topics/new.html.erb | 26 +-- app/views/users/show.html.erb | 44 ++-- app/views/welcome/index.html.erb | 42 ++-- public/403.html | 4 +- public/404.html | 4 +- public/422.html | 4 +- public/500.html | 4 +- spec/rails_helper.rb | 14 ++ 30 files changed, 572 insertions(+), 571 deletions(-) diff --git a/app/components/admin/budgets/show_component.html.erb b/app/components/admin/budgets/show_component.html.erb index 1ad9a1468..5f1142b5d 100644 --- a/app/components/admin/budgets/show_component.html.erb +++ b/app/components/admin/budgets/show_component.html.erb @@ -1,24 +1,24 @@ -
- <%= back_link_to admin_budgets_path %> +<% provide :main_class, "admin-budgets-show" %> - <%= header %> +<%= back_link_to admin_budgets_path %> - <%= render Admin::Budgets::DraftingComponent.new(budget) %> - <%= render Admin::Budgets::LinksComponent.new(budget) %> +<%= header %> -
-

<%= t("admin.budgets.show.groups_and_headings") %>

- <%= render Admin::Budgets::GroupsAndHeadingsComponent.new(budget) %> -
+<%= render Admin::Budgets::DraftingComponent.new(budget) %> +<%= render Admin::Budgets::LinksComponent.new(budget) %> -
-

<%= t("admin.budgets.edit.phases_caption") %>

- <%= t("admin.budgets.edit.phases_table_help_text") %> - <%= render Admin::BudgetPhases::PhasesComponent.new(budget) %> -
+
+

<%= t("admin.budgets.show.groups_and_headings") %>

+ <%= render Admin::Budgets::GroupsAndHeadingsComponent.new(budget) %> +
-
-

<%= t("admin.budgets.edit.actions") %>

- <%= render Admin::Budgets::ActionsComponent.new(budget) %> -
-
+
+

<%= t("admin.budgets.edit.phases_caption") %>

+ <%= t("admin.budgets.edit.phases_table_help_text") %> + <%= render Admin::BudgetPhases::PhasesComponent.new(budget) %> +
+ +
+

<%= t("admin.budgets.edit.actions") %>

+ <%= render Admin::Budgets::ActionsComponent.new(budget) %> +
diff --git a/app/components/admin/budgets_wizard/headings/index_component.html.erb b/app/components/admin/budgets_wizard/headings/index_component.html.erb index b5989b9de..e010d0fdd 100644 --- a/app/components/admin/budgets_wizard/headings/index_component.html.erb +++ b/app/components/admin/budgets_wizard/headings/index_component.html.erb @@ -1,14 +1,14 @@ -
- <%= back_link_to admin_budgets_wizard_budget_groups_path(budget, url_params), back_link_text %> +<% provide :main_class, "admin-budgets-wizard-headings-index" %> - <%= header %> +<%= back_link_to admin_budgets_wizard_budget_groups_path(budget, url_params), back_link_text %> - <%= render Admin::Budgets::HelpComponent.new("headings") %> - <%= render Admin::BudgetsWizard::CreationTimelineComponent.new("headings") %> +<%= header %> - <% unless single_heading? %> - <%= render Admin::BudgetsWizard::Headings::GroupSwitcherComponent.new(group) %> - <%= render Admin::BudgetHeadings::HeadingsComponent.new(headings) %> - <% end %> - <%= render Admin::BudgetsWizard::Headings::CreationStepComponent.new(new_heading) %> -
+<%= render Admin::Budgets::HelpComponent.new("headings") %> +<%= render Admin::BudgetsWizard::CreationTimelineComponent.new("headings") %> + +<% unless single_heading? %> + <%= render Admin::BudgetsWizard::Headings::GroupSwitcherComponent.new(group) %> + <%= render Admin::BudgetHeadings::HeadingsComponent.new(headings) %> +<% end %> +<%= render Admin::BudgetsWizard::Headings::CreationStepComponent.new(new_heading) %> diff --git a/app/components/budgets/groups/index_component.html.erb b/app/components/budgets/groups/index_component.html.erb index e577bad50..d0fd0028d 100644 --- a/app/components/budgets/groups/index_component.html.erb +++ b/app/components/budgets/groups/index_component.html.erb @@ -1,8 +1,8 @@ +<% provide :main_class, "budget-groups-index" %> + <% content_for :canonical do %> <%= render "shared/canonical", href: budget_groups_url %> <% end %> -
- <%= header(before: back_link_to(budget_path(budget))) %> - <%= render Budgets::GroupsAndHeadingsComponent.new(budget) %> -
+<%= header(before: back_link_to(budget_path(budget))) %> +<%= render Budgets::GroupsAndHeadingsComponent.new(budget) %> diff --git a/app/components/budgets/investments/new_component.html.erb b/app/components/budgets/investments/new_component.html.erb index 49ff6f2f2..69eaada5b 100644 --- a/app/components/budgets/investments/new_component.html.erb +++ b/app/components/budgets/investments/new_component.html.erb @@ -1,7 +1,7 @@ -
- <%= back_link_to budgets_path %> +<% provide :main_class, "budget-investment-new" %> - <%= header %> +<%= back_link_to budgets_path %> - <%= render "/budgets/investments/form", form_url: budget_investments_path(budget) %> -
+<%= header %> + +<%= render "/budgets/investments/form", form_url: budget_investments_path(budget) %> diff --git a/app/components/debates/new_component.html.erb b/app/components/debates/new_component.html.erb index 1335ca037..b19f45cc0 100644 --- a/app/components/debates/new_component.html.erb +++ b/app/components/debates/new_component.html.erb @@ -1,20 +1,20 @@ -
- <%= back_link_to debates_path, t("debates.index.section_header.title") %> +<% provide :main_class, "debate-new" %> - <%= header do %> - <%= new_window_link_to t("debates.new.more_info"), help_path(anchor: "debates") %> - <% end %> +<%= back_link_to debates_path, t("debates.index.section_header.title") %> - +
+
    +
  • <%= t("debates.new.recommendation_one") %>
  • +
  • <%= t("debates.new.recommendation_two") %>
  • +
  • <%= t("debates.new.recommendation_three") %>
  • +
  • <%= t("debates.new.recommendation_four") %>
  • +
+ + +<%= render Debates::FormComponent.new(debate) %> diff --git a/app/components/moderation/users/index_component.html.erb b/app/components/moderation/users/index_component.html.erb index ef5cb7db2..24962c975 100644 --- a/app/components/moderation/users/index_component.html.erb +++ b/app/components/moderation/users/index_component.html.erb @@ -1,47 +1,47 @@ -
-

<%= t("moderation.users.index.title") %>

+<% provide :main_class, "moderation-users-index" %> - <%= render Admin::SearchComponent.new(label: t("moderation.users.index.search_placeholder")) %> +

<%= t("moderation.users.index.title") %>

- <% if users.present? %> -

<%= page_entries_info users %>

+<%= render Admin::SearchComponent.new(label: t("moderation.users.index.search_placeholder")) %> - - - - - - - <% users.each do |user| %> - - - + + <% end %> + +
<%= t("admin.hidden_users.index.user") %><%= t("admin.actions.actions") %>
- <%= user.name %> - - <% if user.hidden? %> - <%= status(user) %> - <% else %> - <%= render Admin::TableActionsComponent.new(user, actions: []) do |actions| %> - <%= actions.action( - :hide, - text: t("moderation.users.index.hide"), - confirm: ->(name) { t("moderation.users.index.confirm_hide", name: name) }, - method: :put - ) %> - <%= actions.action( - :block, - text: t("moderation.users.index.block"), - confirm: ->(name) { t("moderation.users.index.confirm_block", name: name) }, - method: :put - ) %> - <% end %> +<% if users.present? %> +

<%= page_entries_info users %>

+ + + + + + + + <% users.each do |user| %> + + + - - <% end %> - -
<%= t("admin.hidden_users.index.user") %><%= t("admin.actions.actions") %>
+ <%= user.name %> + + <% if user.hidden? %> + <%= status(user) %> + <% else %> + <%= render Admin::TableActionsComponent.new(user, actions: []) do |actions| %> + <%= actions.action( + :hide, + text: t("moderation.users.index.hide"), + confirm: ->(name) { t("moderation.users.index.confirm_hide", name: name) }, + method: :put + ) %> + <%= actions.action( + :block, + text: t("moderation.users.index.block"), + confirm: ->(name) { t("moderation.users.index.confirm_block", name: name) }, + method: :put + ) %> <% end %> -
+ <% end %> +
- <%= paginate users %> - <% end %> -
+ <%= paginate users %> +<% end %> diff --git a/app/components/proposals/new_component.html.erb b/app/components/proposals/new_component.html.erb index 508335b8d..72551a0a2 100644 --- a/app/components/proposals/new_component.html.erb +++ b/app/components/proposals/new_component.html.erb @@ -1,19 +1,19 @@ -
- <%= back_link_to proposals_path, t("proposals.index.section_header.title") %> +<% provide :main_class, "proposal-new" %> - <%= header do %> - <%= new_window_link_to t("proposals.new.more_info"), help_path(anchor: "proposals") %> - <% end %> +<%= back_link_to proposals_path, t("proposals.index.section_header.title") %> - +
+
    +
  • <%= t("proposals.new.recommendation_one") %>
  • +
  • <%= t("proposals.new.recommendation_two") %>
  • +
  • <%= t("proposals.new.recommendation_three") %>
  • +
+ + +<%= render Proposals::FormComponent.new(proposal, url: proposals_path) %> diff --git a/app/components/sdg/goals/index_component.html.erb b/app/components/sdg/goals/index_component.html.erb index cc4ff27d2..bb2b0e285 100644 --- a/app/components/sdg/goals/index_component.html.erb +++ b/app/components/sdg/goals/index_component.html.erb @@ -1,25 +1,24 @@ <% provide(:title) { title } %> +<% provide :main_class, "sdg-goals-index" %> -
- <% if header.present? %> - <%= render "shared/header", header: header %> - <% else %> -
-

<%= title %>

+<% if header.present? %> + <%= render "shared/header", header: header %> +<% else %> +
+

<%= title %>

+
+<% end %> + +<%= render Shared::BannerComponent.new("sdg") %> + +<%= link_list(*goal_links, class: "sdg-goal-list") %> + +<% phases.each do |phase| %> +
+
+

<%= phase.title %>

- <% end %> - <%= render Shared::BannerComponent.new("sdg") %> - - <%= link_list(*goal_links, class: "sdg-goal-list") %> - - <% phases.each do |phase| %> -
-
-

<%= phase.title %>

-
- - <%= render "shared/cards", cards: phase.cards.sort_by_order %> -
- <% end %> -
+ <%= render "shared/cards", cards: phase.cards.sort_by_order %> + +<% end %> diff --git a/app/components/sdg/goals/show_component.html.erb b/app/components/sdg/goals/show_component.html.erb index 74b2a3a6b..f49df6e46 100644 --- a/app/components/sdg/goals/show_component.html.erb +++ b/app/components/sdg/goals/show_component.html.erb @@ -1,39 +1,38 @@ <% provide(:title) { goal.title } %> +<% provide :main_class, "sdg-goal-show" %> -
- <%= back_link_to sdg_goals_path %> +<%= back_link_to sdg_goals_path %> -
-
-

<%= heading %>

-
-
-
- <%= long_description %> -
-
- - -
+
+
+

<%= heading %>

+
+
+
+ <%= long_description %>
-
- - <%= render ::Widget::Feeds::ParticipationComponent.new(feeds) %> - - <% if processes_feed %> -
- <%= render ::Widget::Feeds::FeedComponent.new(processes_feed) %> +
+ +
- <% end %> +
+
- <%= render SDG::Goals::TargetsComponent.new(goal) %> -
+<%= render ::Widget::Feeds::ParticipationComponent.new(feeds) %> + +<% if processes_feed %> +
+ <%= render ::Widget::Feeds::FeedComponent.new(processes_feed) %> +
+<% end %> + +<%= render SDG::Goals::TargetsComponent.new(goal) %> diff --git a/app/components/subscriptions/edit_component.html.erb b/app/components/subscriptions/edit_component.html.erb index 09e65926b..73320ff8d 100644 --- a/app/components/subscriptions/edit_component.html.erb +++ b/app/components/subscriptions/edit_component.html.erb @@ -1,13 +1,13 @@ -
- <%= form_for user, url: subscriptions_path(token: user.subscriptions_token) do |f| %> -

<%= t("account.show.notifications") %>

+<% provide :main_class, "subscriptions-edit" %> -
<%= f.check_box :email_on_comment %>
-
<%= f.check_box :email_on_comment_reply %>
-
<%= f.check_box :newsletter %>
-
<%= f.check_box :email_digest %>
-
<%= f.check_box :email_on_direct_message %>
+<%= form_for user, url: subscriptions_path(token: user.subscriptions_token) do |f| %> +

<%= t("account.show.notifications") %>

- <%= f.submit t("account.show.save_changes_submit"), class: "button margin-top" %> - <% end %> -
+
<%= f.check_box :email_on_comment %>
+
<%= f.check_box :email_on_comment_reply %>
+
<%= f.check_box :newsletter %>
+
<%= f.check_box :email_digest %>
+
<%= f.check_box :email_on_direct_message %>
+ + <%= f.submit t("account.show.save_changes_submit"), class: "button margin-top" %> +<% end %> diff --git a/app/views/budgets/ballot/show.html.erb b/app/views/budgets/ballot/show.html.erb index 0b9edf168..faa1a7061 100644 --- a/app/views/budgets/ballot/show.html.erb +++ b/app/views/budgets/ballot/show.html.erb @@ -1,3 +1,5 @@ -
+<% provide :main_class, "budget-ballot-show" %> + +
<%= render "budgets/ballot/ballot" %> -
+ diff --git a/app/views/budgets/groups/show.html.erb b/app/views/budgets/groups/show.html.erb index 6997f6485..f0687630a 100644 --- a/app/views/budgets/groups/show.html.erb +++ b/app/views/budgets/groups/show.html.erb @@ -1,28 +1,28 @@ -
-
- <%= back_link_to budget_path(@budget) %> -

<%= t("budgets.groups.show.title") %>

-
+<% provide :main_class, "budget-group-show" %> -
-
-
- <% @group.headings.sort_by_name.each_slice(7) do |slice| %> -
- <% slice.each do |heading| %> - - <%= link_to heading.name, budget_investments_path(heading_id: heading.id) %> -
-
- <% end %> -
- <% end %> -
-
+
+ <%= back_link_to budget_path(@budget) %> +

<%= t("budgets.groups.show.title") %>

+
-
- <%= image_tag(image_path_for("map.jpg")) %> +
+
+
+ <% @group.headings.sort_by_name.each_slice(7) do |slice| %> +
+ <% slice.each do |heading| %> + + <%= link_to heading.name, budget_investments_path(heading_id: heading.id) %> +
+
+ <% end %> +
+ <% end %>
-
+ +
+ <%= image_tag(image_path_for("map.jpg")) %> +
+ diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index cad36ad83..c9f677908 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -1,31 +1,30 @@ <%= render Shared::BannerComponent.new("budgets") %> <% provide :title do %><%= t("budgets.index.title") %><% end %> +<% provide :main_class, "budgets-index" %> <% content_for :canonical do %> <%= render "shared/canonical", href: budgets_url %> <% end %> -
- <% if @budget.present? %> - <%= render Budgets::BudgetComponent.new(@budget) %> +<% if @budget.present? %> + <%= render Budgets::BudgetComponent.new(@budget) %> - <% if @finished_budgets.present? %> - <%= render "finished", budgets: @finished_budgets %> - <% end %> - <% else %> -
-

<%= t("budgets.index.title") %>

-
+ <% if @finished_budgets.present? %> + <%= render "finished", budgets: @finished_budgets %> + <% end %> +<% else %> +
+

<%= t("budgets.index.title") %>

+
-
-
-
- <%= t("budgets.index.empty_budgets") %> -
+
+
+
+ <%= t("budgets.index.empty_budgets") %>
- <% end %> +
+<% end %> - <%= render Budgets::FooterComponent.new %> -
+<%= render Budgets::FooterComponent.new %> diff --git a/app/views/budgets/investments/index.html.erb b/app/views/budgets/investments/index.html.erb index 08a481d6d..8d35a1ef8 100644 --- a/app/views/budgets/investments/index.html.erb +++ b/app/views/budgets/investments/index.html.erb @@ -15,76 +15,74 @@ <% end %> <% end %> -
- <% if @search_terms || @advanced_search_terms %> - <%= render Shared::SearchResultsSummaryComponent.new( - results: @investments, - search_terms: @search_terms, - advanced_search_terms: @advanced_search_terms - ) %> - <% else %> - <%= render "/budgets/investments/header" %> - <% end %> +<% if @search_terms || @advanced_search_terms %> + <%= render Shared::SearchResultsSummaryComponent.new( + results: @investments, + search_terms: @search_terms, + advanced_search_terms: @advanced_search_terms + ) %> +<% else %> + <%= render "/budgets/investments/header" %> +<% end %> -
-
+
+
- <% if @current_filter == "unfeasible" %> -
-

<%= t("budgets.investments.index.unfeasible") %>

-
- <%= t("budgets.investments.index.unfeasible_text") %> -
+ <% if @current_filter == "unfeasible" %> +
+

<%= t("budgets.investments.index.unfeasible") %>

+
+ <%= t("budgets.investments.index.unfeasible_text") %>
- <% elsif @heading.present? %> -
-
- <%= render "view_mode" %> -
+
+ <% elsif @heading.present? %> +
+
+ <%= render "view_mode" %>
+
+ <% end %> + + <%= render Shared::AdvancedSearchComponent.new %> + + <% if unfeasible_or_unselected_filter %> + + <% else %> + <%= render("shared/order_links", i18n_namespace: "budgets.investments.index") %> + <% end %> + + <% if investments_default_view? %> + + <% @investments.each do |investment| %> + <%= render "/budgets/investments/investment", + investment: investment, + investment_ids: @investment_ids, + ballot: @ballot %> <% end %> + <% else %> - <%= render Shared::AdvancedSearchComponent.new %> - - <% if unfeasible_or_unselected_filter %> - - <% else %> - <%= render("shared/order_links", i18n_namespace: "budgets.investments.index") %> + <% @investments.each do |investment| %> + <%= render "/budgets/investments/investment_minimal", + investment: investment %> <% end %> + <% end %> - <% if investments_default_view? %> - - <% @investments.each do |investment| %> - <%= render "/budgets/investments/investment", - investment: investment, - investment_ids: @investment_ids, - ballot: @ballot %> - <% end %> - <% else %> - - <% @investments.each do |investment| %> - <%= render "/budgets/investments/investment_minimal", - investment: investment %> - <% end %> - <% end %> - - <%= paginate @investments %> -
- -
- -
- + <%= paginate @investments %>
-
+ +
+ +
+ + diff --git a/app/views/debates/index.html.erb b/app/views/debates/index.html.erb index 6e51a2b5e..e711762aa 100644 --- a/app/views/debates/index.html.erb +++ b/app/views/debates/index.html.erb @@ -8,80 +8,77 @@ <%= render "shared/canonical", href: debates_url %> <% end %> -
- <% if @search_terms || @advanced_search_terms %> - <%= render Shared::SearchResultsSummaryComponent.new( - results: @debates, - search_terms: @search_terms, - advanced_search_terms: @advanced_search_terms - ) %> - <% else %> - <%= render "shared/section_header", i18n_namespace: "debates.index.section_header", image: "debates" %> - <% end %> +<% if @search_terms || @advanced_search_terms %> + <%= render Shared::SearchResultsSummaryComponent.new( + results: @debates, + search_terms: @search_terms, + advanced_search_terms: @advanced_search_terms + ) %> +<% else %> + <%= render "shared/section_header", i18n_namespace: "debates.index.section_header", image: "debates" %> +<% end %> - <% if feature?("user.recommendations") && @recommended_debates.present? %> - <%= render "shared/recommended_index", recommended: @recommended_debates, - disable_recommendations_path: recommendations_disable_debates_path, - namespace: "debates" %> - <% end %> +<% if feature?("user.recommendations") && @recommended_debates.present? %> + <%= render "shared/recommended_index", recommended: @recommended_debates, + disable_recommendations_path: recommendations_disable_debates_path, + namespace: "debates" %> +<% end %> -
-
+
+
- <%= render Shared::BannerComponent.new("debates") %> + <%= render Shared::BannerComponent.new("debates") %> - <% unless @search_terms || !has_featured? %> - <%= render "featured_debates" %> - <% end %> + <% unless @search_terms || !has_featured? %> + <%= render "featured_debates" %> + <% end %> -
-
- <%= render "view_mode" %> -
+
+
+ <%= render "view_mode" %>
+
- <%= render Shared::AdvancedSearchComponent.new %> + <%= render Shared::AdvancedSearchComponent.new %> - <%= render "shared/order_links", i18n_namespace: "debates.index" %> + <%= render "shared/order_links", i18n_namespace: "debates.index" %> -
- <%= link_to t("debates.index.start_debate"), new_debate_path, class: "button expanded" %> -
+
+ <%= link_to t("debates.index.start_debate"), new_debate_path, class: "button expanded" %> +
- <% if @debates.any? || current_user.blank? %> - <% if debates_default_view? %> - <%= render @debates %> - <% else %> - <% @debates.each do |debate| %> - <%= render "debates/debate_minimal", debate: debate %> - <% end %> - <% end %> + <% if @debates.any? || current_user.blank? %> + <% if debates_default_view? %> + <%= render @debates %> <% else %> - <%= empty_recommended_debates_message_text(current_user) %> + <% @debates.each do |debate| %> + <%= render "debates/debate_minimal", debate: debate %> + <% end %> <% end %> - <%= paginate @debates %> - - <% unless @search_terms || @advanced_search_terms %> -
-

- <%= t("debates.index.section_footer.title") %> -

-

<%= t("debates.index.section_footer.description") %>

-

<%= t("debates.index.section_footer.help_text_1") %>

-

<%= sanitize(t("debates.index.section_footer.help_text_2", - org: link_to(setting["org_name"], new_user_registration_path))) %>

-

-
- <% end %> -
- -
- - -
+ <% else %> + <%= empty_recommended_debates_message_text(current_user) %> + <% end %> + <%= paginate @debates %> + <% unless @search_terms || @advanced_search_terms %> +
+

+ <%= t("debates.index.section_footer.title") %> +

+

<%= t("debates.index.section_footer.description") %>

+

<%= t("debates.index.section_footer.help_text_1") %>

+

<%= sanitize(t("debates.index.section_footer.help_text_2", + org: link_to(setting["org_name"], new_user_registration_path))) %>

+

+
+ <% end %>
-
+ +
+ + +
+ diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb index 7049c9aa5..c84b71711 100644 --- a/app/views/layouts/admin.html.erb +++ b/app/views/layouts/admin.html.erb @@ -22,7 +22,7 @@ <% end %> -
+
<%= label_tag :show_menu, t("admin.menu.admin"), "aria-hidden": true, class: "button hollow expanded" %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 58cffe017..4d3711b6a 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -17,9 +17,11 @@
<%= render "layouts/header", with_subnavigation: true %> - <%= render "layouts/flash" %> - <%= yield %> +
+ <%= render "layouts/flash" %> + <%= yield %> +
diff --git a/app/views/layouts/devise.html.erb b/app/views/layouts/devise.html.erb index 26943bf56..daf933f52 100644 --- a/app/views/layouts/devise.html.erb +++ b/app/views/layouts/devise.html.erb @@ -21,11 +21,11 @@
-
+
<%= render "layouts/flash" %> <%= yield %> -
+
diff --git a/app/views/proposals/index.html.erb b/app/views/proposals/index.html.erb index 57dd73dc6..00cc1c7e3 100644 --- a/app/views/proposals/index.html.erb +++ b/app/views/proposals/index.html.erb @@ -8,123 +8,120 @@ <%= render "shared/canonical", href: proposals_url %> <% end %> -
- <% if [ - @search_terms, - @advanced_search_terms, - params[:retired].present?, - params[:selected].present? - ].any? %> - <%= render Shared::SearchResultsSummaryComponent.new( - results: @proposals, - search_terms: @search_terms, - advanced_search_terms: @advanced_search_terms - ) do %> - <% if params[:retired].present? %> -

<%= t("proposals.index.retired_proposals") %>

- <% elsif params[:selected].present? %> -

<%= t("proposals.index.selected_proposals") %>

- <% end %> +<% if [ + @search_terms, + @advanced_search_terms, + params[:retired].present?, + params[:selected].present? + ].any? %> + <%= render Shared::SearchResultsSummaryComponent.new( + results: @proposals, + search_terms: @search_terms, + advanced_search_terms: @advanced_search_terms + ) do %> + <% if params[:retired].present? %> +

<%= t("proposals.index.retired_proposals") %>

+ <% elsif params[:selected].present? %> +

<%= t("proposals.index.selected_proposals") %>

<% end %> - <% else %> - <%= render "shared/section_header", i18n_namespace: "proposals.index.section_header", image: "proposals" %> <% end %> +<% else %> + <%= render "shared/section_header", i18n_namespace: "proposals.index.section_header", image: "proposals" %> +<% end %> - <% if show_recommended_proposals? %> - <%= render "shared/recommended_index", recommended: @recommended_proposals, - disable_recommendations_path: recommendations_disable_proposals_path, - namespace: "proposals" %> - <% end %> +<% if show_recommended_proposals? %> + <%= render "shared/recommended_index", recommended: @recommended_proposals, + disable_recommendations_path: recommendations_disable_proposals_path, + namespace: "proposals" %> +<% end %> -
-
+
+
- <%= render Shared::BannerComponent.new("proposals") %> + <%= render Shared::BannerComponent.new("proposals") %> - <% if show_featured_proposals? %> -
-
+ +
+ +
+ diff --git a/app/views/proposals/summary.html.erb b/app/views/proposals/summary.html.erb index 9f74c3963..fa88e17f0 100644 --- a/app/views/proposals/summary.html.erb +++ b/app/views/proposals/summary.html.erb @@ -1,53 +1,51 @@ -
-
-
- <%= back_link_to %> +
+
+ <%= back_link_to %> - <% @proposals.each do |group_name, proposals| %> -
-

<%= group_name %>

+ <% @proposals.each do |group_name, proposals| %> +
+

<%= group_name %>

- <% proposals[0..2].each do |proposal| %> -
-
-
-
-
-

<%= link_to proposal.title, namespaced_proposal_path(proposal) %>

+ <% proposals[0..2].each do |proposal| %> +
+
+
+
+
+

<%= link_to proposal.title, namespaced_proposal_path(proposal) %>

-

- <% if proposal.author.hidden? || proposal.author.erased? %> - <%= t("proposals.show.author_deleted") %> - <% else %> - <%= proposal.author.name %> - <% if proposal.author.display_official_position_badge? %> - - <%= proposal.author.official_position %> - - <% end %> +

+ <% if proposal.author.hidden? || proposal.author.erased? %> + <%= t("proposals.show.author_deleted") %> + <% else %> + <%= proposal.author.name %> + <% if proposal.author.display_official_position_badge? %> + + <%= proposal.author.official_position %> + <% end %> -

+ <% end %> +

-
-

<%= proposal.summary %>

-
+
+

<%= proposal.summary %>

- <% end %> -
- <% end %> -
- -
- -
+
+ <% end %> +
+ <% end %>
-
+ +
+ +
+ diff --git a/app/views/topics/edit.html.erb b/app/views/topics/edit.html.erb index 8788b1229..4b2d24b7c 100644 --- a/app/views/topics/edit.html.erb +++ b/app/views/topics/edit.html.erb @@ -1,7 +1,7 @@ -
- <%= back_link_to community_path(@community) %> +<% provide :main_class, "topic-edit" %> -

<%= t("community.topic.edit") %>

+<%= back_link_to community_path(@community) %> - <%= render "form" %> -
+

<%= t("community.topic.edit") %>

+ +<%= render "form" %> diff --git a/app/views/topics/new.html.erb b/app/views/topics/new.html.erb index aed9fbd71..85af118f1 100644 --- a/app/views/topics/new.html.erb +++ b/app/views/topics/new.html.erb @@ -1,17 +1,17 @@ -
- <%= back_link_to community_path(@community) %> +<% provide :main_class, "topic-new" %> -

<%= t("community.topic.create") %>

+<%= back_link_to community_path(@community) %> - +
+
    +
  • <%= t("community.topic.sidebar.recommendation_one") %>
  • +
  • <%= t("community.topic.sidebar.recommendation_two") %>
  • +
  • <%= t("community.topic.sidebar.recommendation_three") %>
  • +
+ + +<%= render "form" %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 0b500609b..8c85c6d0f 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -1,28 +1,26 @@ -
-
-
+
+
- <% if @user != current_user %> - <% if @user.email_on_direct_message? %> - <%= link_to t("users.show.send_private_message"), - new_user_direct_message_path(@user), - class: "button hollow float-right" %> - <% else %> -
- <%= t("users.show.no_private_messages") %> -
- <% end %> + <% if @user != current_user %> + <% if @user.email_on_direct_message? %> + <%= link_to t("users.show.send_private_message"), + new_user_direct_message_path(@user), + class: "button hollow float-right" %> + <% else %> +
+ <%= t("users.show.no_private_messages") %> +
<% end %> + <% end %> -

- <%= render Shared::AvatarComponent.new(@user, size: 60) %> - <%= @user.name %> - <% if current_user&.administrator? %> - <%= @user.email %> - <% end %> -

+

+ <%= render Shared::AvatarComponent.new(@user, size: 60) %> + <%= @user.name %> + <% if current_user&.administrator? %> + <%= @user.email %> + <% end %> +

- <%= render Users::PublicActivityComponent.new(@user) %> -
+ <%= render Users::PublicActivityComponent.new(@user) %>
-
+ diff --git a/app/views/welcome/index.html.erb b/app/views/welcome/index.html.erb index 717986f95..68a11c14d 100644 --- a/app/views/welcome/index.html.erb +++ b/app/views/welcome/index.html.erb @@ -13,28 +13,26 @@ <%= render "shared/header", header: @header %> -
- <%= render "feeds" %> +<%= render "feeds" %> -
- <% if @cards.any? %> -
"> -

<%= t("welcome.cards.title") %>

+
+ <% if @cards.any? %> +
"> +

<%= t("welcome.cards.title") %>

- <%= render "shared/cards", cards: @cards %> -
- <% end %> - - <% if feed_processes_enabled? %> -
- <%= render "processes" %> -
- <% end %> -
- - <% if feature?("user.recommendations") && (@recommended_debates.present? || @recommended_proposals.present?) %> - <%= render "recommended", - recommended_debates: @recommended_debates, - recommended_proposals: @recommended_proposals %> + <%= render "shared/cards", cards: @cards %> +
<% end %> -
+ + <% if feed_processes_enabled? %> +
+ <%= render "processes" %> +
+ <% end %> + + +<% if feature?("user.recommendations") && (@recommended_debates.present? || @recommended_proposals.present?) %> + <%= render "recommended", + recommended_debates: @recommended_debates, + recommended_proposals: @recommended_proposals %> +<% end %> diff --git a/public/403.html b/public/403.html index 7c0b688ef..da449d613 100644 --- a/public/403.html +++ b/public/403.html @@ -33,9 +33,9 @@ -
+

403

Access to this page has been disabled by the administrators.

-
+
diff --git a/public/404.html b/public/404.html index 5b5362192..82a7fdc35 100644 --- a/public/404.html +++ b/public/404.html @@ -33,9 +33,9 @@ -
+

404

Not found.

-
+
diff --git a/public/422.html b/public/422.html index cca90e5a3..3e118553d 100644 --- a/public/422.html +++ b/public/422.html @@ -33,9 +33,9 @@ -
+

422

The change you wanted was rejected.

-
+
diff --git a/public/500.html b/public/500.html index 0b563452a..3084a5e2c 100644 --- a/public/500.html +++ b/public/500.html @@ -33,9 +33,9 @@ -
+

500

Internal server error.

-
+
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 00577c207..a0309ef8a 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -53,6 +53,20 @@ end FactoryBot.use_parent_strategy = false +module Capybara + module DSL + alias_method :original_visit, :visit + + def visit(url, ...) + original_visit(url, ...) + + unless url.match?("robots.txt") || url.match?("active_storage/representations") + expect(page).to have_css "main", count: 1 + end + end + end +end + Capybara.register_driver :headless_chrome do |app| options = Selenium::WebDriver::Chrome::Options.new.tap do |opts| opts.add_argument "--headless" From 45c1f9356237d7217de82c76a068407c640b839b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 29 Sep 2023 22:30:34 +0200 Subject: [PATCH 4/6] Add a link to skip to the main content While people using screen readers already have keyboard shortcuts to jump to the
tag, there are people who navigate the page with the keyboard using just the tab key, and for them, this link provides a way to save time and start reading the main content instead of having to manually go through all the navigation links every time a new page is loaded. Note that we had to add an additional `width: 0` rule because Foundation's `element-invisible` would apply `1px` and the test checking for `visible: :hidden` would faile. --- .../layout/skip_to_main_content.scss | 22 +++++++++++++++++++ .../skip_to_main_content_component.html.erb | 3 +++ .../layout/skip_to_main_content_component.rb | 2 ++ app/views/layouts/admin.html.erb | 3 ++- app/views/layouts/application.html.erb | 3 ++- app/views/layouts/dashboard.html.erb | 3 ++- app/views/layouts/devise.html.erb | 3 ++- app/views/layouts/management.html.erb | 3 ++- config/locales/en/general.yml | 1 + config/locales/es/general.yml | 1 + public/403.html | 2 +- public/404.html | 2 +- public/422.html | 2 +- public/500.html | 2 +- spec/rails_helper.rb | 2 ++ spec/system/home_spec.rb | 16 ++++++++++++++ 16 files changed, 61 insertions(+), 9 deletions(-) create mode 100644 app/assets/stylesheets/layout/skip_to_main_content.scss create mode 100644 app/components/layout/skip_to_main_content_component.html.erb create mode 100644 app/components/layout/skip_to_main_content_component.rb diff --git a/app/assets/stylesheets/layout/skip_to_main_content.scss b/app/assets/stylesheets/layout/skip_to_main_content.scss new file mode 100644 index 000000000..dcfc3db78 --- /dev/null +++ b/app/assets/stylesheets/layout/skip_to_main_content.scss @@ -0,0 +1,22 @@ +.skip-to-main-content { + @include grid-column-gutter; + + a { + &:not(:focus) { + @include element-invisible; + width: 0 !important; + } + + &:focus { + @include body-colors; + + $outline-size: $focus-inner-width + $focus-middle-width + $focus-outer-width; + + padding: 0.4rem; + position: absolute; + $global-left: $outline-size; + top: $outline-size; + z-index: 1000; + } + } +} diff --git a/app/components/layout/skip_to_main_content_component.html.erb b/app/components/layout/skip_to_main_content_component.html.erb new file mode 100644 index 000000000..e22492693 --- /dev/null +++ b/app/components/layout/skip_to_main_content_component.html.erb @@ -0,0 +1,3 @@ +
+ <%= link_to t("layouts.skip_to_main_content"), "#main" %> +
diff --git a/app/components/layout/skip_to_main_content_component.rb b/app/components/layout/skip_to_main_content_component.rb new file mode 100644 index 000000000..2cd600df3 --- /dev/null +++ b/app/components/layout/skip_to_main_content_component.rb @@ -0,0 +1,2 @@ +class Layout::SkipToMainContentComponent < ApplicationComponent +end diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb index c84b71711..182fabd69 100644 --- a/app/views/layouts/admin.html.erb +++ b/app/views/layouts/admin.html.erb @@ -7,6 +7,7 @@ + <%= render Layout::SkipToMainContentComponent.new %> <%= render Layout::AdminHeaderComponent.new(current_user) %>