From 41bbf7e541eb8a739812dd43059f496e0dd89753 Mon Sep 17 00:00:00 2001
From: Jamil <jamilbk@users.noreply.github.com>
Date: Mon, 25 Sep 2023 14:29:56 -0700
Subject: [PATCH] fix(portal): sidebar active item state (#2119)

Adds `active_path` to determine whether or not to highlight a sidebar
item.

~~Leaving as draft for now to allow @devsnaked to contribute. Edit: Will
use this PR as the base for @devsnaked's upcoming changes~~

Edit: fixes #2065
---
 .github/workflows/elixir.yml                  |   6 -
 .../lib/web/components/layouts/app.html.heex  |  49 +++++-
 .../web/components/navigation_components.ex   |  17 +-
 elixir/apps/web/lib/web/nav.ex                |  12 ++
 elixir/apps/web/lib/web/router.ex             |   3 +-
 .../web/test/web/live/nav/sidebar_test.exs    | 155 ++++++++++++++++++
 6 files changed, 224 insertions(+), 18 deletions(-)
 create mode 100644 elixir/apps/web/lib/web/nav.ex
 create mode 100644 elixir/apps/web/test/web/live/nav/sidebar_test.exs

diff --git a/.github/workflows/elixir.yml b/.github/workflows/elixir.yml
index e609d3834..04f1dd044 100644
--- a/.github/workflows/elixir.yml
+++ b/.github/workflows/elixir.yml
@@ -307,12 +307,6 @@ jobs:
         options: --cap-add=IPC_LOCK
     steps:
       - uses: nanasess/setup-chromedriver@v2
-        with:
-          # XXX: This is an unfortunate workaround due to this issue:
-          # https://github.com/nanasess/setup-chromedriver/issues/199
-          # Still, it may not hurt to pin chromedriver and/or Chrome for more repeatable tests and
-          # possibly even matrix these to multiple versions to increase browser coverage.
-          chromedriver-version: '117.0.5938'
       - uses: erlef/setup-beam@v1
         with:
           otp-version: "26.0.2"
diff --git a/elixir/apps/web/lib/web/components/layouts/app.html.heex b/elixir/apps/web/lib/web/components/layouts/app.html.heex
index b4e2b3867..fca659809 100644
--- a/elixir/apps/web/lib/web/components/layouts/app.html.heex
+++ b/elixir/apps/web/lib/web/components/layouts/app.html.heex
@@ -1,44 +1,75 @@
 <.topbar subject={@subject} />
 
 <.sidebar>
-  <.sidebar_item navigate={~p"/#{@account}/dashboard"} icon="hero-chart-bar-square-solid">
+  <.sidebar_item
+    current_path={@current_path}
+    navigate={~p"/#{@account}/dashboard"}
+    icon="hero-chart-bar-square-solid"
+  >
     Dashboard
   </.sidebar_item>
 
-  <.sidebar_item navigate={~p"/#{@account}/actors"} icon="hero-user-circle-solid">
+  <.sidebar_item
+    current_path={@current_path}
+    navigate={~p"/#{@account}/actors"}
+    icon="hero-user-circle-solid"
+  >
     Actors
   </.sidebar_item>
-  <.sidebar_item navigate={~p"/#{@account}/groups"} icon="hero-user-group-solid">
+  <.sidebar_item
+    current_path={@current_path}
+    navigate={~p"/#{@account}/groups"}
+    icon="hero-user-group-solid"
+  >
     Groups
   </.sidebar_item>
-  <.sidebar_item navigate={~p"/#{@account}/clients"} icon="hero-device-phone-mobile-solid">
+  <.sidebar_item
+    current_path={@current_path}
+    navigate={~p"/#{@account}/clients"}
+    icon="hero-device-phone-mobile-solid"
+  >
     Clients
   </.sidebar_item>
 
   <.sidebar_item
+    current_path={@current_path}
     navigate={~p"/#{@account}/gateway_groups"}
     icon="hero-arrow-left-on-rectangle-solid"
   >
     Gateways
   </.sidebar_item>
 
-  <.sidebar_item navigate={~p"/#{@account}/relay_groups"} icon="hero-arrows-right-left">
+  <.sidebar_item
+    current_path={@current_path}
+    navigate={~p"/#{@account}/relay_groups"}
+    icon="hero-arrows-right-left"
+  >
     Relays
   </.sidebar_item>
 
-  <.sidebar_item navigate={~p"/#{@account}/resources"} icon="hero-server-stack-solid">
+  <.sidebar_item
+    current_path={@current_path}
+    navigate={~p"/#{@account}/resources"}
+    icon="hero-server-stack-solid"
+  >
     Resources
   </.sidebar_item>
 
-  <.sidebar_item navigate={~p"/#{@account}/policies"} icon="hero-shield-check-solid">
+  <.sidebar_item
+    current_path={@current_path}
+    navigate={~p"/#{@account}/policies"}
+    icon="hero-shield-check-solid"
+  >
     Policies
   </.sidebar_item>
 
-  <.sidebar_item_group id="settings" icon="hero-cog-solid">
+  <.sidebar_item_group current_path={@current_path} id="settings" icon="hero-cog-solid">
     <:name>Settings</:name>
 
     <:item navigate={~p"/#{@account}/settings/account"}>Account</:item>
-    <:item navigate={~p"/#{@account}/settings/identity_providers"}>Identity Providers</:item>
+    <:item navigate={~p"/#{@account}/settings/identity_providers"}>
+      Identity Providers
+    </:item>
     <:item navigate={~p"/#{@account}/settings/dns"}>DNS</:item>
   </.sidebar_item_group>
 
diff --git a/elixir/apps/web/lib/web/components/navigation_components.ex b/elixir/apps/web/lib/web/components/navigation_components.ex
index 3cfa8a6d0..8748aa8fe 100644
--- a/elixir/apps/web/lib/web/components/navigation_components.ex
+++ b/elixir/apps/web/lib/web/components/navigation_components.ex
@@ -146,6 +146,8 @@ defmodule Web.NavigationComponents do
   attr :icon, :string, required: true
   attr :navigate, :string, required: true
   slot :inner_block, required: true
+  attr :current_path, :string, required: true
+  attr :active_class, :string, required: false, default: "dark:bg-gray-700 bg-gray-100"
 
   def sidebar_item(assigns) do
     ~H"""
@@ -154,6 +156,7 @@ defmodule Web.NavigationComponents do
       flex items-center p-2
       text-base font-medium text-gray-900
       rounded-lg
+      #{String.starts_with?(@current_path, @navigate) && @active_class}
       hover:bg-gray-100
       dark:text-white dark:hover:bg-gray-700 group]}>
         <.icon name={@icon} class={~w[
@@ -170,7 +173,8 @@ defmodule Web.NavigationComponents do
 
   attr :id, :string, required: true, doc: "ID of the nav group container"
   attr :icon, :string, required: true
-  # attr :navigate, :string, required: true
+  attr :current_path, :string, required: true
+  attr :active_class, :string, required: false, default: "dark:bg-gray-700 bg-gray-100"
 
   slot :name, required: true
 
@@ -179,6 +183,13 @@ defmodule Web.NavigationComponents do
   end
 
   def sidebar_item_group(assigns) do
+    dropdown_hidden =
+      !Enum.any?(assigns.item, fn item ->
+        String.starts_with?(assigns.current_path, item.navigate)
+      end)
+
+    assigns = assign(assigns, dropdown_hidden: dropdown_hidden)
+
     ~H"""
     <li>
       <button
@@ -190,6 +201,7 @@ defmodule Web.NavigationComponents do
           hover:bg-gray-100 dark:text-white dark:hover:bg-gray-700]}
         aria-controls={"dropdown-#{@id}"}
         data-collapse-toggle={"dropdown-#{@id}"}
