Proposal: Adding LiveView Component to Heroicons #1
Conversation
There was a problem hiding this comment.
Thank you for this PR!
I've been thinking about doing this as well so I really appreciate it.
I'm trying to figure out what the situation is where I want a component in a LiveView app for these. E.g. in my LiveView app I do
<%= Heroicons.Solid.server(class: "ml-6 w-12 h-12 text-green-400" %>
What does adding live components buy us? We're now being more verbose as
<%= live_component @socket, Heroicons.LiveComponent, type: :outline, icon: :server, class: "ml-6 w-12 h-12 text-green-400" %>
We could gain some more efficient diffing, e.g if you change the class in a diff a component would be able to send a smaller diff, but I think we'd need to get the SVG into the render function with the classes as part of the assigns for that to work.
@bmalum were you writing LiveView code where having a component would have been really helpful?
|
|
||
| def render(%{type: :outline, class: class, icon: icon} = assigns) do | ||
| ~L""" | ||
| <%= apply(Elixir.Heroicons.Outline, icon, [%{class: class}]) |> Phoenix.HTML.raw %> |
There was a problem hiding this comment.
I think the functions return html safe data, is the call to raw necessary?
There was a problem hiding this comment.
good point - you are right - I updated this part.
Signed-off-by: Martin Karrer <martin@bmalum.com>
|
I usually also do use Maybe I'm wrong but I thought to get a minimal diff as possible, best way is to use components and |
|
That makes sense! I have a feeling that as written the component diffs would be the same and would have the entire SVG in it even if you're updating just the class. |
|
I think the stateless function components (phoenixframework/phoenix_live_view#1428) are exactly what we need here. I'm going to close this in favor or #3 |
Return to compile-time generation
First of all - thank you for providing the base package. I did almost the same, for my component library and then noticed your hex package.
I decided to added my (tbh very small part) to your package. I can fully understand if you think that this should not be included in your package, because it adds more dependencies.
Let me know if there is something to improve!