From 0dec47c055d5859a2c377cc9efa6f59eff46715d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Mon, 15 Apr 2024 19:02:24 +0200
Subject: [PATCH] Use just the banner title as the banner link text

We were using an <a> tag wrapping the whole content of the banner in
order to make the whole banner clickable. However, that made the text of
the link less concise, affecting people using screen readers. So,
instead, we're using the `card` mixin, which we introduced in commit
f285dfcba.

We're making this change now because the HTML5 Sanitizer that we're
about to enable in the next commit was handling the whitespace inside
the banner differently, causing one test to fail, and we didn't find a
different way to fix it.
---
 app/assets/stylesheets/shared/banner.scss     | 21 ++++++++++++-------
 .../shared/banner_component.html.erb          |  2 +-
 app/components/shared/banner_component.rb     |  8 +++----
 spec/system/admin/banners_spec.rb             |  2 +-
 4 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/app/assets/stylesheets/shared/banner.scss b/app/assets/stylesheets/shared/banner.scss
index fc25fcc62..553f39399 100644
--- a/app/assets/stylesheets/shared/banner.scss
+++ b/app/assets/stylesheets/shared/banner.scss
@@ -1,5 +1,7 @@
 .banner {
   @include full-width-background;
+  @include card;
+  padding: 0 rem-calc(16);
 
   .debates-list &::before,
   .proposals-list &::before,
@@ -7,18 +9,21 @@
     content: none;
   }
 
+  &:hover {
+    text-decoration: underline;
+  }
+
   a {
-    display: block;
-
-    > * {
-      padding: 0 rem-calc(16);
-
-      &:empty {
-        display: none;
-      }
+    &,
+    &:hover {
+      color: inherit;
     }
   }
 
+  > :empty {
+    display: none;
+  }
+
   + .budget-header,
   + .budgets-index > .budget-header,
   + .jumbo {
diff --git a/app/components/shared/banner_component.html.erb b/app/components/shared/banner_component.html.erb
index cbea18d0e..6fb3236cd 100644
--- a/app/components/shared/banner_component.html.erb
+++ b/app/components/shared/banner_component.html.erb
@@ -1,3 +1,3 @@
 <div class="banner" style="background-color:<%= banner.background_color %>;">
-  <%= sanitize link, attributes: %w[href style] %>
+  <%= sanitize banner_content, attributes: %w[href style] %>
 </div>
diff --git a/app/components/shared/banner_component.rb b/app/components/shared/banner_component.rb
index 21aba7c65..bffbe1a7b 100644
--- a/app/components/shared/banner_component.rb
+++ b/app/components/shared/banner_component.rb
@@ -19,10 +19,8 @@ class Shared::BannerComponent < ApplicationComponent
 
   private
 
-    def link
-      link_to banner.target_url do
-        tag.h2(banner.title, style: "color:#{banner.font_color}") +
-          tag.h3(banner.description, style: "color:#{banner.font_color}")
-      end
+    def banner_content
+      tag.h2(link_to(banner.title, banner.target_url), style: "color:#{banner.font_color}") +
+        tag.h3(banner.description, style: "color:#{banner.font_color}")
     end
 end
diff --git a/spec/system/admin/banners_spec.rb b/spec/system/admin/banners_spec.rb
index c2bbab091..29a6eeb7d 100644
--- a/spec/system/admin/banners_spec.rb
+++ b/spec/system/admin/banners_spec.rb
@@ -86,7 +86,7 @@ describe "Admin banners magement", :admin do
     visit proposals_path
 
     expect(page).to have_content "Such banner"
-    expect(page).to have_link "Such banner many text wow link", href: "https://www.url.com"
+    expect(page).to have_link "Such banner", href: "https://www.url.com"
   end
 
   scenario "Publish a banner with a translation different than the current locale" do