From 86ad2df46d4792d4c3841ac77acc2962bda0509b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 13 Jan 2022 17:51:13 +0100 Subject: [PATCH] Unify code in debates/legislation vote links We were using the same code to render links to agree and disagree, so we can extract a new component for this code. We're also adding component tests to make it easier to test whether we're breaking anything while refactoring, although the code is probably already covered by system tests. Since the votes mixin was only used in one place, we're removing it and moving most of its code to a new CSS file for the shared component. --- app/assets/stylesheets/application.scss | 1 + app/assets/stylesheets/in_favor_against.scss | 97 +++++++++++++ app/assets/stylesheets/participation.scss | 133 +++--------------- .../debates/votes_component.html.erb | 39 +---- app/components/debates/votes_component.rb | 6 +- .../proposals/votes_component.html.erb | 39 +---- .../legislation/proposals/votes_component.rb | 6 +- .../in_favor_against_component.html.erb | 40 ++++++ .../shared/in_favor_against_component.rb | 25 ++++ app/helpers/votes_helper.rb | 11 -- .../debates/votes_component_spec.rb | 29 ++++ .../proposals/votes_component_spec.rb | 42 ++++++ 12 files changed, 254 insertions(+), 214 deletions(-) create mode 100644 app/assets/stylesheets/in_favor_against.scss create mode 100644 app/components/shared/in_favor_against_component.html.erb create mode 100644 app/components/shared/in_favor_against_component.rb create mode 100644 spec/components/debates/votes_component_spec.rb create mode 100644 spec/components/legislation/proposals/votes_component_spec.rb diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 0aa0834a7..4420f2b36 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -27,6 +27,7 @@ @import "milestones"; @import "pages"; @import "dashboard"; +@import "in_favor_against"; @import "legislation"; @import "legislation_process"; @import "legislation_process_form"; diff --git a/app/assets/stylesheets/in_favor_against.scss b/app/assets/stylesheets/in_favor_against.scss new file mode 100644 index 000000000..1fb29b0a4 --- /dev/null +++ b/app/assets/stylesheets/in_favor_against.scss @@ -0,0 +1,97 @@ +.in-favor-against { + display: inline-block; + + .icon-like, + .icon-unlike { + background: #fff; + border: 2px solid; + border-radius: rem-calc(3); + color: $text-light; + display: inline-block; + font-size: rem-calc(30); + line-height: rem-calc(30); + padding: rem-calc(3) rem-calc(6); + position: relative; + + &:hover, + &:active { + color: #fff; + cursor: pointer; + opacity: 1 !important; + } + } + + .icon-like { + + &:hover, + &:active { + background: $like; + border: 2px solid $like; + } + } + + .icon-unlike { + + &:hover, + &:active { + background: $unlike; + border: 2px solid $unlike; + } + } + + .like, + .unlike { + line-height: rem-calc(48); + vertical-align: super; + text-decoration: none; + + .percentage { + color: $text; + display: inline-block; + font-size: $small-font-size; + line-height: $line-height * 2; + padding-right: $line-height / 2; + vertical-align: top; + + @include breakpoint(medium) { + display: block; + line-height: $line-height; + padding-right: 0; + } + } + } + + .voted { + + .icon-like, + .icon-unlike { + color: #fff; + } + + .icon-like { + background: $like; + border: 2px solid $like; + } + + .icon-unlike { + background: $unlike; + border: 2px solid $unlike; + } + } + + .no-voted { + + .icon-like, + .icon-unlike { + opacity: 0.3; + } + } + + .against { + margin-left: $line-height / 4; + } + + .divider { + margin: 0 rem-calc(6); + } +} diff --git a/app/assets/stylesheets/participation.scss b/app/assets/stylesheets/participation.scss index 4bab52179..33509d545 100644 --- a/app/assets/stylesheets/participation.scss +++ b/app/assets/stylesheets/participation.scss @@ -14,120 +14,6 @@ // 01. Votes and supports // ---------------------- -@mixin votes { - border-top: 1px solid $border; - margin-top: $line-height; - padding: $line-height / 2 0; - position: relative; - - @include breakpoint(medium) { - border-left: 1px solid $border; - border-top: 0; - margin-top: 0; - } - - .icon-like, - .icon-unlike { - background: #fff; - border: 2px solid; - border-radius: rem-calc(3); - color: $text-light; - display: inline-block; - font-size: rem-calc(30); - line-height: rem-calc(30); - padding: rem-calc(3) rem-calc(6); - position: relative; - - &:hover, - &:active { - color: #fff; - cursor: pointer; - opacity: 1 !important; - } - } - - .icon-like { - - &:hover, - &:active { - background: $like; - border: 2px solid $like; - } - } - - .icon-unlike { - - &:hover, - &:active { - background: $unlike; - border: 2px solid $unlike; - } - } - - .like, - .unlike { - line-height: rem-calc(48); - vertical-align: super; - text-decoration: none; - - .percentage { - color: $text; - display: inline-block; - font-size: $small-font-size; - line-height: $line-height * 2; - padding-right: $line-height / 2; - vertical-align: top; - - @include breakpoint(medium) { - display: block; - line-height: $line-height; - padding-right: 0; - } - } - } - - .voted { - - .icon-like, - .icon-unlike { - color: #fff; - } - - .icon-like { - background: $like; - border: 2px solid $like; - } - - .icon-unlike { - background: $unlike; - border: 2px solid $unlike; - } - } - - .no-voted { - - .icon-like, - .icon-unlike { - opacity: 0.3; - } - } - - .total-votes { - font-weight: bold; - float: right; - line-height: $line-height * 2; - - @include breakpoint(medium) { - display: block; - float: none; - } - } - - .divider { - margin: 0 rem-calc(6); - } -} - @mixin supports { padding: $line-height 0; position: relative; @@ -713,14 +599,27 @@ .legislation-proposals { .votes { - @include votes; + border-top: 1px solid $border; + margin-top: $line-height; + padding: $line-height / 2 0; + position: relative; @include breakpoint(medium) { + border-left: 1px solid $border; + border-top: 0; + margin-top: 0; text-align: center; } - .against { - margin-left: $line-height / 4; + .total-votes { + font-weight: bold; + float: right; + line-height: $line-height * 2; + + @include breakpoint(medium) { + display: block; + float: none; + } } } } diff --git a/app/components/debates/votes_component.html.erb b/app/components/debates/votes_component.html.erb index ce933fc54..659a5054c 100644 --- a/app/components/debates/votes_component.html.erb +++ b/app/components/debates/votes_component.html.erb @@ -1,42 +1,5 @@
-
- <% if current_user %> - <%= link_to vote_debate_path(debate, value: "yes"), - class: "like #{voted_classes[:in_favor]}", title: t("votes.agree"), method: "post", remote: true do %> - - <%= t("votes.agree") %> - - <%= votes_percentage("likes", debate) %> - <% end %> - <% else %> - - <% end %> -
- - - -
- <% if current_user %> - <%= link_to vote_debate_path(debate, value: "no"), class: "unlike #{voted_classes[:against]}", title: t("votes.disagree"), method: "post", remote: true do %> - - <%= t("votes.disagree") %> - - <%= votes_percentage("dislikes", debate) %> - <% end %> - <% else %> -
- - <%= t("votes.disagree") %> - - <%= votes_percentage("dislikes", debate) %> -
- <% end %> -
+ <%= render Shared::InFavorAgainstComponent.new(debate) %> <%= t("debates.debate.votes", count: debate.votes_score) %> diff --git a/app/components/debates/votes_component.rb b/app/components/debates/votes_component.rb index 4f7ed8e6d..b8fbed82b 100644 --- a/app/components/debates/votes_component.rb +++ b/app/components/debates/votes_component.rb @@ -1,6 +1,6 @@ class Debates::VotesComponent < ApplicationComponent attr_reader :debate - delegate :css_classes_for_vote, :current_user, :link_to_verify_account, :votes_percentage, to: :helpers + delegate :current_user, :link_to_verify_account, to: :helpers def initialize(debate) @debate = debate @@ -8,10 +8,6 @@ class Debates::VotesComponent < ApplicationComponent private - def voted_classes - @voted_classes ||= css_classes_for_vote(debate) - end - def can_vote? debate.votable_by?(current_user) end diff --git a/app/components/legislation/proposals/votes_component.html.erb b/app/components/legislation/proposals/votes_component.html.erb index 078786754..86620b33e 100644 --- a/app/components/legislation/proposals/votes_component.html.erb +++ b/app/components/legislation/proposals/votes_component.html.erb @@ -1,43 +1,6 @@
<% if proposal.process.proposals_phase.open? %> -
- <% if current_user %> - <%= link_to vote_legislation_process_proposal_path(process_id: proposal.process, id: proposal, value: "yes"), - class: "like #{voted_classes[:in_favor]}", title: t("votes.agree"), method: "post", remote: true do %> - - <%= t("votes.agree") %> - - <%= votes_percentage("likes", proposal) %> - <% end %> - <% else %> - - <% end %> -
- - - -
- <% if current_user %> - <%= link_to vote_legislation_process_proposal_path(process_id: proposal.process, id: proposal, value: "no"), class: "unlike #{voted_classes[:against]}", title: t("votes.disagree"), method: "post", remote: true do %> - - <%= t("votes.disagree") %> - - <%= votes_percentage("dislikes", proposal) %> - <% end %> - <% else %> -
- - <%= t("votes.disagree") %> - - <%= votes_percentage("dislikes", proposal) %> -
- <% end %> -
+ <%= render Shared::InFavorAgainstComponent.new(proposal) %> <% end %> diff --git a/app/components/legislation/proposals/votes_component.rb b/app/components/legislation/proposals/votes_component.rb index fced9f731..a93d69683 100644 --- a/app/components/legislation/proposals/votes_component.rb +++ b/app/components/legislation/proposals/votes_component.rb @@ -1,6 +1,6 @@ class Legislation::Proposals::VotesComponent < ApplicationComponent attr_reader :proposal - delegate :css_classes_for_vote, :current_user, :link_to_verify_account, :votes_percentage, to: :helpers + delegate :current_user, :link_to_verify_account, to: :helpers def initialize(proposal) @proposal = proposal @@ -8,10 +8,6 @@ class Legislation::Proposals::VotesComponent < ApplicationComponent private - def voted_classes - @voted_classes ||= css_classes_for_vote(proposal) - end - def can_vote? proposal.votable_by?(current_user) end diff --git a/app/components/shared/in_favor_against_component.html.erb b/app/components/shared/in_favor_against_component.html.erb new file mode 100644 index 000000000..a085c1d46 --- /dev/null +++ b/app/components/shared/in_favor_against_component.html.erb @@ -0,0 +1,40 @@ +
+
+ <% if current_user %> + <%= link_to polymorphic_path(votable, action: :vote, value: "yes"), + class: "like #{voted_classes[:in_favor]}", title: t("votes.agree"), method: "post", remote: true do %> + + <%= t("votes.agree") %> + + <%= votes_percentage("likes", votable) %> + <% end %> + <% else %> + + <% end %> +
+ + + +
+ <% if current_user %> + <%= link_to polymorphic_path(votable, action: :vote, value: "no"), class: "unlike #{voted_classes[:against]}", title: t("votes.disagree"), method: "post", remote: true do %> + + <%= t("votes.disagree") %> + + <%= votes_percentage("dislikes", votable) %> + <% end %> + <% else %> +
+ + <%= t("votes.disagree") %> + + <%= votes_percentage("dislikes", votable) %> +
+ <% end %> +
+
diff --git a/app/components/shared/in_favor_against_component.rb b/app/components/shared/in_favor_against_component.rb new file mode 100644 index 000000000..4d944d14c --- /dev/null +++ b/app/components/shared/in_favor_against_component.rb @@ -0,0 +1,25 @@ +class Shared::InFavorAgainstComponent < ApplicationComponent + attr_reader :votable + delegate :current_user, :votes_percentage, to: :helpers + + def initialize(votable) + @votable = votable + end + + private + + def voted_classes + @voted_classes ||= css_classes_for_vote + end + + def css_classes_for_vote + case current_user&.voted_as_when_voted_for(votable) + when true + { in_favor: "voted", against: "no-voted" } + when false + { in_favor: "no-voted", against: "voted" } + else + { in_favor: "", against: "" } + end + end +end diff --git a/app/helpers/votes_helper.rb b/app/helpers/votes_helper.rb index e4b4f6a3c..8292717e4 100644 --- a/app/helpers/votes_helper.rb +++ b/app/helpers/votes_helper.rb @@ -12,15 +12,4 @@ module VotesHelper "#{100 - debate_percentage_of_likes(debate)}%" end end - - def css_classes_for_vote(votable) - case current_user&.voted_as_when_voted_for(votable) - when true - { in_favor: "voted", against: "no-voted" } - when false - { in_favor: "no-voted", against: "voted" } - else - { in_favor: "", against: "" } - end - end end diff --git a/spec/components/debates/votes_component_spec.rb b/spec/components/debates/votes_component_spec.rb new file mode 100644 index 000000000..25979df62 --- /dev/null +++ b/spec/components/debates/votes_component_spec.rb @@ -0,0 +1,29 @@ +require "rails_helper" + +describe Debates::VotesComponent do + let(:debate) { create(:debate, title: "What about the 2030 agenda?") } + let(:component) { Debates::VotesComponent.new(debate) } + + describe "Agree and disagree links" do + it "is shown as plain text to anonymous users" do + render_inline component + + expect(page).to have_content "I agree" + expect(page).to have_content "I disagree" + expect(page).to have_content "You must sign in or sign up to continue." + expect(page).not_to have_link "I agree" + expect(page).not_to have_link "I disagree" + end + + it "is shown to identified users" do + sign_in(create(:user)) + + render_inline component + + expect(page).to have_link count: 2 + expect(page).to have_link "I agree", title: "I agree" + expect(page).to have_link "I disagree", title: "I disagree" + expect(page).not_to have_content "You must sign in or sign up to continue." + end + end +end diff --git a/spec/components/legislation/proposals/votes_component_spec.rb b/spec/components/legislation/proposals/votes_component_spec.rb new file mode 100644 index 000000000..e83f2266e --- /dev/null +++ b/spec/components/legislation/proposals/votes_component_spec.rb @@ -0,0 +1,42 @@ +require "rails_helper" + +describe Legislation::Proposals::VotesComponent do + let(:proposal) { create(:legislation_proposal, title: "Require wearing masks at home") } + let(:component) { Legislation::Proposals::VotesComponent.new(proposal) } + + describe "Agree and disagree links" do + it "is not shown when the proposals phase isn't open" do + proposal.process.update!( + proposals_phase_start_date: 2.days.ago, + proposals_phase_end_date: Date.yesterday + ) + + sign_in(create(:user)) + render_inline component + + expect(page).not_to have_content "I agree" + expect(page).not_to have_content "I disagree" + end + + it "is shown as plain text to anonymous users" do + render_inline component + + expect(page).to have_content "I agree" + expect(page).to have_content "I disagree" + expect(page).to have_content "You must sign in or sign up to continue." + expect(page).not_to have_link "I agree" + expect(page).not_to have_link "I disagree" + end + + it "is shown to identified users" do + sign_in(create(:user)) + + render_inline component + + expect(page).to have_link count: 2 + expect(page).to have_link "I agree", title: "I agree" + expect(page).to have_link "I disagree", title: "I disagree" + expect(page).not_to have_content "You must sign in or sign up to continue." + end + end +end