+        aria-hidden={@dropdown_hidden}
       >
         <.icon name={@icon} class={~w[
           w-6 h-6 text-gray-500
@@ -203,11 +215,12 @@ defmodule Web.NavigationComponents do
           group-hover:text-gray-900
           dark:text-gray-400 dark:group-hover:text-white]} />
       </button>
-      <ul id={"dropdown-#{@id}"}>
+      <ul id={"dropdown-#{@id}"} class={if @dropdown_hidden, do: "hidden", else: ""}>
         <li :for={item <- @item}>
           <.link navigate={item.navigate} class={~w[
               flex items-center p-2 pl-11 w-full group rounded-lg
               text-base font-medium text-gray-900
+              #{String.starts_with?(@current_path, item.navigate) && @active_class}
               transition duration-75
               hover:bg-gray-100 dark:text-white dark:hover:bg-gray-700]}>
             <%= render_slot(item) %>
diff --git a/elixir/apps/web/lib/web/nav.ex b/elixir/apps/web/lib/web/nav.ex
new file mode 100644
index 000000000..7f8726d57
--- /dev/null
+++ b/elixir/apps/web/lib/web/nav.ex
@@ -0,0 +1,12 @@
+defmodule Web.Nav do
+  use Web, :verified_routes
+
+  def on_mount(:set_active_sidebar_item, _params, _session, socket) do
+    {:cont,
+     Phoenix.LiveView.attach_hook(socket, :current_path, :handle_params, &set_current_path/3)}
+  end
+
+  defp set_current_path(_params, uri, socket) do
+    {:cont, Phoenix.Component.assign(socket, :current_path, URI.parse(uri).path)}
+  end
+end
diff --git a/elixir/apps/web/lib/web/router.ex b/elixir/apps/web/lib/web/router.ex
index 5e88499a8..843ccb6c1 100644
--- a/elixir/apps/web/lib/web/router.ex
+++ b/elixir/apps/web/lib/web/router.ex
@@ -95,7 +95,8 @@ defmodule Web.Router do
         Web.Sandbox,
         {Web.Auth, :ensure_authenticated},
         {Web.Auth, :ensure_account_admin_user_actor},
