Show "not allowed" message on click

Defining a behavior on hover means making it different for people using a keyboard or a touchscreen (most of the population, nowadays). In this case, we had an accessibility issue where the message wouldn't disappear once it appeared. That meant that, after tabbing through all the links and buttons in, for instance, the debates index, the page would be filled with "participation not allowed" messages, and in order to see the information about how many people have voted, reloading the page was required. For touchscreen users the behavior was similar to what we get on hover, although we've found some inconsistencies when trying to support several elements on the same page. We think in proposals it makes sense to hide the "support" button when users click on it, and the same applies to the buttonsto support and vote investment projects. However, we aren't hiding the buttons to agree/disagree with a debate in order to keep the information about the current number of people agreeing and disagreeing visible. Note we're removing some support spec methods because after these changes the duplication isn't as obvious as it was in the past.
2022-02-19 13:05:56 +01:00
parent 9c4d406a77
commit abec716308
24 changed files with 135 additions and 146 deletions
--- a/app/assets/javascripts/application.js
+++ b/app/assets/javascripts/application.js
@@ -65,7 +65,7 @@
 //= require jquery.amsify.suggestags
 //= require tags
 //= require users
-//= require votes
+//= require participation_not_allowed
 //= require advanced_search
 //= require registration_form
 //= require suggest
