From 00dfa95902741ed22edb2f2fa0a12308e06e2963 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Fri, 5 Feb 2021 13:38:25 +0100
Subject: [PATCH 1/2] Allow toggling the column selector with a keyboard

Using a button tag, it's possible for every user to "click" the element.
Besides, we don't need to call the `preventDefault` function, because
buttons with type "button" don't do anything by default.
---
 app/assets/javascripts/columns_selector.js        |  7 +++----
 app/views/admin/shared/_columns_selector.html.erb |  4 ++--
 spec/system/admin/budget_investments_spec.rb      | 14 ++++----------
 3 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/app/assets/javascripts/columns_selector.js b/app/assets/javascripts/columns_selector.js
index 2f680b7c6..4c5201cd3 100644
--- a/app/assets/javascripts/columns_selector.js
+++ b/app/assets/javascripts/columns_selector.js
@@ -33,8 +33,7 @@
         $("#js-columns-selector-wrapper").append(item);
       });
     },
-    toggleOptions: function(event) {
-      event.preventDefault();
+    toggleOptions: function() {
       $("#js-columns-selector").toggleClass("hollow");
       $("#js-columns-selector-wrapper").toggleClass("hidden");
     },
@@ -73,8 +72,8 @@
       App.ColumnsSelector.initChecks();
       App.ColumnsSelector.initColumns();
       $("#js-columns-selector").on({
-        click: function(event) {
-          App.ColumnsSelector.toggleOptions(event);
+        click: function() {
+          App.ColumnsSelector.toggleOptions();
         }
       });
       $(".column-selector-item input").on({
diff --git a/app/views/admin/shared/_columns_selector.html.erb b/app/views/admin/shared/_columns_selector.html.erb
index dfae1d808..9e8ea9926 100644
--- a/app/views/admin/shared/_columns_selector.html.erb
+++ b/app/views/admin/shared/_columns_selector.html.erb
@@ -1,7 +1,7 @@
-<span class="button columns-selector hollow" id="js-columns-selector" data-cookie="<%= cookie %>" data-default="<%= default.join(",") %>">
+<button type="button" class="button columns-selector hollow" id="js-columns-selector" data-cookie="<%= cookie %>" data-default="<%= default.join(",") %>">
   <span class="icon-banner"> </span>
   <strong><%= t("admin.budget_investments.index.columns") %></strong>
-</span>
+</button>
 
 <div class="hidden columns-selector-wrapper" id="js-columns-selector-wrapper">
   <div class="hidden column-selector-item" id="column_selector_item_template">
diff --git a/spec/system/admin/budget_investments_spec.rb b/spec/system/admin/budget_investments_spec.rb
index 5e4241bb6..738363ac3 100644
--- a/spec/system/admin/budget_investments_spec.rb
+++ b/spec/system/admin/budget_investments_spec.rb
@@ -1825,9 +1825,7 @@ describe "Admin budget investments", :admin do
     scenario "Use column selector to display visible columns", :js do
       visit admin_budget_budget_investments_path(budget)
 
-      within("#js-columns-selector") do
-        find("strong", text: "Columns").click
-      end
+      click_button "Columns"
 
       within("#js-columns-selector-wrapper") do
         selectable_columns.each do |column|
@@ -1857,9 +1855,7 @@ describe "Admin budget investments", :admin do
     scenario "Cookie will be updated after change columns selection", :js do
       visit admin_budget_budget_investments_path(budget)
 
-      within("#js-columns-selector") do
-        find("strong", text: "Columns").click
-      end
+      click_button "Columns"
 
       within("#js-columns-selector-wrapper") do
         uncheck "Title"
@@ -1889,7 +1885,7 @@ describe "Admin budget investments", :admin do
       investment.update!(title: "Don't display me, please!")
 
       visit admin_budget_budget_investments_path(budget)
-      within("#js-columns-selector") { find("strong", text: "Columns").click }
+      click_button "Columns"
       within("#js-columns-selector-wrapper") { uncheck "Title" }
 
       within("#budget_investment_#{investment.id}") do
@@ -1909,9 +1905,7 @@ describe "Admin budget investments", :admin do
 
       go_back
 
-      within("#js-columns-selector") do
-        find("strong", text: "Columns").click
-      end
+      click_button "Columns"
 
       within("#js-columns-selector-wrapper") do
         selectable_columns.each do |column|

From 556966330829df617a21205d57fa2f70493d8ed0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sun, 7 Feb 2021 17:27:00 +0100
Subject: [PATCH 2/2] Use aria-expanded on column selector toggle button

This way screen reader users will be informed about what happens when
the button is clicked. We also simplify the HTML/JavaScript a little
bit.
---
 app/assets/javascripts/columns_selector.js       |  7 +++----
 app/assets/stylesheets/admin.scss                | 16 ++++++++++++----
 .../admin/shared/_columns_selector.html.erb      |  4 ++--
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/app/assets/javascripts/columns_selector.js b/app/assets/javascripts/columns_selector.js
index 4c5201cd3..888dd2c76 100644
--- a/app/assets/javascripts/columns_selector.js
+++ b/app/assets/javascripts/columns_selector.js
@@ -33,9 +33,8 @@
         $("#js-columns-selector-wrapper").append(item);
       });
     },
-    toggleOptions: function() {
-      $("#js-columns-selector").toggleClass("hollow");
-      $("#js-columns-selector-wrapper").toggleClass("hidden");
+    toggleOptions: function(button) {
+      button.attr("aria-expanded", !JSON.parse(button.attr("aria-expanded")));
     },
     hideAll: function() {
       $("[data-field]").addClass("hidden");
@@ -73,7 +72,7 @@
       App.ColumnsSelector.initColumns();
       $("#js-columns-selector").on({
         click: function() {
-          App.ColumnsSelector.toggleOptions();
+          App.ColumnsSelector.toggleOptions($(this));
         }
       });
       $(".column-selector-item input").on({
diff --git a/app/assets/stylesheets/admin.scss b/app/assets/stylesheets/admin.scss
index a469e7c28..51a554038 100644
--- a/app/assets/stylesheets/admin.scss
+++ b/app/assets/stylesheets/admin.scss
@@ -1165,6 +1165,18 @@ table {
   span[class^="icon-"] {
     vertical-align: middle;
   }
+
+  &[aria-expanded='false'] {
+    @include hollow-button;
+
+    + .columns-selector-wrapper {
+      display: none;
+    }
+  }
+
+  &[aria-expanded='true'] {
+    @include regular-button;
+  }
 }
 
 .columns-selector-wrapper {
@@ -1173,10 +1185,6 @@ table {
   border-radius: 10px;
   margin-bottom: 1rem;
 
-  &.hidden {
-    display: none;
-  }
-
   .column-selector-item {
     display: inline-block;
 
diff --git a/app/views/admin/shared/_columns_selector.html.erb b/app/views/admin/shared/_columns_selector.html.erb
index 9e8ea9926..c440a23cc 100644
--- a/app/views/admin/shared/_columns_selector.html.erb
+++ b/app/views/admin/shared/_columns_selector.html.erb
@@ -1,9 +1,9 @@
-<button type="button" class="button columns-selector hollow" id="js-columns-selector" data-cookie="<%= cookie %>" data-default="<%= default.join(",") %>">
+<button type="button" class="columns-selector" id="js-columns-selector" aria-expanded="false" data-cookie="<%= cookie %>" data-default="<%= default.join(",") %>">
   <span class="icon-banner"> </span>
   <strong><%= t("admin.budget_investments.index.columns") %></strong>
 </button>
 
-<div class="hidden columns-selector-wrapper" id="js-columns-selector-wrapper">
+<div class="columns-selector-wrapper" id="js-columns-selector-wrapper">
   <div class="hidden column-selector-item" id="column_selector_item_template">
     <input type="checkbox" name="column-selector[template]" id="column_selector_template" data-column="template">
     <label for="column_selector_template"></label>