-        {Web.Auth, :mount_account}
+        {Web.Auth, :mount_account},
+        {Web.Nav, :set_active_sidebar_item}
       ] do
       live "/dashboard", Dashboard
 
diff --git a/elixir/apps/web/test/web/live/nav/sidebar_test.exs b/elixir/apps/web/test/web/live/nav/sidebar_test.exs
new file mode 100644
index 000000000..21248810a
--- /dev/null
+++ b/elixir/apps/web/test/web/live/nav/sidebar_test.exs
@@ -0,0 +1,155 @@
+defmodule Web.Live.Nav.SidebarTest do
+  use Web.ConnCase, async: true
+
+  setup do
+    account = Fixtures.Accounts.create_account()
+    actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account)
+    identity = Fixtures.Auth.create_identity(account: account, actor: actor)
+
+    %{
+      account: account,
+      actor: actor,
+      identity: identity
+    }
+  end
+
+  test "hides dropdown when path is not within dropdown children", %{
+    conn: conn,
+    account: account,
+    identity: identity
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/actors")
+    refute Enum.empty?(Floki.find(html, "ul#dropdown-settings.hidden"))
+  end
+
+  test "shows dropdown when path is within dropdown children", %{
+    conn: conn,
+    account: account,
+    identity: identity
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/settings/dns")
+    assert Enum.empty?(Floki.find(html, "ul#dropdown-settings.hidden"))
+    refute Enum.empty?(Floki.find(html, "ul#dropdown-settings"))
+  end
+
+  test "renders proper active sidebar item class for actors", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/actors")
+    assert item = Floki.find(html, "a.bg-gray-100[href='/#{account.id}/actors']")
+    assert String.trim(Floki.text(item)) == "Actors"
+  end
+
+  test "renders proper active sidebar item class for groups", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/groups")
+    assert item = Floki.find(html, "a.bg-gray-100[href='/#{account.id}/groups']")
+    assert String.trim(Floki.text(item)) == "Groups"
+  end
+
+  test "renders proper active sidebar item class for clients", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/clients")
+    assert item = Floki.find(html, "a.bg-gray-100[href='/#{account.id}/clients']")
+    assert String.trim(Floki.text(item)) == "Clients"
+  end
+
+  test "renders proper active sidebar item class for gateways", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/gateway_groups")
+    assert item = Floki.find(html, "a.bg-gray-100[href='/#{account.id}/gateway_groups']")
+    assert String.trim(Floki.text(item)) == "Gateways"
+  end
+
+  test "renders proper active sidebar item class for relays", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/relay_groups")
+    assert item = Floki.find(html, "a.bg-gray-100[href='/#{account.id}/relay_groups']")
+    assert String.trim(Floki.text(item)) == "Relays"
+  end
+
+  test "renders proper active sidebar item class for resources", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/resources")
+    assert item = Floki.find(html, "a.bg-gray-100[href='/#{account.id}/resources']")
+    assert String.trim(Floki.text(item)) == "Resources"
+  end
+
+  test "renders proper active sidebar item class for policies", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/policies")
+    assert item = Floki.find(html, "a.bg-gray-100[href='/#{account.id}/policies']")
+    assert String.trim(Floki.text(item)) == "Policies"
+  end
+
+  test "renders proper active sidebar item class for account", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/settings/account")
+    assert item = Floki.find(html, "a.bg-gray-100[href='/#{account.id}/settings/account']")
+    assert String.trim(Floki.text(item)) == "Account"
+  end
+
+  test "renders proper active sidebar item class for identity providers", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} =
+      live(authorize_conn(conn, identity), ~p"/#{account}/settings/identity_providers")
+
+    assert item =
+             Floki.find(html, "a.bg-gray-100[href='/#{account.id}/settings/identity_providers']")
+
+    assert String.trim(Floki.text(item)) == "Identity Providers"
+  end
+
+  test "renders proper active sidebar item class for new OIDC identity provider", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} =
+      live(
+        authorize_conn(conn, identity),
+        ~p"/#{account}/settings/identity_providers/openid_connect/new"
+      )
+
+    assert item =
+             Floki.find(html, "a.bg-gray-100[href='/#{account.id}/settings/identity_providers']")
+
+    assert String.trim(Floki.text(item)) == "Identity Providers"
+  end
+
+  test "renders proper active sidebar item class for dns", %{
+    account: account,
+    identity: identity,
+    conn: conn
+  } do
+    {:ok, _lv, html} = live(authorize_conn(conn, identity), ~p"/#{account}/settings/dns")
+    assert item = Floki.find(html, "a.bg-gray-100[href='/#{account.id}/settings/dns']")
+    assert String.trim(Floki.text(item)) == "DNS"
+  end
+end