From 26a8f2eace22192cafbf7929008b81b80327e6fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 6 May 2021 18:06:26 +0200
Subject: [PATCH] Increase menu button touch area on small screens

Some users might not be able to touch the icon due to a motor
disability. Other users might think the "Menu" text is part of the
button and try to touch it instead.

Making the "Menu" text part of the button makes it easier to show/hide
this menu. Besides, it lets screen reader users with a small screen hear
the word "Menu" associated to the button.

We could simplify the HTML a bit more but Foundation's `hamburger` mixin
uses the `::after` element with `position: absolute`, so we can't apply
it directly to the button without making the CSS more complex.
---
 app/assets/stylesheets/layout.scss       |  5 ++++-
 app/views/layouts/_admin_header.html.erb |  6 ++++--
 app/views/layouts/_header.html.erb       |  6 ++++--
 spec/system/admin_spec.rb                | 18 ++++++++++++++++++
 spec/system/home_spec.rb                 | 18 ++++++++++++++++++
 5 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss
index 425c17d87..c42d10a40 100644
--- a/app/assets/stylesheets/layout.scss
+++ b/app/assets/stylesheets/layout.scss
@@ -667,9 +667,12 @@ body > header,
   }
 }
 
+.menu-button {
+  color: inherit;
+}
+
 .menu-icon {
   @include hamburger($color: currentcolor, $color-hover: currentcolor);
-  color: inherit;
 }
 
 .title-bar {
diff --git a/app/views/layouts/_admin_header.html.erb b/app/views/layouts/_admin_header.html.erb
index 1995fa7ff..73f93c602 100644
--- a/app/views/layouts/_admin_header.html.erb
+++ b/app/views/layouts/_admin_header.html.erb
@@ -11,8 +11,10 @@
 
   <div class="expanded row admin-top-bar">
     <div class="title-bar" data-responsive-toggle="responsive_menu" data-hide-for="medium">
-      <button class="menu-icon" type="button" data-toggle="responsive_menu"></button>
-      <div class="title-bar-title"><%= t("application.menu") %></div>
+      <button class="menu-button" type="button" data-toggle="responsive_menu">
+        <span class="menu-icon"></span>
+        <span class="title-bar-title"><%= t("application.menu") %></span>
+      </button>
     </div>
 
     <div class="top-bar">
diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb
index 6aa11150b..d11c57fe4 100644
--- a/app/views/layouts/_header.html.erb
+++ b/app/views/layouts/_header.html.erb
@@ -16,8 +16,10 @@
     <div class="top-bar">
 
       <span data-responsive-toggle="responsive-menu" data-hide-for="medium" class="float-right">
-        <button type="button" class="menu-icon" data-toggle></button>
-        <%= t("application.menu") %>
+        <button type="button" class="menu-button" data-toggle>
+          <span class="menu-icon"></span>
+          <%= t("application.menu") %>
+        </button>
       </span>
 
       <h1 class="top-bar-title">
diff --git a/spec/system/admin_spec.rb b/spec/system/admin_spec.rb
index 3f115c7dc..88e8000da 100644
--- a/spec/system/admin_spec.rb
+++ b/spec/system/admin_spec.rb
@@ -104,4 +104,22 @@ describe "Admin" do
       expect(page).to have_link "Participatory budgets"
     end
   end
+
+  describe "Menu button", :admin do
+    scenario "is not present on large screens" do
+      visit admin_root_path
+
+      expect(page).not_to have_button "Menu"
+    end
+
+    scenario "toggles the menu on small screens", :small_window do
+      visit admin_root_path
+
+      expect(page).not_to have_link "My account"
+
+      click_button "Menu"
+
+      expect(page).to have_link "My account"
+    end
+  end
 end
diff --git a/spec/system/home_spec.rb b/spec/system/home_spec.rb
index 0cacb822b..f1482e2d9 100644
--- a/spec/system/home_spec.rb
+++ b/spec/system/home_spec.rb
@@ -138,6 +138,24 @@ describe "Home" do
     end
   end
 
+  describe "Menu button" do
+    scenario "is not present on large screens" do
+      visit root_path
+
+      expect(page).not_to have_button "Menu"
+    end
+
+    scenario "toggles the menu on small screens", :small_window do
+      visit root_path
+
+      expect(page).not_to have_link "Sign in"
+
+      click_button "Menu"
+
+      expect(page).to have_link "Sign in"
+    end
+  end
+
   scenario "if there are cards, the 'featured' title will render" do
     create(:widget_card,
       title: "Card text",