From 16c582f2824779a10f7738dad2c7eec1afed5719 Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Mon, 16 Mar 2020 12:54:00 +0100 Subject: [PATCH 01/12] Extract component for budget groups and headings --- .../budgets/groups_and_headings.scss | 29 ++++++++++++++++++ app/assets/stylesheets/participation.scss | 30 ------------------- .../groups_and_headings_component.html.erb | 21 +++++++++++++ .../budgets/groups_and_headings_component.rb | 16 ++++++++++ app/helpers/budgets_helper.rb | 7 ----- app/views/budgets/index.html.erb | 23 +------------- 6 files changed, 67 insertions(+), 59 deletions(-) create mode 100644 app/assets/stylesheets/budgets/groups_and_headings.scss create mode 100644 app/components/budgets/groups_and_headings_component.html.erb create mode 100644 app/components/budgets/groups_and_headings_component.rb diff --git a/app/assets/stylesheets/budgets/groups_and_headings.scss b/app/assets/stylesheets/budgets/groups_and_headings.scss new file mode 100644 index 000000000..44c36f108 --- /dev/null +++ b/app/assets/stylesheets/budgets/groups_and_headings.scss @@ -0,0 +1,29 @@ +.groups-and-headings { + + .heading { + border: 1px solid $border; + border-radius: rem-calc(3); + display: inline-block; + margin-bottom: $line-height / 2; + + a { + display: block; + padding: $line-height / 2; + + &:hover { + background: $highlight; + text-decoration: none; + } + } + + .heading-name { + padding: $line-height / 2; + } + + span { + color: $text; + display: block; + font-size: $small-font-size; + } + } +} diff --git a/app/assets/stylesheets/participation.scss b/app/assets/stylesheets/participation.scss index 764d4592c..579a8a08e 100644 --- a/app/assets/stylesheets/participation.scss +++ b/app/assets/stylesheets/participation.scss @@ -1222,36 +1222,6 @@ color: $brand; } -.groups-and-headings { - - .heading { - border: 1px solid $border; - border-radius: rem-calc(3); - display: inline-block; - margin-bottom: $line-height / 2; - - a { - display: block; - padding: $line-height / 2; - - &:hover { - background: $highlight; - text-decoration: none; - } - } - - .heading-name { - padding: $line-height / 2; - } - - span { - color: $text; - display: block; - font-size: $small-font-size; - } - } -} - .progress-votes { position: relative; diff --git a/app/components/budgets/groups_and_headings_component.html.erb b/app/components/budgets/groups_and_headings_component.html.erb new file mode 100644 index 000000000..dfb997032 --- /dev/null +++ b/app/components/budgets/groups_and_headings_component.html.erb @@ -0,0 +1,21 @@ +
+ <% budget.groups.each do |group| %> +

<%= group.name %>

+ + <% end %> +
diff --git a/app/components/budgets/groups_and_headings_component.rb b/app/components/budgets/groups_and_headings_component.rb new file mode 100644 index 000000000..87d0b247d --- /dev/null +++ b/app/components/budgets/groups_and_headings_component.rb @@ -0,0 +1,16 @@ +class Budgets::GroupsAndHeadingsComponent < ApplicationComponent + attr_reader :budget + + def initialize(budget) + @budget = budget + end + + private + + def heading_name_and_price_html(heading) + tag.div do + concat(heading.name + " ") + concat(tag.span(budget.formatted_heading_price(heading))) + end + end +end diff --git a/app/helpers/budgets_helper.rb b/app/helpers/budgets_helper.rb index fcf937fa0..079599205 100644 --- a/app/helpers/budgets_helper.rb +++ b/app/helpers/budgets_helper.rb @@ -5,13 +5,6 @@ module BudgetsHelper end end - def heading_name_and_price_html(heading, budget) - tag.div do - concat(heading.name + " ") - concat(tag.span(budget.formatted_heading_price(heading))) - end - end - def csv_params csv_params = params.clone.merge(format: :csv) csv_params = csv_params.to_unsafe_h.map { |k, v| [k.to_sym, v] }.to_h diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index bdd3d3504..6d81d4a80 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -27,28 +27,7 @@
- -
- <% current_budget.groups.each do |group| %> -

<%= group.name %>

-
    - <% group.headings.sort_by_name.each do |heading| %> -
  • - <% unless current_budget.informing? || current_budget.finished? %> - <%= link_to budget_investments_path(current_budget.id, - heading_id: heading.id) do %> - <%= heading_name_and_price_html(heading, current_budget) %> - <% end %> - <% else %> -
    - <%= heading_name_and_price_html(heading, current_budget) %> -
    - <% end %> -
    • - <% end %> -
- <% end %> -
+ <%= render Budgets::GroupsAndHeadingsComponent.new(current_budget) %> <% unless current_budget.informing? %>
From f1b707f5490b1d8da376a1dd8e7f6838b704d335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 11 Mar 2021 19:57:37 +0100 Subject: [PATCH 02/12] Simplify styling headings with the same height Using flex we don't have to rely on JavaScript to equalize the item. Besides, we've had problems with JavaScript in the past. We're also adjusting the width of the elements; previously, even though we defined a width of 16.666% for each element, only five elements would be on the same row. It happenend because these elements were styled with inline-block and the generated HTML contained a newline character between
  • tags, meaning a space character was introduced between elements. The width of the mentioned space character wasn't being taken into account when calculating the width. Using flex, there's no space character between items and we have to define the margin between them. We're taking this margin into account when calculating the width. --- .../budgets/groups_and_headings.scss | 19 ++++++++++++++++++- .../groups_and_headings_component.html.erb | 4 ++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/budgets/groups_and_headings.scss b/app/assets/stylesheets/budgets/groups_and_headings.scss index 44c36f108..1be85a2be 100644 --- a/app/assets/stylesheets/budgets/groups_and_headings.scss +++ b/app/assets/stylesheets/budgets/groups_and_headings.scss @@ -1,10 +1,27 @@ .groups-and-headings { + $spacing: rem-calc(4); + + .headings-list { + display: flex; + flex-wrap: wrap; + list-style: none; + margin-left: -$spacing; + } .heading { border: 1px solid $border; border-radius: rem-calc(3); - display: inline-block; margin-bottom: $line-height / 2; + margin-left: $spacing; + width: 100%; + + @include breakpoint(medium) { + width: calc(100% / 3 - #{$spacing}); + } + + @include breakpoint(large) { + width: calc(100% / 6 - #{$spacing}); + } a { display: block; diff --git a/app/components/budgets/groups_and_headings_component.html.erb b/app/components/budgets/groups_and_headings_component.html.erb index dfb997032..6c88b9850 100644 --- a/app/components/budgets/groups_and_headings_component.html.erb +++ b/app/components/budgets/groups_and_headings_component.html.erb @@ -1,9 +1,9 @@
    <% budget.groups.each do |group| %>

    <%= group.name %>

    -
      +
        <% group.headings.sort_by_name.each do |heading| %> -
      • +
      • <% unless budget.informing? || budget.finished? %> <%= link_to budget_investments_path(budget.id, heading_id: heading.id) do %> From 4c23f639becf0af00a4b878960cf7c5572c3e2b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 15 Mar 2021 23:59:18 +0100 Subject: [PATCH 03/12] Use heading name as link text Using the name instead of using the name and the price is IMHO more consistent with the rest of the application, particularly for screen reader users. Writing texts clicking those links is also easier. I think the main reason why we used the price as part of the link was so the clickable area was bigger. We can accomplish the same result with CSS. --- .../budgets/groups_and_headings.scss | 35 ++++++++++++++----- .../groups_and_headings_component.html.erb | 17 ++++----- .../budgets/groups_and_headings_component.rb | 7 ++-- spec/system/budgets/budgets_spec.rb | 4 +-- spec/system/budgets/investments_spec.rb | 2 +- 5 files changed, 38 insertions(+), 27 deletions(-) diff --git a/app/assets/stylesheets/budgets/groups_and_headings.scss b/app/assets/stylesheets/budgets/groups_and_headings.scss index 1be85a2be..6f6b4a2d9 100644 --- a/app/assets/stylesheets/budgets/groups_and_headings.scss +++ b/app/assets/stylesheets/budgets/groups_and_headings.scss @@ -13,6 +13,8 @@ border-radius: rem-calc(3); margin-bottom: $line-height / 2; margin-left: $spacing; + padding: $line-height / 2; + position: relative; width: 100%; @include breakpoint(medium) { @@ -23,22 +25,37 @@ width: calc(100% / 6 - #{$spacing}); } - a { - display: block; - padding: $line-height / 2; + &:focus-within { + outline: $outline-focus; - &:hover { - background: $highlight; - text-decoration: none; + a:focus { + outline: none; } } - .heading-name { - padding: $line-height / 2; + a { + + &::after, + &::before { + bottom: 0; + content: ""; + left: 0; + position: absolute; + right: 0; + top: 0; + } + + &:hover { + text-decoration: none; + } + + &:hover::before { + background: $highlight; + z-index: -1; + } } span { - color: $text; display: block; font-size: $small-font-size; } diff --git a/app/components/budgets/groups_and_headings_component.html.erb b/app/components/budgets/groups_and_headings_component.html.erb index 6c88b9850..6f44960dd 100644 --- a/app/components/budgets/groups_and_headings_component.html.erb +++ b/app/components/budgets/groups_and_headings_component.html.erb @@ -4,16 +4,13 @@
          <% group.headings.sort_by_name.each do |heading| %>
        • - <% unless budget.informing? || budget.finished? %> - <%= link_to budget_investments_path(budget.id, - heading_id: heading.id) do %> - <%= heading_name_and_price_html(heading) %> - <% end %> - <% else %> -
          - <%= heading_name_and_price_html(heading) %> -
          - <% end %> + <%= link_to_unless( + (budget.informing? || budget.finished?), + heading.name, + budget_investments_path(budget.id, heading_id: heading.id) + ) %> + + <%= price(heading) %>
          • <% end %>
        diff --git a/app/components/budgets/groups_and_headings_component.rb b/app/components/budgets/groups_and_headings_component.rb index 87d0b247d..b16357e4b 100644 --- a/app/components/budgets/groups_and_headings_component.rb +++ b/app/components/budgets/groups_and_headings_component.rb @@ -7,10 +7,7 @@ class Budgets::GroupsAndHeadingsComponent < ApplicationComponent private - def heading_name_and_price_html(heading) - tag.div do - concat(heading.name + " ") - concat(tag.span(budget.formatted_heading_price(heading))) - end + def price(heading) + tag.span(budget.formatted_heading_price(heading)) end end diff --git a/spec/system/budgets/budgets_spec.rb b/spec/system/budgets/budgets_spec.rb index 022ad0693..15e142afd 100644 --- a/spec/system/budgets/budgets_spec.rb +++ b/spec/system/budgets/budgets_spec.rb @@ -118,7 +118,7 @@ describe "Budgets" do visit budgets_path within("#budget_info") do - expect(page).not_to have_link "#{heading.name} €1,000,000" + expect(page).not_to have_link heading.name expect(page).to have_content "#{heading.name} €1,000,000" expect(page).not_to have_link("List of all investment projects") @@ -136,7 +136,7 @@ describe "Budgets" do visit budgets_path within("#budget_info") do - expect(page).not_to have_link "#{heading.name} €1,000,000" + expect(page).not_to have_link heading.name expect(page).to have_content "#{heading.name} €1,000,000" expect(page).to have_css("div.map") diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 5640def42..5cdfb9b8d 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -1230,7 +1230,7 @@ describe "Budget Investments" do first(:link, "Participatory budgeting").click - click_link "More hospitals €666,666" + click_link "More hospitals" within("#budget_investment_#{investment1.id}") do expect(page).to have_content investment1.title From bbb4e04c7c959cfa4ea01d4393ef4fc9ebce0956 Mon Sep 17 00:00:00 2001 From: decabeza Date: Tue, 21 Apr 2020 12:38:29 +0200 Subject: [PATCH 04/12] Extract budget investment's info to a component --- .../investments/info_component.html.erb | 24 +++++++++++++++++++ .../budgets/investments/info_component.rb | 7 ++++++ .../budgets/investments/_investment.html.erb | 24 +------------------ 3 files changed, 32 insertions(+), 23 deletions(-) create mode 100644 app/components/budgets/investments/info_component.html.erb create mode 100644 app/components/budgets/investments/info_component.rb diff --git a/app/components/budgets/investments/info_component.html.erb b/app/components/budgets/investments/info_component.html.erb new file mode 100644 index 000000000..91e757cd0 --- /dev/null +++ b/app/components/budgets/investments/info_component.html.erb @@ -0,0 +1,24 @@ +

        + <%= l investment.created_at.to_date %> + + <% if investment.author.hidden? || investment.author.erased? %> +  •  + + <%= t("budgets.investments.show.author_deleted") %> + + <% else %> +  •  + + <%= investment.author.name %> + + <% if investment.author.official? %> +  •  + + <%= investment.author.official_position %> + + <% end %> + <% end %> + +  •  + <%= investment.heading.name %> +

        diff --git a/app/components/budgets/investments/info_component.rb b/app/components/budgets/investments/info_component.rb new file mode 100644 index 000000000..b491c4349 --- /dev/null +++ b/app/components/budgets/investments/info_component.rb @@ -0,0 +1,7 @@ +class Budgets::Investments::InfoComponent < ApplicationComponent + attr_reader :investment + + def initialize(investment) + @investment = investment + end +end diff --git a/app/views/budgets/investments/_investment.html.erb b/app/views/budgets/investments/_investment.html.erb index 8d1464a9f..ef96f1f3f 100644 --- a/app/views/budgets/investments/_investment.html.erb +++ b/app/views/budgets/investments/_investment.html.erb @@ -21,30 +21,8 @@ <% cache [locale_and_user_status(investment), "index", investment, investment.author] do %>

        <%= link_to investment.title, namespaced_budget_investment_path(investment) %>

        -

        - <%= l investment.created_at.to_date %> + <%= render Budgets::Investments::InfoComponent.new(investment) %> - <% if investment.author.hidden? || investment.author.erased? %> -  •  - - <%= t("budgets.investments.show.author_deleted") %> - - <% else %> -  •  - - <%= investment.author.name %> - - <% if investment.author.official? %> -  •  - - <%= investment.author.official_position %> - - <% end %> - <% end %> - -  •  - <%= investment.heading.name %> -

        <%= wysiwyg(investment.description) %>
        From f936c992e240dc5ce84062595cc7c842ceb24ddd Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Thu, 30 Apr 2020 10:53:11 +0700 Subject: [PATCH 05/12] Extract footer for budgets to a component --- app/components/budgets/footer_component.html.erb | 10 ++++++++++ app/components/budgets/footer_component.rb | 2 ++ app/views/budgets/index.html.erb | 11 +---------- 3 files changed, 13 insertions(+), 10 deletions(-) create mode 100644 app/components/budgets/footer_component.html.erb create mode 100644 app/components/budgets/footer_component.rb diff --git a/app/components/budgets/footer_component.html.erb b/app/components/budgets/footer_component.html.erb new file mode 100644 index 000000000..b5e0a4342 --- /dev/null +++ b/app/components/budgets/footer_component.html.erb @@ -0,0 +1,10 @@ +
        +
        +
        +

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

        +

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

        +
        +
        +
        diff --git a/app/components/budgets/footer_component.rb b/app/components/budgets/footer_component.rb new file mode 100644 index 000000000..ba9360c74 --- /dev/null +++ b/app/components/budgets/footer_component.rb @@ -0,0 +1,2 @@ +class Budgets::FooterComponent < ApplicationComponent +end diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index 6d81d4a80..68a798304 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -60,13 +60,4 @@
        <% end %> -
        -
        -
        -

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

        -

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

        -
        -
        -
        +<%= render Budgets::FooterComponent.new %> From 6cfb862553b69d0cb94477189388880bbfe84838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 15 Mar 2021 13:55:02 +0100 Subject: [PATCH 06/12] Remove unneeded turbolinks: false link parameter It was added because a test failed without turbolinks. However, writing the test so it doesn't update the database at the same time the browser is doing a request also solves the problem and makes the test more robust. --- app/views/budgets/groups/show.html.erb | 3 +- app/views/budgets/show.html.erb | 3 +- spec/system/budgets/ballots_spec.rb | 38 ++++++++++++++++++-------- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/app/views/budgets/groups/show.html.erb b/app/views/budgets/groups/show.html.erb index d8741f6eb..81a797797 100644 --- a/app/views/budgets/groups/show.html.erb +++ b/app/views/budgets/groups/show.html.erb @@ -35,8 +35,7 @@ class="<%= css_for_ballot_heading(heading) %>"> <%= link_to heading.name, budget_investments_path(heading_id: heading.id, - filter: @current_filter), - data: { turbolinks: false } %>
        + filter: @current_filter) %>
        <% end %>
    diff --git a/app/views/budgets/show.html.erb b/app/views/budgets/show.html.erb index f7e6617e0..73f444bec 100644 --- a/app/views/budgets/show.html.erb +++ b/app/views/budgets/show.html.erb @@ -35,8 +35,7 @@ <%= link_to group.name, budget_investments_path(@budget, heading_id: group.headings.first.id, - filter: @current_filter), - data: { turbolinks: false } %> + filter: @current_filter) %> <% else %> <%= link_to group.name, budget_group_path(@budget, group, diff --git a/spec/system/budgets/ballots_spec.rb b/spec/system/budgets/ballots_spec.rb index 3db1a4cdf..a6c737770 100644 --- a/spec/system/budgets/ballots_spec.rb +++ b/spec/system/budgets/ballots_spec.rb @@ -1,4 +1,5 @@ require "rails_helper" +require "sessions_helper" describe "Ballots" do let(:user) { create(:user, :level_two) } @@ -679,23 +680,36 @@ describe "Ballots" do scenario "Edge case voting a non-elegible investment", :js do investment1 = create(:budget_investment, :selected, heading: new_york, price: 10000) - login_as(user) - visit budget_path(budget) - click_link "States" - click_link "New York" + in_browser(:user) do + login_as user + visit budget_path(budget) + click_link "States" + click_link "New York" - new_york.update!(price: 10) + expect(page).to have_css(".in-favor a") + end - within("#budget_investment_#{investment1.id}") do - find(".in-favor a").click + in_browser(:admin) do + login_as create(:administrator).user + visit edit_admin_budget_group_heading_path(budget, states, new_york) + fill_in "Amount", with: 10 + click_button "Save heading" - expect(page).not_to have_content "Remove" - expect(page).not_to have_selector(".participation-not-allowed") + expect(page).to have_content "Heading updated successfully" + end - hover_over_ballot + in_browser(:user) do + within("#budget_investment_#{investment1.id}") do + find(".in-favor a").click - expect(page).to have_selector(".participation-not-allowed") - expect(page).to have_selector(".in-favor a", obscured: true) + expect(page).not_to have_content "Remove" + expect(page).not_to have_selector(".participation-not-allowed") + + hover_over_ballot + + expect(page).to have_selector(".participation-not-allowed") + expect(page).to have_selector(".in-favor a", obscured: true) + end end end From 7e3dd47d5aaebd10a744b3f069e882bd677e8748 Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Thu, 30 Apr 2020 10:55:55 +0700 Subject: [PATCH 07/12] Unify budget landing pages There was a big difference between the current budget and a specific budget landing page. This didn't really make too much sense. Also, it was not possible to know how a draft participatory budget will look before it was published. By unifying those two views now they will look quite similar and it will be possible for administrators to preview any draft budget and to know how the budget will look like before actually publishing it. --- .../budgets/budget_component.html.erb | 32 +++++ app/components/budgets/budget_component.rb | 22 ++++ app/controllers/budgets_controller.rb | 9 +- app/helpers/budgets_helper.rb | 12 -- app/views/budgets/index.html.erb | 40 +------ app/views/budgets/results/show.html.erb | 6 +- app/views/budgets/show.html.erb | 76 +----------- config/locales/en/budgets.yml | 5 - config/locales/es/budgets.yml | 5 - spec/system/budgets/ballots_spec.rb | 96 ++++----------- spec/system/budgets/budgets_spec.rb | 45 ++------ spec/system/budgets/investments_spec.rb | 109 +----------------- spec/system/tags/budget_investments_spec.rb | 6 +- 13 files changed, 107 insertions(+), 356 deletions(-) create mode 100644 app/components/budgets/budget_component.html.erb create mode 100644 app/components/budgets/budget_component.rb diff --git a/app/components/budgets/budget_component.html.erb b/app/components/budgets/budget_component.html.erb new file mode 100644 index 000000000..18f84f767 --- /dev/null +++ b/app/components/budgets/budget_component.html.erb @@ -0,0 +1,32 @@ +
    +
    +
    +

    <%= budget.name %>

    +
    + <%= auto_link_already_sanitized_html wysiwyg(budget.description) %> +
    + +

    + <%= link_to t("budgets.index.section_header.help"), "#section_help" %> +

    +
    +
    +
    + +<%= render Budgets::SubheaderComponent.new(budget) %> +<%= render Budgets::PhasesComponent.new(budget) %> + +
    +
    +
    + <%= render Budgets::GroupsAndHeadingsComponent.new(budget) %> + + <% unless budget.informing? %> +
    +

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

    + <%= render_map(nil, "budgets", false, nil, coordinates) %> +
    + <% end %> +
    +
    +
    diff --git a/app/components/budgets/budget_component.rb b/app/components/budgets/budget_component.rb new file mode 100644 index 000000000..83536cfd6 --- /dev/null +++ b/app/components/budgets/budget_component.rb @@ -0,0 +1,22 @@ +class Budgets::BudgetComponent < ApplicationComponent + delegate :wysiwyg, :auto_link_already_sanitized_html, :render_map, to: :helpers + attr_reader :budget + + def initialize(budget) + @budget = budget + end + + private + + def coordinates + return unless budget.present? + + if budget.publishing_prices_or_later? && budget.investments.selected.any? + investments = budget.investments.selected + else + investments = budget.investments + end + + MapLocation.where(investment_id: investments).map(&:json_data) + end +end diff --git a/app/controllers/budgets_controller.rb b/app/controllers/budgets_controller.rb index b7ee1da8a..fd5ecbbd5 100644 --- a/app/controllers/budgets_controller.rb +++ b/app/controllers/budgets_controller.rb @@ -1,13 +1,11 @@ class BudgetsController < ApplicationController include FeatureFlags include BudgetsHelper - include InvestmentFilters feature_flag :budgets before_action :load_budget, only: :show + before_action :load_current_budget, only: :index load_and_authorize_resource - before_action :set_default_investment_filter, only: :show - has_filters investment_filters, only: :show respond_to :html, :js @@ -17,7 +15,6 @@ class BudgetsController < ApplicationController def index @finished_budgets = @budgets.finished.order(created_at: :desc) - @budgets_coordinates = current_budget_map_locations end private @@ -25,4 +22,8 @@ class BudgetsController < ApplicationController def load_budget @budget = Budget.find_by_slug_or_id! params[:id] end + + def load_current_budget + @budget = current_budget + end end diff --git a/app/helpers/budgets_helper.rb b/app/helpers/budgets_helper.rb index 079599205..6d5751302 100644 --- a/app/helpers/budgets_helper.rb +++ b/app/helpers/budgets_helper.rb @@ -56,18 +56,6 @@ module BudgetsHelper budget.published? || current_user&.administrator? end - def current_budget_map_locations - return unless current_budget.present? - - if current_budget.publishing_prices_or_later? && current_budget.investments.selected.any? - investments = current_budget.investments.selected - else - investments = current_budget.investments - end - - MapLocation.where(investment_id: investments).map(&:json_data) - end - def display_calculate_winners_button?(budget) budget.balloting_or_later? end diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index 68a798304..53f65ea9f 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -6,42 +6,12 @@ <%= render "shared/canonical", href: budgets_url %> <% end %> -<% if current_budget.present? %> -
    -
    -
    -

    <%= current_budget.name %>

    -
    - <%= auto_link_already_sanitized_html wysiwyg(current_budget.description) %> -
    -

    - <%= link_to t("budgets.index.section_header.help"), "#section_help" %> -

    -
    -
    -
    +<% if @budget.present? %> + <%= render Budgets::BudgetComponent.new(@budget) %> - <%= render Budgets::SubheaderComponent.new(current_budget) %> - <%= render Budgets::PhasesComponent.new(current_budget) %> - -
    -
    -
    - <%= render Budgets::GroupsAndHeadingsComponent.new(current_budget) %> - - <% unless current_budget.informing? %> -
    -

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

    - <%= render_map(nil, "budgets", false, nil, @budgets_coordinates) %> -
    - <% end %> -
    -
    - - <% if @finished_budgets.present? %> - <%= render "finished", budgets: @finished_budgets %> - <% end %> -
    + <% if @finished_budgets.present? %> + <%= render "finished", budgets: @finished_budgets %> + <% end %> <% else %>
    diff --git a/app/views/budgets/results/show.html.erb b/app/views/budgets/results/show.html.erb index 420683ffc..605309c1b 100644 --- a/app/views/budgets/results/show.html.erb +++ b/app/views/budgets/results/show.html.erb @@ -61,13 +61,13 @@ <% end %>

    - <%= link_to budget_path(@budget) do %> + <%= link_to budget_investments_path(@budget, heading_id: @heading) do %> <%= t("budgets.results.investment_proyects") %> <% end %>
    - <%= link_to budget_path(@budget, filter: "unfeasible") do %> + <%= link_to budget_investments_path(@budget, heading_id: @heading, filter: "unfeasible") do %> <%= t("budgets.results.unfeasible_investment_proyects") %> <% end %>
    - <%= link_to budget_path(@budget, filter: "unselected") do %> + <%= link_to budget_investments_path(@budget, heading_id: @heading, filter: "unselected") do %> <%= t("budgets.results.not_selected_investment_proyects") %> <% end %>

    diff --git a/app/views/budgets/show.html.erb b/app/views/budgets/show.html.erb index 73f444bec..36ce55708 100644 --- a/app/views/budgets/show.html.erb +++ b/app/views/budgets/show.html.erb @@ -1,75 +1,9 @@ +<%= render Shared::BannerComponent.new("budgets") %> +<% provide :title do %><%= @budget.name %><% end %> + <% content_for :canonical do %> <%= render "shared/canonical", href: budget_url(@budget, filter: @current_filter) %> <% end %> -
    -
    -
    - <%= back_link_to budgets_path %> - -

    <%= @budget.name %>

    - - <%= auto_link_already_sanitized_html wysiwyg(@budget.description) %> -
    -
    -
    - -<%= render Budgets::SubheaderComponent.new(@budget) %> - -
    -
    - <% if @current_filter == "unfeasible" %> -

    <%= t("budgets.show.unfeasible_title") %>

    - <% elsif @current_filter == "unselected" %> -

    <%= t("budgets.show.unselected_title") %>

    - <% end %> - - - - - - <% @budget.groups.each do |group| %> - - - - <% end %> - -
    <%= t("budgets.show.group") %>
    - <% if group.single_heading_group? %> - <%= link_to group.name, - budget_investments_path(@budget, - heading_id: group.headings.first.id, - filter: @current_filter) %> - <% else %> - <%= link_to group.name, - budget_group_path(@budget, group, - filter: @current_filter) %> - <% end %> -
    -
    -
    -
    - -<% if @budget.balloting_or_later? %> - <% unless @current_filter == "unfeasible" %> -
    -
    - - <%= link_to t("budgets.show.unfeasible"), - budget_path(@budget, filter: "unfeasible") %> - -
    -
    - <% end %> - - <% unless @current_filter == "unselected" %> -
    -
    - - <%= link_to t("budgets.show.unselected"), - budget_path(@budget, filter: "unselected") %> - -
    -
    - <% end %> -<% end %> +<%= render Budgets::BudgetComponent.new(@budget) %> +<%= render Budgets::FooterComponent.new %> diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index 7931cc773..1ff9bce72 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -178,11 +178,6 @@ en: one: "You have selected 1 project out of %{limit}" other: "You have selected %{count} projects out of %{limit}" show: - group: Group - unfeasible_title: Unfeasible investments - unfeasible: See unfeasible investments - unselected_title: Investments not selected for balloting phase - unselected: See investments not selected for balloting phase see_results: See results results: link: Results diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index 98eda05f9..c05eaae6b 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -178,11 +178,6 @@ es: one: "Has seleccionado 1 proyecto de %{limit}" other: "Has seleccionado %{count} proyectos de %{limit}" show: - group: Grupo - unfeasible_title: Proyectos de gasto inviables - unfeasible: Ver los proyectos inviables - unselected_title: Proyectos no seleccionados para la votación final - unselected: Ver los proyectos no seleccionados para la votación final see_results: Ver resultados results: link: Resultados diff --git a/spec/system/budgets/ballots_spec.rb b/spec/system/budgets/ballots_spec.rb index a6c737770..2d9a6df07 100644 --- a/spec/system/budgets/ballots_spec.rb +++ b/spec/system/budgets/ballots_spec.rb @@ -62,13 +62,6 @@ describe "Ballots" do let!(:districts) { create(:budget_group, budget: budget, name: "Districts") } context "Group and Heading Navigation" do - scenario "Groups" do - visit budget_path(budget) - - expect(page).to have_link "City" - expect(page).to have_link "Districts" - end - scenario "Headings" do create(:budget_heading, group: city, name: "Investments Type1") create(:budget_heading, group: city, name: "Investments Type2") @@ -76,16 +69,13 @@ describe "Ballots" do create(:budget_heading, group: districts, name: "District 2") visit budget_path(budget) - click_link "City" - expect(page).to have_link "Investments Type1" - expect(page).to have_link "Investments Type2" - - visit budget_path(budget) - click_link "Districts" - - expect(page).to have_link "District 1" - expect(page).to have_link "District 2" + within("#groups_and_headings") do + expect(page).to have_link "Investments Type1" + expect(page).to have_link "Investments Type2" + expect(page).to have_link "District 1" + expect(page).to have_link "District 2" + end end scenario "Investments" do @@ -106,7 +96,6 @@ describe "Ballots" do end visit budget_path(budget) - click_link "City" click_link "Above the city" expect(page).to have_css(".budget-investment", count: 2) @@ -114,8 +103,6 @@ describe "Ballots" do expect(page).to have_content "Observatory" visit budget_path(budget) - - click_link "Districts" click_link "District 1" expect(page).to have_css(".budget-investment", count: 2) @@ -123,22 +110,11 @@ describe "Ballots" do expect(page).to have_content "Zero-emission zone" visit budget_path(budget) - click_link "Districts" click_link "District 2" expect(page).to have_css(".budget-investment", count: 1) expect(page).to have_content "Climbing wall" end - - scenario "Redirect to first heading if there is only one" do - city_heading = create(:budget_heading, group: city, name: "City") - city_investment = create(:budget_investment, :selected, heading: city_heading) - - visit budget_path(budget) - click_link "City" - - expect(page).to have_content city_investment.title - end end context "Adding and Removing Investments" do @@ -146,10 +122,7 @@ describe "Ballots" do create(:budget_investment, :selected, heading: new_york, price: 10000, title: "Bring back King Kong") create(:budget_investment, :selected, heading: new_york, price: 20000, title: "Paint cabs black") - visit budget_path(budget) - click_link "States" - click_link "New York" - + visit budget_investments_path(budget, heading_id: new_york) add_to_ballot("Bring back King Kong") expect(page).to have_css("#amount-spent", text: "€10,000") @@ -176,9 +149,7 @@ describe "Ballots" do scenario "Removing a investment", :js do investment = create(:budget_investment, :selected, heading: new_york, price: 10000, balloters: [user]) - visit budget_path(budget) - click_link "States" - click_link "New York" + visit budget_investments_path(budget, heading_id: new_york) expect(page).to have_content investment.title expect(page).to have_css("#amount-spent", text: "€10,000") @@ -207,9 +178,7 @@ describe "Ballots" do scenario "the Map shoud be visible before and after", :js do create(:budget_investment, :selected, heading: new_york, price: 10000, title: "More bridges") - visit budget_path(budget) - click_link "States" - click_link "New York" + visit budget_investments_path(budget, heading_id: new_york) within("#sidebar") do expect(page).to have_content "OpenStreetMap" @@ -244,8 +213,7 @@ describe "Ballots" do create(:budget_investment, :selected, heading: district_heading1, price: 20000, title: "Average") create(:budget_investment, :selected, heading: district_heading2, price: 30000, title: "Expensive") - visit budget_path(budget) - click_link "City" + visit budget_investments_path(budget, heading_id: city_heading) add_to_ballot("Cheap") @@ -257,9 +225,7 @@ describe "Ballots" do expect(page).to have_content "€10,000" end - visit budget_path(budget) - click_link "Districts" - click_link "District 1" + visit budget_investments_path(budget, heading_id: district_heading1) expect(page).to have_css("#amount-spent", text: "€0") expect(page).to have_css("#amount-spent", text: "€1,000,000") @@ -277,8 +243,7 @@ describe "Ballots" do expect(page).not_to have_content "€10,000" end - visit budget_path(budget) - click_link "City" + visit budget_investments_path(budget, heading_id: city_heading) expect(page).to have_css("#amount-spent", text: "€10,000") expect(page).to have_css("#amount-available", text: "€9,990,000") @@ -291,9 +256,7 @@ describe "Ballots" do expect(page).not_to have_content "€20,000" end - visit budget_path(budget) - click_link "Districts" - click_link "District 2" + visit budget_investments_path(budget, heading_id: district_heading2) expect(page).to have_content("You have active votes in another heading: District 1") end @@ -318,14 +281,11 @@ describe "Ballots" do scenario "Select my heading", :js do create(:budget_investment, :selected, heading: california, title: "Green beach") - visit budget_path(budget) - click_link "States" - click_link "California" + visit budget_investments_path(budget, heading_id: california) add_to_ballot("Green beach") - visit budget_path(budget) - click_link "States" + visit budget_group_path(budget, states) expect(page).to have_content "California" expect(page).to have_css("#budget_heading_#{california.id}.is-active") @@ -346,8 +306,8 @@ describe "Ballots" do add_to_ballot("Avengers Tower") - visit budget_path(budget) - click_link "States" + visit budget_group_path(budget, states) + expect(page).to have_css("#budget_heading_#{new_york.id}.is-active") expect(page).not_to have_css("#budget_heading_#{california.id}.is-active") end @@ -364,16 +324,6 @@ describe "Ballots" do end context "Showing the ballot" do - scenario "Do not display heading name if there is only one heading in the group (example: group city)" do - group = create(:budget_group, budget: budget) - heading = create(:budget_heading, group: group) - visit budget_path(budget) - click_link group.name - # No need to click on the heading name - expect(page).to have_content("Investment projects with scope: #{heading.name}") - expect(page).to have_current_path(budget_investments_path(budget), ignore_query: true) - end - scenario "Displaying the correct group, heading, count & amount" do group1 = create(:budget_group, budget: budget) group2 = create(:budget_group, budget: budget) @@ -543,9 +493,7 @@ describe "Ballots" do investment = create(:budget_investment, heading: new_york, title: "WTF asdfasfd") login_as(user) - visit budget_path(budget) - click_link states.name - click_link new_york.name + visit budget_investments_path(budget, heading_id: new_york) expect(page).not_to have_css("#budget_investment_#{investment.id}") end @@ -554,9 +502,7 @@ describe "Ballots" do investment = create(:budget_investment, :undecided, heading: new_york) login_as(user) - visit budget_path(budget) - click_link states.name - click_link new_york.name + visit budget_investments_path(budget, heading_id: new_york) within("#budget-investments") do expect(page).not_to have_css("div.ballot") @@ -682,9 +628,7 @@ describe "Ballots" do in_browser(:user) do login_as user - visit budget_path(budget) - click_link "States" - click_link "New York" + visit budget_investments_path(budget, heading_id: new_york) expect(page).to have_css(".in-favor a") end diff --git a/spec/system/budgets/budgets_spec.rb b/spec/system/budgets/budgets_spec.rb index 15e142afd..922449988 100644 --- a/spec/system/budgets/budgets_spec.rb +++ b/spec/system/budgets/budgets_spec.rb @@ -307,7 +307,7 @@ describe "Budgets" do map_locations << { longitude: 40.123456789, latitude: "********" } map_locations << { longitude: "**********", latitude: 3.12345678 } - budget_map_locations = map_locations.map do |map_location| + coordinates = map_locations.map do |map_location| { lat: map_location[:latitude], long: map_location[:longitude], @@ -317,8 +317,7 @@ describe "Budgets" do } end - allow_any_instance_of(BudgetsHelper). - to receive(:current_budget_map_locations).and_return(budget_map_locations) + allow_any_instance_of(Budgets::BudgetComponent).to receive(:coordinates).and_return(coordinates) visit budgets_path @@ -329,61 +328,32 @@ describe "Budgets" do end context "Show" do - scenario "List all groups" do - create(:budget_group, budget: budget) - create(:budget_group, budget: budget) - - visit budget_path(budget) - - budget.groups.each { |group| expect(page).to have_link(group.name) } - end - scenario "Links to unfeasible and selected if balloting or later" do budget = create(:budget, :selecting) group = create(:budget_group, budget: budget) - visit budget_path(budget) - - expect(page).not_to have_link "See unfeasible investments" - expect(page).not_to have_link "See investments not selected for balloting phase" - - click_link group.name + visit budget_group_path(budget, group) expect(page).not_to have_link "See unfeasible investments" expect(page).not_to have_link "See investments not selected for balloting phase" budget.update!(phase: :publishing_prices) - visit budget_path(budget) - - expect(page).not_to have_link "See unfeasible investments" - expect(page).not_to have_link "See investments not selected for balloting phase" - - click_link group.name + visit budget_group_path(budget, group) expect(page).not_to have_link "See unfeasible investments" expect(page).not_to have_link "See investments not selected for balloting phase" budget.update!(phase: :balloting) - visit budget_path(budget) - - expect(page).to have_link "See unfeasible investments" - expect(page).to have_link "See investments not selected for balloting phase" - - click_link group.name + visit budget_group_path(budget, group) expect(page).to have_link "See unfeasible investments" expect(page).to have_link "See investments not selected for balloting phase" budget.update!(phase: :finished) - visit budget_path(budget) - - expect(page).to have_link "See unfeasible investments" - expect(page).to have_link "See investments not selected for balloting phase" - - click_link group.name + visit budget_group_path(budget, group) expect(page).to have_link "See unfeasible investments" expect(page).to have_link "See investments not selected for balloting phase" @@ -399,8 +369,7 @@ describe "Budgets" do heading3 = create(:budget_heading, group: group2, name: "Brooklyn") heading4 = create(:budget_heading, group: group2, name: "Queens") - visit budget_path(budget) - click_link "New York" + visit budget_group_path(budget, group1) expect(page).to have_css("#budget_heading_#{heading1.id}") expect(page).to have_css("#budget_heading_#{heading2.id}") diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 5cdfb9b8d..08cdad36f 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -78,7 +78,7 @@ describe "Budget Investments" do unfeasible_investment = create(:budget_investment, :unfeasible, heading: heading) visit budget_path(budget) - click_link "Health" + click_link "More hospitals" expect(page).to have_selector("#budget-investments .budget-investment", count: 3) investments.each do |investment| @@ -95,11 +95,9 @@ describe "Budget Investments" do create(:budget_investment, heading: heading), create(:budget_investment, heading: heading)] - visit budget_path(budget) - click_link "Health" + visit budget_investments_path(budget, heading_id: heading) click_button "View mode" - click_link "List" investments.each do |investment| @@ -110,7 +108,6 @@ describe "Budget Investments" do end click_button "View mode" - click_link "Cards" investments.each do |investment| @@ -238,47 +235,6 @@ describe "Budget Investments" do end end - scenario "by unfeasibilty link for group with one heading" do - budget.update!(phase: :balloting) - group = create(:budget_group, name: "All City", budget: budget) - heading = create(:budget_heading, name: "Madrid", group: group) - - visit budget_path(budget) - click_link "See unfeasible investments" - - click_link "All City" - - expected_path = budget_investments_path(budget, heading_id: heading.id, filter: "unfeasible") - expect(page).to have_current_path(expected_path) - end - - scenario "by unfeasibilty link for group with many headings" do - budget.update!(phase: :balloting) - group = create(:budget_group, name: "Districts", budget: budget) - - barajas = create(:budget_heading, name: "Barajas", group: group) - carabanchel = create(:budget_heading, name: "Carabanchel", group: group) - - create(:budget_investment, :feasible, heading: barajas, title: "Terminal 5") - create(:budget_investment, :unfeasible, heading: barajas, title: "Seaport") - create(:budget_investment, :unfeasible, heading: carabanchel, title: "Airport") - - visit budget_path(budget) - - click_link "See unfeasible investments" - - click_link "Districts" - click_link "Barajas" - - within("#budget-investments") do - expect(page).to have_css(".budget-investment", count: 1) - - expect(page).to have_content "Seaport" - expect(page).not_to have_content "Terminal 5" - expect(page).not_to have_content "Airport" - end - end - context "Results Phase" do before { budget.update(phase: "finished", results_enabled: true) } @@ -286,8 +242,7 @@ describe "Budget Investments" do investment1 = create(:budget_investment, :winner, heading: heading) investment2 = create(:budget_investment, :selected, heading: heading) - visit budget_path(budget) - click_link "Health" + visit budget_investments_path(budget, heading_id: heading) within("#budget-investments") do expect(page).to have_css(".budget-investment", count: 1) @@ -297,7 +252,6 @@ describe "Budget Investments" do visit budget_results_path(budget) click_link "List of all investment projects" - click_link "Health" within("#budget-investments") do expect(page).to have_css(".budget-investment", count: 1) @@ -312,7 +266,6 @@ describe "Budget Investments" do visit budget_results_path(budget) click_link "List of all unfeasible investment projects" - click_link "Health" within("#budget-investments") do expect(page).to have_css(".budget-investment", count: 1) @@ -327,7 +280,6 @@ describe "Budget Investments" do visit budget_results_path(budget) click_link "List of all investment projects not selected for balloting" - click_link "Health" within("#budget-investments") do expect(page).to have_css(".budget-investment", count: 1) @@ -1305,19 +1257,12 @@ describe "Budget Investments" do create(:budget_investment, :selected, price: 100000, heading: new_york_heading, title: "NASA base") login_as(user) - visit budget_path(budget) - - click_link "Global Group" - # No need to click_link "Global Heading" because the link of a group with a single heading - # points to the list of investments directly + visit budget_investments_path(budget, heading: global_heading) add_to_ballot("World T-Shirt") add_to_ballot("Eco pens") - visit budget_path(budget) - - click_link "Health" - click_link "Carabanchel" + visit budget_investments_path(budget, heading: carabanchel_heading) add_to_ballot("Fireworks") add_to_ballot("Bus pass") @@ -1360,10 +1305,7 @@ describe "Budget Investments" do create(:budget_investment, :selected, heading: heading_1, title: "Zero-emission zone") login_as(user) - visit budget_path(budget) - - click_link "Health" - click_link "Heading 1" + visit budget_investments_path(budget, heading_id: heading_1) add_to_ballot("Zero-emission zone") @@ -1413,45 +1355,6 @@ describe "Budget Investments" do end end - scenario "Shows unselected link for group with one heading" do - group = create(:budget_group, name: "All City", budget: budget) - heading = create(:budget_heading, name: "Madrid", group: group) - - visit budget_path(budget) - click_link "See investments not selected for balloting phase" - - click_link "All City" - - expected_path = budget_investments_path(budget, heading_id: heading.id, filter: "unselected") - expect(page).to have_current_path(expected_path) - end - - scenario "Shows unselected link for group with many headings" do - group = create(:budget_group, name: "Districts", budget: budget) - - barajas = create(:budget_heading, name: "Barajas", group: group) - carabanchel = create(:budget_heading, name: "Carabanchel", group: group) - - create(:budget_investment, :selected, heading: barajas, title: "Terminal 5") - create(:budget_investment, :unselected, heading: barajas, title: "Seaport") - create(:budget_investment, :unselected, heading: carabanchel, title: "Airport") - - visit budget_path(budget) - - click_link "See investments not selected for balloting phase" - - click_link "Districts" - click_link "Barajas" - - within("#budget-investments") do - expect(page).to have_css(".budget-investment", count: 1) - - expect(page).to have_content "Seaport" - expect(page).not_to have_content "Terminal 5" - expect(page).not_to have_content "Airport" - end - end - scenario "Do not display vote button for unselected investments in index" do investment = create(:budget_investment, :unselected, heading: heading) diff --git a/spec/system/tags/budget_investments_spec.rb b/spec/system/tags/budget_investments_spec.rb index 6abd533df..032c60ab5 100644 --- a/spec/system/tags/budget_investments_spec.rb +++ b/spec/system/tags/budget_investments_spec.rb @@ -256,8 +256,7 @@ describe "Tags" do end login_as(admin) if budget.drafting? - visit budget_path(budget) - click_link group.name + visit budget_investments_path(budget, heading: heading.id) within "#tag-cloud" do click_link new_tag @@ -305,8 +304,7 @@ describe "Tags" do end login_as(admin) if budget.drafting? - visit budget_path(budget) - click_link group.name + visit budget_investments_path(budget, heading: heading.id) within "#categories" do click_link tag_medio_ambiente.name From f3c7b596580dc909ff80bf0d7f6dcd922518419b Mon Sep 17 00:00:00 2001 From: decabeza Date: Mon, 4 May 2020 12:44:33 +0200 Subject: [PATCH 08/12] Fix heading tag hierarchy on budgets index --- app/components/budgets/budget_component.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/budgets/budget_component.html.erb b/app/components/budgets/budget_component.html.erb index 18f84f767..30053714c 100644 --- a/app/components/budgets/budget_component.html.erb +++ b/app/components/budgets/budget_component.html.erb @@ -23,7 +23,7 @@ <% unless budget.informing? %>
    -

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

    +

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

    <%= render_map(nil, "budgets", false, nil, coordinates) %>
    <% end %> From 05e35844308090ce27586363a997b4ce49775369 Mon Sep 17 00:00:00 2001 From: decabeza Date: Fri, 9 Oct 2020 12:05:58 +0200 Subject: [PATCH 09/12] Adjust groups and headings styles --- .../stylesheets/budgets/groups_and_headings.scss | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/budgets/groups_and_headings.scss b/app/assets/stylesheets/budgets/groups_and_headings.scss index 6f6b4a2d9..b3e532f1d 100644 --- a/app/assets/stylesheets/budgets/groups_and_headings.scss +++ b/app/assets/stylesheets/budgets/groups_and_headings.scss @@ -1,5 +1,5 @@ .groups-and-headings { - $spacing: rem-calc(4); + $spacing: $line-height / 2; .headings-list { display: flex; @@ -9,10 +9,12 @@ } .heading { - border: 1px solid $border; - border-radius: rem-calc(3); + border: 2px solid $border; + border-radius: rem-calc(6); + box-shadow: 0 0 10px 0 rgba(0, 0, 0, 0.1); margin-bottom: $line-height / 2; margin-left: $spacing; + margin-top: $line-height / 4; padding: $line-height / 2; position: relative; width: 100%; @@ -34,6 +36,7 @@ } a { + font-weight: bold; &::after, &::before { @@ -58,6 +61,7 @@ span { display: block; font-size: $small-font-size; + padding-top: $line-height / 2; } } } From 135e154fe3a044214e7d138c4bb02e145c3807b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 15 Mar 2021 14:24:30 +0100 Subject: [PATCH 10/12] Fix "go back" link in budget group and investments Even if we usually only access these pages for the current budget, that might not always be the case, and now that we've unified budget landing pages, there's no point in them pointing to the index anymore. --- app/views/budgets/groups/show.html.erb | 2 +- app/views/budgets/investments/_header.html.erb | 2 +- spec/system/budgets/groups_spec.rb | 8 ++++++++ spec/system/budgets/investments_spec.rb | 4 ++++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/views/budgets/groups/show.html.erb b/app/views/budgets/groups/show.html.erb index 81a797797..aa1786f6b 100644 --- a/app/views/budgets/groups/show.html.erb +++ b/app/views/budgets/groups/show.html.erb @@ -5,7 +5,7 @@
    - <%= back_link_to budgets_path %> + <%= back_link_to budget_path(@budget) %>

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

    diff --git a/app/views/budgets/investments/_header.html.erb b/app/views/budgets/investments/_header.html.erb index 0b18c3e1f..36fd6c02b 100644 --- a/app/views/budgets/investments/_header.html.erb +++ b/app/views/budgets/investments/_header.html.erb @@ -4,7 +4,7 @@
    - <%= back_link_to budgets_path %> + <%= back_link_to budget_path(@budget) %> <% if can? :show, @ballot %> <%= link_to t("budgets.investments.header.check_ballot"), diff --git a/spec/system/budgets/groups_spec.rb b/spec/system/budgets/groups_spec.rb index df2eb6c8e..9b26bbd13 100644 --- a/spec/system/budgets/groups_spec.rb +++ b/spec/system/budgets/groups_spec.rb @@ -68,5 +68,13 @@ describe "Budget Groups" do expect(page).to have_link "Southwest" expect(page).not_to have_link "See investments not selected for balloting phase unfeasible investments" end + + scenario "Back link", :js do + visit budget_group_path(budget, group) + + click_link "Go back" + + expect(page).to have_current_path budget_path(budget) + end end end diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 08cdad36f..65f463bd6 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -88,6 +88,10 @@ describe "Budget Investments" do expect(page).not_to have_content(unfeasible_investment.title) end end + + click_link "Go back" + + expect(page).to have_current_path budget_path(budget) end scenario "Index view mode" do From 2552330fe0a7f50c276763bfe4755f7c07164470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 19 Mar 2021 13:39:58 +0100 Subject: [PATCH 11/12] Fix "go back" link in ballot page Since the `@ballot_referer` variable was only set in the lines controller, it didn't work when we accessed the ballot page without adding a line. Note it still doesn't work if we access the ballot page directly by entering the URL in the browser's address bar. --- .../budgets/ballot/lines_controller.rb | 5 -- app/views/budgets/ballot/_ballot.html.erb | 2 +- spec/system/budgets/ballots_spec.rb | 49 +++++++++++++------ 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/app/controllers/budgets/ballot/lines_controller.rb b/app/controllers/budgets/ballot/lines_controller.rb index e9df6ab2c..c8c02d296 100644 --- a/app/controllers/budgets/ballot/lines_controller.rb +++ b/app/controllers/budgets/ballot/lines_controller.rb @@ -7,7 +7,6 @@ module Budgets before_action :load_tag_cloud before_action :load_categories before_action :load_investments - before_action :load_ballot_referer authorize_resource :budget authorize_resource :ballot @@ -67,10 +66,6 @@ module Budgets @categories = Tag.category.order(:name) end - def load_ballot_referer - @ballot_referer = session[:ballot_referer] - end - def load_map @investments ||= [] @investments_map_coordinates = MapLocation.where(investment: @investments).map(&:json_data) diff --git a/app/views/budgets/ballot/_ballot.html.erb b/app/views/budgets/ballot/_ballot.html.erb index c320a1652..8abf49a19 100644 --- a/app/views/budgets/ballot/_ballot.html.erb +++ b/app/views/budgets/ballot/_ballot.html.erb @@ -1,6 +1,6 @@
    - <%= back_link_to @ballot_referer %> + <%= back_link_to session[:ballot_referer] %>

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

    diff --git a/spec/system/budgets/ballots_spec.rb b/spec/system/budgets/ballots_spec.rb index 2d9a6df07..b762ce830 100644 --- a/spec/system/budgets/ballots_spec.rb +++ b/spec/system/budgets/ballots_spec.rb @@ -422,28 +422,45 @@ describe "Ballots" do end end - scenario "Back link after removing an investment from Ballot", :js do - create(:budget_investment, :selected, heading: new_york, price: 10, title: "Sully monument") + describe "Back link", :js do + scenario "after adding and removing an investment from the ballot" do + create(:budget_investment, :selected, heading: new_york, price: 10, title: "Sully monument") - login_as(user) - visit budget_investments_path(budget, heading_id: new_york.id) - add_to_ballot("Sully monument") + login_as(user) + visit budget_investments_path(budget, heading_id: new_york.id) + add_to_ballot("Sully monument") - within(".budget-heading") do - click_link "Check and confirm my ballot" + within(".budget-heading") do + click_link "Check and confirm my ballot" + end + + expect(page).to have_content("You have voted one investment") + + within(".ballot-list li", text: "Sully monument") do + find(".icon-x").click + end + + expect(page).to have_content("You have voted 0 investments") + + click_link "Go back" + + expect(page).to have_current_path(budget_investments_path(budget, heading_id: new_york.id)) end - expect(page).to have_content("You have voted one investment") + scenario "before adding any investments" do + login_as(user) + visit budget_investments_path(budget, heading_id: new_york.id) - within(".ballot-list li", text: "Sully monument") do - find(".icon-x").click + within(".budget-heading") do + click_link "Check and confirm my ballot" + end + + expect(page).to have_content("You have voted 0 investments") + + click_link "Go back" + + expect(page).to have_current_path(budget_investments_path(budget, heading_id: new_york.id)) end - - expect(page).to have_content("You have voted 0 investments") - - click_link "Go back" - - expect(page).to have_current_path(budget_investments_path(budget, heading_id: new_york.id)) end context "Permissions" do From 644557a09423984b937c066ab155109f0bad3a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 19 Mar 2021 13:46:42 +0100 Subject: [PATCH 12/12] Remove redundant code to set ballot referer We were setting it twice: once inside the action and once after the action. --- app/controllers/budgets/ballots_controller.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/controllers/budgets/ballots_controller.rb b/app/controllers/budgets/ballots_controller.rb index 52b68c630..ecf0bb67e 100644 --- a/app/controllers/budgets/ballots_controller.rb +++ b/app/controllers/budgets/ballots_controller.rb @@ -4,7 +4,6 @@ module Budgets before_action :load_budget authorize_resource :budget before_action :load_ballot - after_action :store_referer, only: [:show] def show authorize! :show, @ballot @@ -22,9 +21,5 @@ module Budgets query = Budget::Ballot.where(user: current_user, budget: @budget) @ballot = @budget.balloting? ? query.first_or_create! : query.first_or_initialize end - - def store_referer - session[:ballot_referer] = request.referer - end end end