From 0f45dbb896ced256279b39367b52ad8507382fe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 10 Oct 2023 22:18:22 +0200 Subject: [PATCH 1/6] Simplify border-related rules for like/unlike icons We were keeping the same style and width when they were pressed, so we can simply overwrite the color. --- app/assets/stylesheets/in_favor_against.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/in_favor_against.scss b/app/assets/stylesheets/in_favor_against.scss index 6e8d48481..ecc5944ff 100644 --- a/app/assets/stylesheets/in_favor_against.scss +++ b/app/assets/stylesheets/in_favor_against.scss @@ -41,7 +41,7 @@ &:active, &[aria-pressed=true] { background: $like; - border: 2px solid $like; + border-color: $like; } } } @@ -54,7 +54,7 @@ &:active, &[aria-pressed=true] { background: $unlike; - border: 2px solid $unlike; + border-color: $unlike; } } } From 1387356c86f0888ba08120deffd35503d6ec0f8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 10 Oct 2023 22:25:17 +0200 Subject: [PATCH 2/6] Refactor like/unlike buttons SCSS We were using the same selectors three times. Since we're going to change that part of the code, we're simplifying it so we don't have to do the same changes three times. --- app/assets/stylesheets/in_favor_against.scss | 31 +++++++------------- app/assets/stylesheets/layout.scss | 17 +++++------ 2 files changed, 18 insertions(+), 30 deletions(-) diff --git a/app/assets/stylesheets/in_favor_against.scss b/app/assets/stylesheets/in_favor_against.scss index ecc5944ff..03accb68e 100644 --- a/app/assets/stylesheets/in_favor_against.scss +++ b/app/assets/stylesheets/in_favor_against.scss @@ -22,41 +22,30 @@ line-height: rem-calc(30); padding: rem-calc(3) rem-calc(6) rem-calc(6); position: relative; + } + + @mixin like-unlike-icon($icon, $pressed-color) { + @include has-fa-icon($icon, solid); &:not([disabled]) { + cursor: pointer; + &:hover, &:active, &[aria-pressed=true] { + background: $pressed-color; + border-color: $pressed-color; color: #fff; - cursor: pointer; } } } .in-favor button { - @include has-fa-icon(thumbs-up, solid); - - &:not([disabled]) { - &:hover, - &:active, - &[aria-pressed=true] { - background: $like; - border-color: $like; - } - } + @include like-unlike-icon(thumbs-up, $like); } .against button { - @include has-fa-icon(thumbs-down, solid); - - &:not([disabled]) { - &:hover, - &:active, - &[aria-pressed=true] { - background: $unlike; - border-color: $unlike; - } - } + @include like-unlike-icon(thumbs-down, $unlike); } .percentage { diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index fc211c76a..ab5874c24 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -1707,22 +1707,21 @@ table { } } - .in-favor button { - @include has-fa-icon(thumbs-up, solid); + @mixin like-unlike-icon($icon, $pressed-color) { + @include has-fa-icon($icon, solid); &:hover, &[aria-pressed=true] { - color: $like; + color: $pressed-color; } } - .against button { - @include has-fa-icon(thumbs-down, solid); + .in-favor button { + @include like-unlike-icon(thumbs-up, $like); + } - &:hover, - &[aria-pressed=true] { - color: $unlike; - } + .against button { + @include like-unlike-icon(thumbs-down, $unlike); } } From 11a33c12e374d852e3cea4dc6d39c8e1a77e45df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 10 Oct 2023 22:28:53 +0200 Subject: [PATCH 3/6] Increase contrast in like/unlike buttons The colors we were using (gray for unpressed buttons and green or red for pressed buttons) didn't contrast well against a white background, so we're now using darker colors. However, with darker colors, using solid icons for the unpressed buttons makes it harder to differentiate when a button is pressed and when it isn't, particularly for color-blind people. So we're now using regular icons for the unpressed buttons and solid icons for the pressed ones. --- app/assets/stylesheets/_consul_settings.scss | 4 ++-- app/assets/stylesheets/in_favor_against.scss | 5 +++-- app/assets/stylesheets/layout.scss | 5 +++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/_consul_settings.scss b/app/assets/stylesheets/_consul_settings.scss index 34c716112..162671568 100644 --- a/app/assets/stylesheets/_consul_settings.scss +++ b/app/assets/stylesheets/_consul_settings.scss @@ -73,8 +73,8 @@ $border: #dee0e3 !default; $debates: $brand !default; -$like: #7bd2a8 !default; -$unlike: #ef8585 !default; +$like: #38a36f !default; +$unlike: #ea6666 !default; $delete: #db2e0f !default; $check: #46db91 !default; diff --git a/app/assets/stylesheets/in_favor_against.scss b/app/assets/stylesheets/in_favor_against.scss index 03accb68e..de0ceefd9 100644 --- a/app/assets/stylesheets/in_favor_against.scss +++ b/app/assets/stylesheets/in_favor_against.scss @@ -16,7 +16,7 @@ background: #fff; border: 2px solid; border-radius: rem-calc(3); - color: $text-light; + color: $dark-gray; display: inline-block; font-size: rem-calc(30); line-height: rem-calc(30); @@ -25,7 +25,7 @@ } @mixin like-unlike-icon($icon, $pressed-color) { - @include has-fa-icon($icon, solid); + @include has-fa-icon($icon, regular); &:not([disabled]) { cursor: pointer; @@ -33,6 +33,7 @@ &:hover, &:active, &[aria-pressed=true] { + @include has-fa-icon($icon, solid); background: $pressed-color; border-color: $pressed-color; color: #fff; diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index ab5874c24..6c71d9d49 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -1695,7 +1695,7 @@ table { button, form { - color: $text-light; + color: $dark-gray; display: inline-block; } @@ -1708,10 +1708,11 @@ table { } @mixin like-unlike-icon($icon, $pressed-color) { - @include has-fa-icon($icon, solid); + @include has-fa-icon($icon, regular); &:hover, &[aria-pressed=true] { + @include has-fa-icon($icon, solid); color: $pressed-color; } } From 3482e6e058a4c1afaf4dddfd0e07b64a109bb0b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 10 Oct 2023 22:50:20 +0200 Subject: [PATCH 4/6] Change styles of pressed buttons on hover Since we were styling pressed buttons the same way as buttons on hover, a person hovering on the button and then clicking it wouldn't notice that the buttons had been pressed unless they noticed that the number and percentages of votes had changes. They wouldn't notice the changes when unpressing the buttons either, since, after clicking the button, the cursor would still be over it, and so the hover styles would apply. Furthermore, it was hard for mouse users to realize that a button could be unpressed, since the style of pressed buttons didn't change on hover. So we're now changing the icons on hover without changing the background. This way all four states (unpressed, unpressed on hover, pressed, pressed on hover) are styled in a different way. --- app/assets/stylesheets/in_favor_against.scss | 9 ++++++--- app/assets/stylesheets/layout.scss | 7 +++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/in_favor_against.scss b/app/assets/stylesheets/in_favor_against.scss index de0ceefd9..1fb0c288e 100644 --- a/app/assets/stylesheets/in_favor_against.scss +++ b/app/assets/stylesheets/in_favor_against.scss @@ -30,10 +30,13 @@ &:not([disabled]) { cursor: pointer; - &:hover, - &:active, - &[aria-pressed=true] { + &[aria-pressed=false]:hover, + &[aria-pressed=false]:active, + &[aria-pressed=true]:not(:hover, :active) { @include has-fa-icon($icon, solid); + } + + &[aria-pressed=true] { background: $pressed-color; border-color: $pressed-color; color: #fff; diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 6c71d9d49..895749f89 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -1710,9 +1710,12 @@ table { @mixin like-unlike-icon($icon, $pressed-color) { @include has-fa-icon($icon, regular); - &:hover, - &[aria-pressed=true] { + &[aria-pressed=false]:hover, + &[aria-pressed=true]:not(:hover) { @include has-fa-icon($icon, solid); + } + + &[aria-pressed=true] { color: $pressed-color; } } From 38e81f2858cb36e6dcf6bfad2c650851c50521a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 11 Oct 2023 02:44:37 +0200 Subject: [PATCH 5/6] Style "like" buttons to hint they can be unpressed We added the feature to undo a vote and made it obvious for people using screen readers using the `aria-pressed` attribute. However, for sighted people, the only way to know a button can be unpressed is to try to press it again and see what happens. The most obvious way to indicate it would be to add a text indicating that you can undo your vote. However, that'd require changing the design quite a bit. So, after trying a few techniques, we're using a classic approach browsers have used by default for years: using an inset border for pressed buttons and an outset border for unpressed ones. It might now be enough, though; we haven't done usability tests to confirm this point. Since icons to like/unlike comments don't have a border, I'm not sure what to do in this case; the icons are small and that makes it hard to style them in a distinct way. So for now we're not changing these icons. --- app/assets/stylesheets/in_favor_against.scss | 21 +++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/in_favor_against.scss b/app/assets/stylesheets/in_favor_against.scss index 1fb0c288e..a4dd6972c 100644 --- a/app/assets/stylesheets/in_favor_against.scss +++ b/app/assets/stylesheets/in_favor_against.scss @@ -14,7 +14,6 @@ button { background: #fff; - border: 2px solid; border-radius: rem-calc(3); color: $dark-gray; display: inline-block; @@ -22,6 +21,26 @@ line-height: rem-calc(30); padding: rem-calc(3) rem-calc(6) rem-calc(6); position: relative; + + &[aria-pressed=true] { + border-style: inset; + border-width: 3px 2px 2px 3px; + + &:active { + border-style: outset; + border-width: 2px 3px 3px 2px; + } + } + + &[aria-pressed=false] { + border-style: outset; + border-width: 2px 3px 3px 2px; + + &:active { + border-style: inset; + border-width: 3px 2px 2px 3px; + } + } } @mixin like-unlike-icon($icon, $pressed-color) { From 7070b0915b20c43bb7b5ba3ff910a0147f30d47d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 11 Oct 2023 14:11:27 +0200 Subject: [PATCH 6/6] Allow liking/unliking when JavaScript is disabled Even if pretty much nobody uses a browser with JavaScript disabled when navigating our sites, there might be times where JavaScript isn't loaded for reasons like a slow internet connections not getting the JavaScript files or a technical issue. So we're making it possible to still use the like/unlike buttons in these cases. --- app/controllers/comments/votes_controller.rb | 2 ++ app/controllers/debates/votes_controller.rb | 2 ++ .../legislation/proposals/votes_controller.rb | 2 ++ config/locales/en/responders.yml | 2 ++ .../comments/votes_controller_spec.rb | 26 ++++++++++++++++++- .../debates/votes_controller_spec.rb | 24 +++++++++++++++++ .../proposals/votes_controller_spec.rb | 24 +++++++++++++++++ 7 files changed, 81 insertions(+), 1 deletion(-) diff --git a/app/controllers/comments/votes_controller.rb b/app/controllers/comments/votes_controller.rb index d8289f9cf..e5ed1dfad 100644 --- a/app/controllers/comments/votes_controller.rb +++ b/app/controllers/comments/votes_controller.rb @@ -10,6 +10,7 @@ module Comments @comment.vote_by(voter: current_user, vote: params[:value]) respond_to do |format| + format.html { redirect_to request.referer, notice: I18n.t("flash.actions.create.vote") } format.js { render :show } end end @@ -18,6 +19,7 @@ module Comments @comment.unvote_by(current_user) respond_to do |format| + format.html { redirect_to request.referer, notice: I18n.t("flash.actions.destroy.vote") } format.js { render :show } end end diff --git a/app/controllers/debates/votes_controller.rb b/app/controllers/debates/votes_controller.rb index ae0cf33d5..23cbbd477 100644 --- a/app/controllers/debates/votes_controller.rb +++ b/app/controllers/debates/votes_controller.rb @@ -9,6 +9,7 @@ module Debates @debate.register_vote(current_user, params[:value]) respond_to do |format| + format.html { redirect_to request.referer, notice: I18n.t("flash.actions.create.vote") } format.js { render :show } end end @@ -17,6 +18,7 @@ module Debates @debate.unvote_by(current_user) respond_to do |format| + format.html { redirect_to request.referer, notice: I18n.t("flash.actions.destroy.vote") } format.js { render :show } end end diff --git a/app/controllers/legislation/proposals/votes_controller.rb b/app/controllers/legislation/proposals/votes_controller.rb index 5f70a5b95..d60715678 100644 --- a/app/controllers/legislation/proposals/votes_controller.rb +++ b/app/controllers/legislation/proposals/votes_controller.rb @@ -13,6 +13,7 @@ module Legislation @proposal.vote_by(voter: current_user, vote: params[:value]) respond_to do |format| + format.html { redirect_to request.referer, notice: I18n.t("flash.actions.create.vote") } format.js { render :show } end end @@ -21,6 +22,7 @@ module Legislation @proposal.unvote_by(current_user) respond_to do |format| + format.html { redirect_to request.referer, notice: I18n.t("flash.actions.destroy.vote") } format.js { render :show } end end diff --git a/config/locales/en/responders.yml b/config/locales/en/responders.yml index be118de2a..31e9350b0 100644 --- a/config/locales/en/responders.yml +++ b/config/locales/en/responders.yml @@ -17,6 +17,7 @@ en: support: "Investment supported successfully" topic: "Topic created successfully." valuator_group: "Valuator group created successfully" + vote: "Vote created successfully" save_changes: notice: Changes saved update: @@ -35,3 +36,4 @@ en: topic: "Topic deleted successfully." poll_question_answer_video: "Answer video deleted successfully." valuator_group: "Valuator group deleted successfully" + vote: "Vote deleted successfully" diff --git a/spec/controllers/comments/votes_controller_spec.rb b/spec/controllers/comments/votes_controller_spec.rb index cc164423f..fb8ee489a 100644 --- a/spec/controllers/comments/votes_controller_spec.rb +++ b/spec/controllers/comments/votes_controller_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe Comments::VotesController do - let(:comment) { create(:comment) } + let(:comment) { create(:debate_comment) } describe "POST create" do it "allows voting" do @@ -11,6 +11,18 @@ describe Comments::VotesController do post :create, xhr: true, params: { comment_id: comment.id, value: "yes" } end.to change { comment.reload.votes_for.size }.by(1) end + + it "redirects authenticated users without JavaScript to the same page" do + request.env["HTTP_REFERER"] = comment_path(comment) + sign_in create(:user) + + expect do + post :create, params: { comment_id: comment.id, value: "yes" } + end.to change { comment.reload.votes_for.size }.by(1) + + expect(response).to redirect_to comment_path(comment) + expect(flash[:notice]).to eq "Vote created successfully" + end end describe "DELETE destroy" do @@ -30,5 +42,17 @@ describe Comments::VotesController do delete :destroy, xhr: true, params: { comment_id: comment.id, id: vote } end.to change { comment.reload.votes_for.size }.by(-1) end + + it "redirects authenticated users without JavaScript to the same page" do + request.env["HTTP_REFERER"] = debate_path(comment.commentable) + sign_in user + + expect do + delete :destroy, params: { comment_id: comment.id, id: vote } + end.to change { comment.reload.votes_for.size }.by(-1) + + expect(response).to redirect_to debate_path(comment.commentable) + expect(flash[:notice]).to eq "Vote deleted successfully" + end end end diff --git a/spec/controllers/debates/votes_controller_spec.rb b/spec/controllers/debates/votes_controller_spec.rb index 74bd009d2..0e55aa3a3 100644 --- a/spec/controllers/debates/votes_controller_spec.rb +++ b/spec/controllers/debates/votes_controller_spec.rb @@ -18,6 +18,19 @@ describe Debates::VotesController do expect(response).to redirect_to new_user_session_path end + it "redirects authenticated users without JavaScript to the same page" do + debate = create(:debate) + request.env["HTTP_REFERER"] = debate_path(debate) + sign_in create(:user) + + expect do + post :create, params: { debate_id: debate.id, value: "yes" } + end.to change { debate.reload.votes_for.size }.by(1) + + expect(response).to redirect_to debate_path(debate) + expect(flash[:notice]).to eq "Vote created successfully" + end + describe "Vote with too many anonymous votes" do it "allows vote if user is allowed" do Setting["max_ratio_anon_votes_on_debates"] = 100 @@ -52,5 +65,16 @@ describe Debates::VotesController do delete :destroy, xhr: true, params: { debate_id: debate.id, id: vote } end.to change { debate.reload.votes_for.size }.by(-1) end + + it "redirects authenticated users without JavaScript to the same page" do + request.env["HTTP_REFERER"] = debates_path + + expect do + delete :destroy, params: { debate_id: debate.id, id: vote } + end.to change { debate.reload.votes_for.size }.by(-1) + + expect(response).to redirect_to debates_path + expect(flash[:notice]).to eq "Vote deleted successfully" + end end end diff --git a/spec/controllers/legislation/proposals/votes_controller_spec.rb b/spec/controllers/legislation/proposals/votes_controller_spec.rb index f618277b1..e43560027 100644 --- a/spec/controllers/legislation/proposals/votes_controller_spec.rb +++ b/spec/controllers/legislation/proposals/votes_controller_spec.rb @@ -36,6 +36,18 @@ describe Legislation::Proposals::VotesController do post :create, xhr: true, params: vote_params end.not_to change { proposal.reload.votes_for.size } end + + it "redirects authenticated users without JavaScript to the same page" do + request.env["HTTP_REFERER"] = legislation_process_proposal_path(legislation_process, proposal) + sign_in create(:user, :level_two) + + expect do + post :create, params: vote_params + end.to change { proposal.reload.votes_for.size }.by(1) + + expect(response).to redirect_to legislation_process_proposal_path(legislation_process, proposal) + expect(flash[:notice]).to eq "Vote created successfully" + end end describe "DELETE destroy" do @@ -52,5 +64,17 @@ describe Legislation::Proposals::VotesController do delete :destroy, xhr: true, params: vote_params end.to change { proposal.reload.votes_for.size }.by(-1) end + + it "redirects authenticated users without JavaScript to the same page" do + request.env["HTTP_REFERER"] = legislation_process_proposals_path(legislation_process) + sign_in user + + expect do + delete :destroy, params: vote_params + end.to change { proposal.reload.votes_for.size }.by(-1) + + expect(response).to redirect_to legislation_process_proposals_path(legislation_process) + expect(flash[:notice]).to eq "Vote deleted successfully" + end end end