fix(portal): sidebar active item state #2119
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
jamilbk
force-pushed
the
jamilbk-fix/sidebar-active-state
branch
from
6718d7c
to
46234a4
Compare
| @@ -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 :active_path, :string, required: true | |||
There was a problem hiding this comment.
Maybe we should call it current_path?
| @@ -154,6 +156,7 @@ defmodule Web.NavigationComponents do | |||
| flex items-center p-2 | |||
| text-base font-medium text-gray-900 | |||
| rounded-lg | |||
| #{if String.starts_with?(@active_path, @navigate), do: @active_class, else: ""} | |||
| hover:bg-gray-100 | |||
| dark:text-white dark:hover:bg-gray-700 group]}> | |||
There was a problem hiding this comment.
I think for state changes we should move them out of ~H and do something like:
activity_class =
if String.starts_with?(assigns.current_path, assigns.navigate) do
assigns.active_class
end
assigns = assign(assigns, activity_class: activity_class)
And then to make classes more readable we can leverage list of strings instead of sigil (I noticed Elixir formatted makes some weird moves with it so was trying to avoid it in new code):
~H"""
... class={[
"flex items-center p-2",
"text-base font-medium text-gray-900",
"rounded-lg",
@activity_class
]}
"""
There was a problem hiding this comment.
👍 Will get this feedback applied
There was a problem hiding this comment.
Or we can do like Phoenix team does:
class={[
"flex items-center p-2",
"text-base font-medium text-gray-900",
"rounded-lg",
String.starts_with?(@current_path, @navigate) && @active_class,
]}
There was a problem hiding this comment.
I think the second one might end up being cleaner because we need the particular navigate attribute for each item inside the comprehension
|
@AndrewDryga fixed. |
Adds
active_pathto 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 changesEdit: fixes #2065