@@ -122,7 +122,7 @@ var initialize_modules = function() {
  App.Questions.initialize();
  App.Comments.initialize();
  App.Users.initialize();
-  App.Votes.initialize();
+  App.ParticipationNotAllowed.initialize();
  App.Tags.initialize();
  App.FoundationExtras.initialize();
  App.LocationChanger.initialize();
--- a/app/assets/javascripts/participation_not_allowed.js
+++ b/app/assets/javascripts/participation_not_allowed.js
@@ -0,0 +1,29 @@
+(function() {
+  "use strict";
+
+  App.ParticipationNotAllowed = {
+    not_allowed: function(votes_selector) {
+      var buttons_selector = votes_selector + " [type='submit']";
+
+      $("body").on("click", buttons_selector, function(event) {
+        var votes = $(event.target).closest(votes_selector);
+        var not_allowed = $("div.participation-not-allowed", votes);
+
+        if (not_allowed.length > 0) {
+          event.preventDefault();
+          not_allowed.show().focus();
+
+          if (votes_selector === "div.votes") {
+            $("button", votes).prop("disabled", true);
+          } else {
+            $(event.target).closest("form").remove();
+          }
+        }
+      });
+    },
+    initialize: function() {
+      App.ParticipationNotAllowed.not_allowed("div.votes");
+      App.ParticipationNotAllowed.not_allowed("div.supports");
+    }
+  };
+}).call(this);
--- a/app/assets/javascripts/votes.js
+++ b/app/assets/javascripts/votes.js
@@ -1,19 +0,0 @@
-(function() {
-  "use strict";
-  App.Votes = {
-    hoverize: function(votes) {
-      $(document).on({
-        "mouseenter focus": function() {
-          $("div.participation-not-allowed", this).show();
-        },
-        mouseleave: function() {
-          $("div.participation-not-allowed", this).hide();
-        }
-      }, votes);
-    },
-    initialize: function() {
-      App.Votes.hoverize("div.votes");
-      App.Votes.hoverize("div.supports");
-    }
-  };
-}).call(this);
--- a/app/assets/stylesheets/in_favor_against.scss
+++ b/app/assets/stylesheets/in_favor_against.scss
@@ -23,31 +23,37 @@
    padding: rem-calc(3) rem-calc(6) rem-calc(6);
    position: relative;

-    &:hover,
-    &:active {
-      color: #fff;
-      cursor: pointer;
-      opacity: 1 !important;
+    &:not([disabled]) {
+      &:hover,
+      &:active {
+        color: #fff;
+        cursor: pointer;
+        opacity: 1 !important;
+      }
    }
  }

  .in-favor button {
    @include has-fa-icon(thumbs-up, solid);

-    &:hover,
-    &:active {
-      background: $like;
-      border: 2px solid $like;
+    &:not([disabled]) {
+      &:hover,
+      &:active {
+        background: $like;
+        border: 2px solid $like;
+      }
    }
  }

  .against button {
    @include has-fa-icon(thumbs-down, solid);

-    &:hover,
-    &:active {
-      background: $unlike;
-      border: 2px solid $unlike;
+    &:not([disabled]) {
+      &:hover,
+      &:active {
+        background: $unlike;
+        border: 2px solid $unlike;
+      }
    }
  }

--- a/app/assets/stylesheets/participation.scss
+++ b/app/assets/stylesheets/participation.scss
@@ -98,15 +98,11 @@
 .participation-not-allowed {
  background: $warning-bg;
  color: $color-warning;
-  left: 0;
+  display: none;
  line-height: $line-height;
-  min-height: 100%;
  padding: $line-height / 2;
-  position: absolute;
  text-align: center;
-  top: 0;
  width: 100%;
-  z-index: 2;

  &,
  p {
@@ -327,7 +323,7 @@
 }

 .debate-questions .debate-questions .participation-not-allowed {
-  position: static;
+  display: block;
 }

 .bullet {
@@ -667,10 +663,6 @@
      font-size: $base-font-size;
      font-weight: bold;

-      &:disabled {
-        opacity: 1;
-      }
-
      &:hover {
        background: $budget-hover;
        color: #fff;
--- a/app/components/budgets/investments/ballot_component.html.erb
+++ b/app/components/budgets/investments/ballot_component.html.erb
@@ -32,7 +32,6 @@
                      title: t("budgets.investments.investment.support_title"),
                      method: :post,
                      remote: true,
-                      disabled: reason.present?,
                      "aria-label": vote_aria_label do %>
          <%= t("budgets.investments.investment.add") %>
        <% end %>
--- a/app/components/budgets/investments/votes_component.html.erb
+++ b/app/components/budgets/investments/votes_component.html.erb
@@ -28,7 +28,6 @@
            method: "post",
            remote: !display_support_alert?,
            data:   ({ confirm: confirm_vote_message } if display_support_alert?),
-            disabled: !current_user,
            "aria-label": support_aria_label %>
      <% end %>
    <% end %>
--- a/app/components/debates/votes_component.html.erb
+++ b/app/components/debates/votes_component.html.erb
@@ -1,9 +1,9 @@
 <div class="votes">
  <%= render Shared::InFavorAgainstComponent.new(debate) %>

+  <%= render Shared::ParticipationNotAllowedComponent.new(debate, cannot_vote_text: cannot_vote_text) %>
+
  <span class="total-votes">
    <%= t("debates.debate.votes", count: debate.votes_score) %>
  </span>
-
-  <%= render Shared::ParticipationNotAllowedComponent.new(debate, cannot_vote_text: cannot_vote_text) %>
 </div>
--- a/app/components/legislation/proposals/votes_component.html.erb
+++ b/app/components/legislation/proposals/votes_component.html.erb
@@ -3,12 +3,12 @@
    <%= render Shared::InFavorAgainstComponent.new(proposal) %>
  <% end %>

+  <%= render Shared::ParticipationNotAllowedComponent.new(proposal, cannot_vote_text: cannot_vote_text) %>
+
  <span class="total-votes">
    <%= t("proposals.proposal.votes", count: proposal.votes_score) %>
  </span>

-  <%= render Shared::ParticipationNotAllowedComponent.new(proposal, cannot_vote_text: cannot_vote_text) %>
-
  <% if current_user&.voted_as_when_voted_for(proposal) && setting["twitter_handle"] %>
    <div class="share-supported">
      <%= render "shared/social_share",
--- a/app/components/proposals/votes_component.html.erb
+++ b/app/components/proposals/votes_component.html.erb
@@ -12,7 +12,6 @@
          title: t("proposals.proposal.support_title"),
          method: "post",
          remote: true,
-          disabled: !can_vote?,
          "aria-label": support_aria_label do %>
        <%= t("proposals.proposal.support") %>
      <% end %>
--- a/app/components/shared/in_favor_against_component.html.erb
+++ b/app/components/shared/in_favor_against_component.html.erb
@@ -5,8 +5,7 @@
        title: t("votes.agree"),
        "aria-label": agree_aria_label,
        method: "post",
-        remote: true,
-        disabled: !current_user do %>
+        remote: true do %>
      <span class="show-for-sr"><%= t("votes.agree") %></span>
    <% end %>
    <span class="percentage"><%= votes_percentage("likes", votable) %></span>
@@ -18,8 +17,7 @@
        title: t("votes.disagree"),
        "aria-label": disagree_aria_label,
        method: "post",
-        remote: true,
-        disabled: !current_user do %>
+        remote: true do %>
      <span class="show-for-sr"><%= t("votes.disagree") %></span>
    <% end %>
    <span class="percentage"><%= votes_percentage("dislikes", votable) %></span>
--- a/app/components/shared/participation_not_allowed_component.html.erb
+++ b/app/components/shared/participation_not_allowed_component.html.erb
@@ -1,5 +1,3 @@
-<div tabindex="0">
-  <div class="participation-not-allowed" style="display:none" aria-hidden="false">
-    <%= body %>
-  </div>
+<div class="participation-not-allowed" tabindex="-1">
+  <%= body %>
 </div>
--- a/spec/components/budgets/investments/ballot_component_spec.rb
+++ b/spec/components/budgets/investments/ballot_component_spec.rb
@@ -18,7 +18,7 @@ describe Budgets::Investments::BallotComponent do

      render_inline component

-      expect(page).to have_button "Vote", disabled: true
+      expect(page).to have_button "Vote"
      expect(page).to have_content "Only verified users can vote on investments; verify your account."
    end

--- a/spec/components/budgets/investments/votes_component_spec.rb
+++ b/spec/components/budgets/investments/votes_component_spec.rb
@@ -18,11 +18,12 @@ describe Budgets::Investments::VotesComponent do
        expect(page).to have_button "Support Renovate sidewalks in Main Street"
      end

-      it "disables the button to support the investment to unidentified users" do
+      it "renders the support button and a reminder to sign in to unidentified users" do
        render_inline component

        expect(page).to have_button count: 1, disabled: :all
-        expect(page).to have_button "Support", disabled: true
+        expect(page).to have_button "Support"
+        expect(page).to have_content "You must sign in or sign up to continue."
      end

      describe "button to remove support" do
--- a/spec/components/debates/votes_component_spec.rb
+++ b/spec/components/debates/votes_component_spec.rb
@@ -5,11 +5,11 @@ describe Debates::VotesComponent do
  let(:component) { Debates::VotesComponent.new(debate) }

  describe "Agree and disagree buttons" do
-    it "is disabled to anonymous users" do
+    it "is shown to anonymous users alongside a reminder to sign in" do
      render_inline component

-      expect(page).to have_button "I agree", disabled: true
-      expect(page).to have_button "I disagree", disabled: true
+      expect(page).to have_button "I agree"
+      expect(page).to have_button "I disagree"
      expect(page).to have_content "You must sign in or sign up to continue."
    end

--- a/spec/components/legislation/proposals/votes_component_spec.rb
+++ b/spec/components/legislation/proposals/votes_component_spec.rb
@@ -18,11 +18,11 @@ describe Legislation::Proposals::VotesComponent do
      expect(page).not_to have_button "I disagree", disabled: :all
    end

-    it "is disabled to anonymous users" do
+    it "is shown to anonymous users alongside a reminder to sign in" do
      render_inline component

-      expect(page).to have_button "I agree", disabled: true
-      expect(page).to have_button "I disagree", disabled: true
+      expect(page).to have_button "I agree"
+      expect(page).to have_button "I disagree"
      expect(page).to have_content "You must sign in or sign up to continue."
    end

--- a/spec/components/proposals/votes_component_spec.rb
+++ b/spec/components/proposals/votes_component_spec.rb
@@ -5,12 +5,12 @@ describe Proposals::VotesComponent do
  let(:component) { Proposals::VotesComponent.new(proposal) }

  describe "support proposal button" do
-    it "is disabled to unverified users" do
+    it "is shown to unverified users" do
      sign_in(create(:user))

      render_inline component

-      expect(page).to have_button "Support", disabled: true
+      expect(page).to have_button "Support"
    end

    it "is shown to verified users" do
@@ -38,8 +38,8 @@ describe Proposals::VotesComponent do
    it "asks anonymous users to sign in or sign up" do
      render_inline component

-      expect(page).to have_link "sign in", visible: :hidden
-      expect(page).to have_link "sign up", visible: :hidden
+      expect(page).to have_link "sign in"
+      expect(page).to have_link "sign up"
    end

    it "says voting is not allowed to organizations" do
@@ -58,7 +58,7 @@ describe Proposals::VotesComponent do
      render_inline component

      expect(page).to have_content "Only verified users can vote on proposals"
-      expect(page).to have_link "verify your account", visible: :hidden
+      expect(page).to have_link "verify your account"
      expect(page).not_to have_link "sign in", visible: :all
      expect(page).not_to have_link "sign up", visible: :all
    end
--- a/spec/support/common_actions.rb
+++ b/spec/support/common_actions.rb
@@ -13,7 +13,6 @@ module CommonActions
  include Translations
  include Users
  include Verifications
-  include Votes

  def app_host
    "#{Capybara.app_host}:#{Capybara::Server.ports.values.last}"
--- a/spec/support/common_actions/budgets.rb
+++ b/spec/support/common_actions/budgets.rb
@@ -4,11 +4,6 @@ module Budgets
    expect(page).to have_button "Vote", disabled: true, obscured: true
  end

-  def hover_over_ballot
-    scroll_to find("div.ballot"), align: :bottom
-    first("div.ballot p").hover
-  end
-
  def add_to_ballot(investment_title)
    within(".budget-investment", text: investment_title) do
      click_button "Vote"
--- a/spec/support/common_actions/votes.rb
+++ b/spec/support/common_actions/votes.rb
@@ -1,16 +0,0 @@
-module Votes
-  def expect_message_you_need_to_sign_in
-    expect(page).to have_content "You must sign in or sign up to continue"
-    expect(page).to have_selector(".in-favor", obscured: true)
-  end
-
-  def expect_message_to_many_anonymous_votes
-    expect(page).to have_content "Too many anonymous votes to admit vote"
-    expect(page).to have_button "I agree", obscured: true
-  end
-
-  def expect_message_only_verified_can_vote_proposals
-    expect(page).to have_content "Only verified users can vote on proposals"
-    expect(page).to have_selector(".in-favor", obscured: true)
-  end
-end
--- a/spec/system/budget_polls/voter_spec.rb
+++ b/spec/system/budget_polls/voter_spec.rb
@@ -81,11 +81,12 @@ describe "BudgetPolls", :with_frozen_time do
      login_as(user)

      visit budget_investment_path(budget, investment)
-      first("div.ballot p").hover

      within("#budget_investment_#{investment.id}") do
+        click_button "Vote"
+
        expect(page).to have_content "You have already participated offline"
-        expect(page).to have_button "Vote", disabled: true, obscured: true
+        expect(page).not_to have_button "Vote", disabled: :all
      end
    end
  end
@@ -111,7 +112,6 @@ describe "BudgetPolls", :with_frozen_time do
      end

      visit budget_investment_path(budget, investment)
-      find("div.ballot").hover

      within("#budget_investment_#{investment.id}") do
        expect(page).to have_content "Remove vote"
--- a/spec/system/budgets/ballots_spec.rb
+++ b/spec/system/budgets/ballots_spec.rb
@@ -446,10 +446,10 @@ describe "Ballots" do
      visit budget_investments_path(budget, heading_id: new_york.id)

      within("#budget_investment_#{investment.id}") do
-        hover_over_ballot
+        click_button "Vote"

        expect(page).to have_content "You must sign in or sign up to continue."
-        expect(page).to have_button "Vote", disabled: true, obscured: true
+        expect(page).not_to have_button "Vote", disabled: :all
      end
    end

@@ -461,10 +461,10 @@ describe "Ballots" do
      visit budget_investments_path(budget, heading_id: new_york.id)

      within("#budget_investment_#{investment.id}") do
-        hover_over_ballot
+        click_button "Vote"

        expect(page).to have_content "Only verified users can vote on investments"
-        expect(page).to have_button "Vote", disabled: true, obscured: true
+        expect(page).not_to have_button "Vote", disabled: :all
      end
    end

@@ -476,9 +476,10 @@ describe "Ballots" do
      visit budget_investments_path(budget, heading_id: new_york.id)

      within("#budget_investment_#{investment.id}") do
-        hover_over_ballot
+        click_button "Vote"

-        expect_message_organizations_cannot_vote
+        expect(page).to have_content "Organization"
+        expect(page).not_to have_button "Vote", disabled: :all
      end
    end

@@ -512,10 +513,10 @@ describe "Ballots" do
      visit budget_investments_path(budget, heading: new_york)

      within("#budget_investment_#{bi2.id}") do
-        hover_over_ballot
+        click_button "Vote"

        expect(page).to have_content("already voted a different heading")
-        expect(page).to have_button "Vote", disabled: true, obscured: true
+        expect(page).not_to have_button "Vote", disabled: :all
      end
    end

@@ -528,10 +529,10 @@ describe "Ballots" do
      visit budget_investments_path(budget, heading_id: california.id)

      within("#budget_investment_#{bi2.id}") do
-        hover_over_ballot
+        click_button "Vote"

        expect(page).to have_content("You have already assigned the available budget")
-        expect(page).to have_button "Vote", disabled: true, obscured: true
+        expect(page).not_to have_button "Vote", disabled: :all
      end
    end

@@ -542,20 +543,13 @@ describe "Ballots" do
      login_as(user)
      visit budget_investments_path(budget, heading_id: california.id)

-      within(".budget-investment", text: "Build replicants") do
-        hover_over_ballot
-
-        expect(page).not_to have_content("You have already assigned the available budget")
-        expect(page).to have_button "Vote", obscured: false
-      end
-
      add_to_ballot("Build replicants")

      within(".budget-investment", text: "Build terminators") do
-        hover_over_ballot
+        click_button "Vote"

        expect(page).to have_content("You have already assigned the available budget")
-        expect(page).to have_button "Vote", disabled: true, obscured: true
+        expect(page).not_to have_button "Vote", disabled: :all
      end
    end

@@ -568,10 +562,11 @@ describe "Ballots" do
      visit budget_investments_path(budget, heading_id: california.id)

      within("#budget_investment_#{bi2.id}") do
-        hover_over_ballot
+        click_button "Vote"

        expect(page).to have_content("You have already assigned the available budget")
-        expect(page).to have_button "Vote", disabled: true, obscured: true
+        expect(page).not_to have_button "Vote", disabled: :all
+        expect(page).not_to have_button "Remove vote"
      end

      within("#budget_investment_#{bi1.id}") do
@@ -580,10 +575,10 @@ describe "Ballots" do
      end

      within("#budget_investment_#{bi2.id}") do
-        hover_over_ballot
+        click_button "Vote"

        expect(page).not_to have_content("You have already assigned the available budget")
-        expect(page).to have_button "Vote", obscured: false
+        expect(page).to have_button "Remove vote"
      end
    end

@@ -596,10 +591,10 @@ describe "Ballots" do
      visit budget_investments_path(budget, heading_id: california.id)

      within("#budget_investment_#{bi2.id}") do
-        hover_over_ballot
+        click_button "Vote"

        expect(page).to have_content("You have already assigned the available budget")
-        expect(page).to have_button "Vote", disabled: true, obscured: true
+        expect(page).not_to have_button "Vote", disabled: :all
      end

      within("#budget_investment_#{bi1.id}_sidebar") do
@@ -609,10 +604,10 @@ describe "Ballots" do
      expect(page).not_to have_css "#budget_investment_#{bi1.id}_sidebar"

      within("#budget_investment_#{bi2.id}") do
-        hover_over_ballot
+        click_button "Vote"

        expect(page).not_to have_content("You have already assigned the available budget")
-        expect(page).to have_button "Vote", obscured: false
+        expect(page).to have_button "Remove vote"
      end
    end

@@ -641,13 +636,13 @@ describe "Ballots" do
      within("#budget_investment_#{investment1.id}") do
        click_button "Vote"

+        expect(page).to have_css ".participation-not-allowed", visible: :hidden
        expect(page).not_to have_content "Remove"
-        expect(page).not_to have_selector(".participation-not-allowed")

-        hover_over_ballot
+        click_button "Vote"

-        expect(page).to have_selector(".participation-not-allowed")
-        expect(page).to have_button "Vote", disabled: true, obscured: true
+        expect(page).to have_css ".participation-not-allowed"
+        expect(page).not_to have_button "Vote", disabled: :all
      end
    end

--- a/spec/system/budgets/votes_spec.rb
+++ b/spec/system/budgets/votes_spec.rb
@@ -229,9 +229,8 @@ describe "Votes" do
      expect(page).to have_content("YOU CAN STILL CAST 1 VOTE")

      within(".budget-investment", text: investment2.title) do
-        find("div.ballot").hover
-
-        expect(page).not_to have_content("You have already assigned the available budget")
+        expect(page).not_to have_css ".participation-not-allowed", visible: :all
+        expect(page).not_to have_content "You have already assigned the available budget"
      end

      visit budget_ballot_path(budget)
--- a/spec/system/votes_spec.rb
+++ b/spec/system/votes_spec.rb
@@ -252,8 +252,11 @@ describe "Votes" do

    visit debates_path
    within("#debate_#{debate.id}") do
-      find("div.votes").hover
-      expect_message_you_need_to_sign_in
+      click_button "I agree"
+
+      expect(page).to have_content "You must sign in or sign up to continue"
+      expect(page).to have_button "I agree", disabled: true
+      expect(page).to have_button "I disagree", disabled: true
    end
  end

@@ -262,14 +265,18 @@ describe "Votes" do

    visit proposals_path
    within("#proposal_#{proposal.id}") do
-      find("div.supports").hover
-      expect_message_you_need_to_sign_in
+      click_button "Support"
+
+      expect(page).to have_content "You must sign in or sign up to continue"
+      expect(page).not_to have_button "Support", disabled: :all
    end

    visit proposal_path(proposal)
    within("#proposal_#{proposal.id}") do
-      find("div.supports").hover
-      expect_message_you_need_to_sign_in
+      click_button "Support"
+
+      expect(page).to have_content "You must sign in or sign up to continue"
+      expect(page).not_to have_button "Support", disabled: :all
    end
  end

@@ -310,14 +317,18 @@ describe "Votes" do

    visit debates_path
    within("#debate_#{debate.id}") do
-      find("div.votes").hover
-      expect_message_to_many_anonymous_votes
+      click_button "I agree"
+
+      expect(page).to have_content "Too many anonymous votes to admit vote"
+      expect(page).to have_button "I agree", disabled: true
    end

    visit debate_path(debate)
    within("#debate_#{debate.id}") do
-      find("div.votes").hover
-      expect_message_to_many_anonymous_votes
+      click_button "I agree"
+
+      expect(page).to have_content "Too many anonymous votes to admit vote"
+      expect(page).to have_button "I agree", disabled: true
    end
  end

@@ -329,14 +340,18 @@ describe "Votes" do
    visit proposals_path

    within("#proposal_#{proposal.id}") do
-      find("div.supports").hover
-      expect_message_only_verified_can_vote_proposals
+      click_button "Support"
+
+      expect(page).to have_content "Only verified users can vote on proposals"
+      expect(page).not_to have_button "Support", disabled: :all
    end

    visit proposal_path(proposal)
    within("#proposal_#{proposal.id}") do
-      find("div.supports").hover
-      expect_message_only_verified_can_vote_proposals
+      click_button "Support"
+
+      expect(page).to have_content "Only verified users can vote on proposals"
+      expect(page).not_to have_button "Support", disabled: :all
    end
  end
 end