Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Surface.component macro #109

Closed
lnr0626 opened this issue Aug 11, 2020 · 16 comments
Closed

Surface.component macro #109

lnr0626 opened this issue Aug 11, 2020 · 16 comments

Comments

@lnr0626
Copy link
Collaborator

lnr0626 commented Aug 11, 2020

With #15 wrapped up, I think we have most of the things we need to support a Surface.component macro that could be used inside LEEx. The generated code should look somewhat similar to https://github.com/msaraiva/surface/blob/master/lib/surface/compiler/eex_engine.ex#L212

I think it would be worthwhile to think through the usage a bit though. The simplest approach would be to have something like:

<%= Surface.component(@socket, MyComponent, [prop1: @value, prop2: @value], """
<!-- contents for MyComponent -->
""" %>

which would be equivalent to:

<MyComponent prop1={{ @value }} prop2={{ @value }}>
    <!-- contents for MyComponent -->
</MyComponent>

However, something that could be used more like the following would probably be more natural within an LEEx template:

<%= Surface.component(@socket, MyComponent, [prop1: @value, prop2: @value]) do %>
<!-- contents for MyComponent -->
<% end %>
@msaraiva
Copy link
Member

Yeah, for the default slot it would be as natural as using live_component, however, handling named slots will be tricky.

@msaraiva
Copy link
Member

This feature will also allow users to use the existing render_component/3 macro in tests as described here.

Currently, users need to either define a parent LiveView in their tests as in: https://github.com/msaraiva/surface/blob/011ff48401f6b3659f03fbd6a92e7028b0173547/test/live_component_test.exs#L168

or use our render_live/2 macro (which creates one on the fly) as in: https://github.com/msaraiva/surface/blob/011ff48401f6b3659f03fbd6a92e7028b0173547/test/live_component_test.exs#L123

The latter requires users to copy & paste the helper macro from our test_helper.exs which is suboptimal.

With Surface.component ready, we could define our own render_surface_component/3 wrapping render_component/3 adding support for slots and contexts.

@aleDsz
Copy link

aleDsz commented Nov 30, 2020

Using this approach, I'm creating and component library for a design system using Surface to provide a more easy way to develop web application with Phoenix LiveView.

So, I'm trying to make a way to test component renderization using render_component/3 or render_live/2 but not what I'm expecting to see.

I tried to use the same tests that you have on Surface but It's not what I want.

For now, I'm trying this way:

test "returns Button component without button properties" do
  html = """
  <button class="a-btn">Sample</button>
  """

  assert render_component(Button, %{children: "Sample"}) =~ html
end

source

I don't know if it's the better way to check if it's rendering the inner content as this test purpose.

@msaraiva
Copy link
Member

Hey @aleDsz!

If i understood correctly, you're using children to pass markup to be used as the body of the button. If so, you should be using slots. So in Astro.SurfaceComponent, instead of:

data children, :any

you should have:

slot default

and the Button component should render this:

  def render(assigns) do
    ~H"""
    <button class={{ @classes }}><slot/></button>
    """
  end

Also, make sure you define the classes prop as :css_class instead of :string. This way you can make use of some nice syntactic sugar when passing class values and avoid lots of conditionals in your code.

Now, as explained in the previous comment, render_component/2 will not work with components defining slots. The minimum you'd need to make it work is defining a helper function like this one:

  def init_surface(inner_block, assigns \\ []) do
    wrapped_block = fn _, _ -> inner_block end
    surface_assigns = %{provided_templates: [:__default__]}

    assigns
    |> Map.new()
    |> Map.put(:inner_block, wrapped_block)
    |> Map.put(:__surface__, surface_assigns)
  end

And use it as:

test "returns Button component without button properties" do
 html = """
 <button class="a-btn">Sample</button> 
 """

 assigns = %{}
 code = ~H"Sample"

 assert render_component(Button, init_surface(code)) =~ html
end

If you need, you can pass any additional prop using:

render_component(Button, init_surface(code, class: "some_class")) =~ html

Pay attention that this helper will work only for default slots. Named slots will require more work. If all your components have only default slots, this should to the trick for now until we have a final solution.

@aleDsz
Copy link

aleDsz commented Nov 30, 2020

Hi @msaraiva, thanks for your response!

Interesting, I'm also providing a test case for Components which i can simply provide a function that make easy to test those components.

I'll try today and any problem I'll post here.

Also, If I'm trying to generate class automatically, so I'll need to use :css_class instead o :string for my prop classes, right? So, how can I get all defined properties from my Component to compile all of it into classes?

@msaraiva
Copy link
Member

msaraiva commented Dec 3, 2020

how can I get all defined properties from my Component to compile all of it into classes?

@aleDsz if the module is already closed (compiled), you can use module.__props__(). If the module is still open, you need to use Module.get_attribute(module, :prop). In case you're using the latter, make sure your code runs after all props have been defined.

@aleDsz
Copy link

aleDsz commented Dec 3, 2020

@aleDsz if the module is already closed (compiled), you can use module.__props__(). If the module is still open, you need to use Module.get_attribute(module, :prop). In case you're using the latter, make sure your code runs after all props have been defined.

Oh, interesting. I was using module.__get_props__() and didn't notice that Module have module.__props__(). I'll try it later.
Thanks @msaraiva

@msaraiva
Copy link
Member

msaraiva commented Dec 4, 2020

Proposal for passing slotables when using vanilla LV (from the discussion on slack):

<%= surface_component @socket, MyComponent, prop1: @value do %>
  <% template(:header) -> %>  
    <div class="header">
      ...
    </div>
  <!-- We could have `:default` as default value so you could use just `template()` -->
  <% template(:default) -> %>
    <div class="header">
      ...
    </div>
  <% template(:footer) -> %>  
    <div class="footer">
      ...
    </div>
<% end %>

@ohmree
Copy link

ohmree commented Jan 10, 2021

Are there plans to enable writing and using components using the jsx-like syntax in separate files instead of embedding them in ~H sigils?

For me this has the same advantage as having separate (l)eex files for templates - my editor has better support for them.
I know it's a bit subjective, but I also like the jsx-like syntax better than the livecomponents-style function call syntax (<MyComponent prop={{ @value }} /> vs <%= component(@socket, MyComponent, prop: @value) %>)

@lnr0626
Copy link
Collaborator Author

lnr0626 commented Jan 11, 2021

Hi @ohmree,

Unless I'm misunderstanding what you're asking for, this is already supported - you can move the code to a file with the same base name using the sface extension, and delete the render function (similar to co-located live view templates). Eg:

lib/my_app_web/my_component.ex:

defmodule MyAppWeb.MyComponent do

  use Surface.Comonent

  prop my_prop, :any

end

lib/my_app_web/my_component.sface:

<div>...</div>

It works the same for Surface.LiveComponent and Surface.LiveView

@ohmree
Copy link

ohmree commented Jan 11, 2021

@lnr0626 This is exactly what I meant, thanks!

Just out of interest, is this documented anywhere?
Because I've looked at surface in the past and didn't realize you could do that.

@msaraiva
Copy link
Member

Just out of interest, is this documented anywhere?

Hi @ohmree!

There’s a small "Colocated templates” section in http://surface-demo.msaraiva.io/components_basics.

@msaraiva
Copy link
Member

I'm closing this one as it's not a priority and the requirements may change very soon. We can reevaluate it after Phoniex 1.6 is released.

@lastobelus
Copy link

lastobelus commented Apr 3, 2021

@msaraiva This issue leaves me somewhat confused as to how I would go about starting to use SurfaceUI in an existing Phoenix LiveVew App.

Would the only way be to convert all my existing "*.html.leex" templates to .sface syntax (for conditional rendering, loops etc.)?

@msaraiva
Copy link
Member

msaraiva commented Apr 5, 2021

Hi @lastobelus!

Would the only way be to convert all my existing "*.html.leex" templates to .sface syntax (for conditional rendering, loops etc.)?

Not all. But until phoenix has built-in support for features like slots and contexts, you need to convert at least those that include components that use any of those features. Other simpler components might work without any issue. We'll update the docs to make that clear. See surface-ui/surface_site#25.

@lastobelus
Copy link

@msaraiva I've had a go at translating phx.gen.live to a Surface version, here.

It's just a straight translation, no attempt to further decompose into components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants