From 1f10afac640421d2864089056c2790af984f481a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 30 Jun 2021 20:17:28 +0200
Subject: [PATCH] Simplify language selection with a few languages

As mentioned in commit 5214d89c8, using the `change` event of a `select`
field to automatically change location is really annoying for keyboard
users, since the event will trigger when pressing the down key to
navigate through the options or when typing a key to start searching for
an option. This might cause a lot of frustration.

Most multilanguage CONSUL sites enable between 2 and 4 languages. In
these cases, it's easier to just display the list of languages to
simplify the selection.

This way in this situation we also make it clear which languages are
available. If we use a `<select>` tag, users will have to open it in
order to check whether the site is available in their preferred
language.

This is also useful when the current language uses characters users
don't recognize; users will recognize their own language in the list of
available languages, while it might be harder to recognize the language
selector allows them to switch to a different language.

In this case, we're also hiding the label because a list of links with
language names is usually self explanatory for sighted users. We're
still providing it for screen reader users so they immediately know the
list allows them to change the language and if they don't need to do so
they can quickly skip it.
---
 .../stylesheets/layout/locale_switcher.scss   | 51 ++++++++++++++-----
 .../layout/locale_switcher_component.html.erb | 20 +++++---
 .../layout/locale_switcher_component.rb       | 29 +++++++++--
 .../layout/locale_switcher_component_spec.rb  | 24 +++++++++
 4 files changed, 101 insertions(+), 23 deletions(-)

diff --git a/app/assets/stylesheets/layout/locale_switcher.scss b/app/assets/stylesheets/layout/locale_switcher.scss
index 424e7c793..c3f7c2be3 100644
--- a/app/assets/stylesheets/layout/locale_switcher.scss
+++ b/app/assets/stylesheets/layout/locale_switcher.scss
@@ -1,25 +1,25 @@
 .locale {
-  @include has-fa-icon(angle-down, solid, after);
   float: left;
   margin-bottom: $line-height / 4;
   margin-top: $line-height / 4;
   position: relative;
 
-  &::after {
-    color: #808080;
-    font-size: $small-font-size;
-    margin-right: 0;
-    pointer-events: none;
-    position: absolute;
-    right: 2px;
-    top: 50%;
-    transform: translateY(-50%);
-  }
-
   .locale-form {
+    @include has-fa-icon(angle-down, solid, after);
     display: inline-block;
     position: relative;
 
+    &::after {
+      color: #808080;
+      font-size: $small-font-size;
+      margin-right: 0;
+      pointer-events: none;
+      position: absolute;
+      right: 2px;
+      top: 50%;
+      transform: translateY(-50%);
+    }
+
     label {
       color: #fff;
       font-size: $tiny-font-size;
@@ -51,4 +51,31 @@
       }
     }
   }
+
+  p {
+    @include element-invisible;
+  }
+
+  ul {
+    $gap: 0.75em;
+    display: flex;
+    flex-wrap: wrap;
+    margin-left: -$gap;
+
+    > * {
+      margin-left: $gap;
+    }
+
+    li::after {
+      content: none;
+    }
+
+    a {
+      margin-left: 0;
+    }
+
+    [aria-current] {
+      font-weight: bold;
+    }
+  }
 }
diff --git a/app/components/layout/locale_switcher_component.html.erb b/app/components/layout/locale_switcher_component.html.erb
index dfb1c2c39..0ff9aac9a 100644
--- a/app/components/layout/locale_switcher_component.html.erb
+++ b/app/components/layout/locale_switcher_component.html.erb
@@ -1,10 +1,14 @@
 <div class="locale">
-  <form class="locale-form">
-    <label class="inline-block" for="locale-switcher">
-      <%= t("layouts.header.locale") %>
-    </label>
-    <select class="js-location-changer locale-switcher inline-block" name="locale-switcher" id="locale-switcher">
-      <%= options_for_select(language_options, current_path_with_query_params(locale: I18n.locale)) %>
-    </select>
-  </form>
+  <% if many_locales? %>
+    <form class="locale-form">
+      <label class="inline-block" for="locale-switcher"><%= label %></label>
+      <select class="js-location-changer locale-switcher inline-block" name="locale-switcher" id="locale-switcher">
+        <%= options_for_select(language_options, current_path_with_query_params(locale: I18n.locale)) %>
+      </select>
+    </form>
+  <% else %>
+    <p id="<%= label_id %>"><%= label %></p>
+
+    <%= link_list(*language_links, "aria-labelledby": label_id) %>
+  <% end %>
 </div>
diff --git a/app/components/layout/locale_switcher_component.rb b/app/components/layout/locale_switcher_component.rb
index 1983e176b..4e7408adf 100644
--- a/app/components/layout/locale_switcher_component.rb
+++ b/app/components/layout/locale_switcher_component.rb
@@ -1,5 +1,5 @@
 class Layout::LocaleSwitcherComponent < ApplicationComponent
-  delegate :name_for_locale, :current_path_with_query_params, to: :helpers
+  delegate :name_for_locale, :link_list, :current_path_with_query_params, to: :helpers
 
   def render?
     locales.size > 1
@@ -7,13 +7,36 @@ class Layout::LocaleSwitcherComponent < ApplicationComponent
 
   private
 
+    def many_locales?
+      locales.size > 4
+    end
+
     def locales
       I18n.available_locales
     end
 
-    def language_options
+    def label
+      t("layouts.header.locale")
+    end
+
+    def label_id
+      "locale_switcher_label"
+    end
+
+    def language_links
       locales.map do |locale|
-        [name_for_locale(locale), current_path_with_query_params(locale: locale), lang: locale]
+        [
+          name_for_locale(locale),
+          current_path_with_query_params(locale: locale),
+          locale == I18n.locale,
+          lang: locale
+        ]
+      end
+    end
+
+    def language_options
+      language_links.map do |text, path, _, options|
+        [text, path, options]
       end
     end
 end
diff --git a/spec/components/layout/locale_switcher_component_spec.rb b/spec/components/layout/locale_switcher_component_spec.rb
index c4ae8b1be..5d1462e49 100644
--- a/spec/components/layout/locale_switcher_component_spec.rb
+++ b/spec/components/layout/locale_switcher_component_spec.rb
@@ -28,6 +28,7 @@ describe Layout::LocaleSwitcherComponent, type: :component do
 
       expect(page).to have_css "form"
       expect(page).to have_select "Language:", options: %w[Deutsch English Español Français Nederlands]
+      expect(page).not_to have_css "ul"
     end
 
     it "selects the current locale" do
@@ -53,4 +54,27 @@ describe Layout::LocaleSwitcherComponent, type: :component do
       end
     end
   end
+
+  context "with a few languages" do
+    before do
+      allow(I18n).to receive(:available_locales).and_return(%i[en es fr])
+    end
+
+    it "renders a list of links" do
+      render_inline component
+
+      expect(page).to have_css "ul"
+      expect(page).to have_link "English", href: "/?locale=en"
+      expect(page).to have_link "Español", href: "/?locale=es"
+      expect(page).to have_link "Français", href: "/?locale=fr"
+      expect(page).not_to have_css "form"
+    end
+
+    it "marks the current locale" do
+      render_inline component
+
+      expect(page).to have_css "[aria-current]", count: 1
+      expect(page).to have_css "[aria-current]", exact_text: "English"
+    end
+  end
 end