From f92008e91e1884e3d0a4ff6d047b1827d96313ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 6 May 2021 17:28:20 +0200 Subject: [PATCH 1/4] Simplify system tests using a small window --- spec/spec_helper.rb | 9 +++++++++ spec/system/debates_spec.rb | 12 +----------- spec/system/proposals_spec.rb | 24 ++---------------------- 3 files changed, 12 insertions(+), 33 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9bff58db0..b24cd6902 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -98,6 +98,15 @@ RSpec.configure do |config| Delayed::Worker.delay_jobs = false end + config.before(:each, :small_window) do + @window_size = Capybara.current_window.size + Capybara.current_window.resize_to(639, 479) + end + + config.after(:each, :small_window) do + Capybara.current_window.resize_to(*@window_size) + end + config.before(:each, :remote_translations) do allow(RemoteTranslations::Microsoft::AvailableLocales) .to receive(:available_locales).and_return(I18n.available_locales.map(&:to_s)) diff --git a/spec/system/debates_spec.rb b/spec/system/debates_spec.rb index b7714387c..217af3655 100644 --- a/spec/system/debates_spec.rb +++ b/spec/system/debates_spec.rb @@ -103,17 +103,7 @@ describe "Debates" do end end - context "On small devices" do - let!(:window_size) { Capybara.current_window.size } - - before do - Capybara.current_window.resize_to(639, 479) - end - - after do - Capybara.current_window.resize_to(*window_size) - end - + context "On small devices", :small_window do scenario "Shows links to share on telegram and whatsapp too" do visit debate_path(create(:debate)) diff --git a/spec/system/proposals_spec.rb b/spec/system/proposals_spec.rb index 0b104ea8a..172adfe5d 100644 --- a/spec/system/proposals_spec.rb +++ b/spec/system/proposals_spec.rb @@ -148,17 +148,7 @@ describe "Proposals" do end end - context "On small devices" do - let!(:window_size) { Capybara.current_window.size } - - before do - Capybara.current_window.resize_to(639, 479) - end - - after do - Capybara.current_window.resize_to(*window_size) - end - + context "On small devices", :small_window do scenario "Shows links to share on telegram and whatsapp too" do visit proposal_path(create(:proposal)) @@ -253,17 +243,7 @@ describe "Proposals" do end end - describe "Show sticky support button on mobile screens" do - let!(:window_size) { Capybara.current_window.size } - - before do - Capybara.current_window.resize_to(640, 480) - end - - after do - Capybara.current_window.resize_to(*window_size) - end - + describe "Show sticky support button on small screens", :small_window do scenario "On a first visit" do proposal = create(:proposal) visit proposal_path(proposal) From cc69b81ec16b7a17ef77394dfda9ef74e4d1ea1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 6 May 2021 17:48:29 +0200 Subject: [PATCH 2/4] Simplify styles for the menu button Using `currentcolor` and `color: inherit` is IMHO more expressive (we're saying we want to use the same color as the text) and makes it easier to customize these colors in other CONSUL installations. We also remove duplication as we can use the same styles for both the admin and the public layouts. --- app/assets/stylesheets/admin.scss | 8 -------- app/assets/stylesheets/layout.scss | 11 ++--------- app/views/layouts/_header.html.erb | 2 +- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/app/assets/stylesheets/admin.scss b/app/assets/stylesheets/admin.scss index 65f44bbfb..8aede0497 100644 --- a/app/assets/stylesheets/admin.scss +++ b/app/assets/stylesheets/admin.scss @@ -159,14 +159,6 @@ $table-header: #ecf1f6; right: 12px; } - .menu-icon { - - &::after { - background: #000; - box-shadow: 0 7px 0 #000, 0 14px 0 #000; - } - } - .notifications.unread-notifications::after { color: $admin-color; } diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index e4fd19f45..425c17d87 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -668,15 +668,8 @@ body > header, } .menu-icon { - - &.dark { - - &:hover::after, - &::after { - background: #fff; - box-shadow: 0 7px 0 #fff, 0 14px 0 #fff; - } - } + @include hamburger($color: currentcolor, $color-hover: currentcolor); + color: inherit; } .title-bar { diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb index 346582e84..6aa11150b 100644 --- a/app/views/layouts/_header.html.erb +++ b/app/views/layouts/_header.html.erb @@ -16,7 +16,7 @@
- + <%= t("application.menu") %> From 26a8f2eace22192cafbf7929008b81b80327e6fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 6 May 2021 18:06:26 +0200 Subject: [PATCH 3/4] 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 @@
- -
<%= t("application.menu") %>
+
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 @@
- - <%= t("application.menu") %> +

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", From 747e6146a74b7908fade173ef6b2a9869fab1a7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 19 May 2021 00:43:26 +0200 Subject: [PATCH 4/4] Add a border to the menu button This way we make it more obvious that it's clickable and, since adding a border requires adding padding, we increase the touch area, making it easier to use and more accessible for users with motor disabilities. Since now the button looks like a button, we don't need the pointer cursor Foundation adds to hamburger icons and can use the default cursor browsers provide for buttons. --- app/assets/stylesheets/layout.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index c42d10a40..d94a1224b 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -668,11 +668,15 @@ body > header, } .menu-button { + border: 1px solid; + border-radius: $button-radius; color: inherit; + padding: 0.6em; } .menu-icon { @include hamburger($color: currentcolor, $color-hover: currentcolor); + cursor: inherit; } .title-bar {