komposable / komponent

An opinionated way of organizing front-end code in Ruby on Rails, based on components
http://komponent.io
MIT License
427 stars 31 forks source link

Component rendered twice #83

Closed slecorvaisier closed 6 years ago

slecorvaisier commented 6 years ago

Hi

I came across a situation where a component will render itself twice, the second rendering being nested in the first one (probably considered as a block). It happens when the component is using itself a rails block, like a field_wrapper.

Here is a gist: https://gist.github.com/slecorvaisier/99336ed2b49652d5e192e856dcf0295e

And a screenshot of the DOM:

screen shot 2018-03-30 at 14 50 05

Does Komponent work nicely with .slim views ?

Thanks in advance for your help

Spone commented 6 years ago

Does Komponent work nicely with .slim views ?

Quick reply to this: yes, I use it mainly with Slim personally.

We'll investigate the issue, thanks for the report.

slecorvaisier commented 6 years ago

Thanks! We just noticed the component is rendering properly without the f.field_wrapper ... do

Gist updated with the form helper

florentferry commented 6 years ago

Hello @slecorvaisier,

Is your issue resolved ? I can't reproduce your problem with my simple use case from what you described here. If not resolved, can you provide the komponent version you are using.

= wrapper do
 = component("example")
def wrapper(&block)
  content_tag :div, do
    capture(&block)
  end
end
slecorvaisier commented 6 years ago

Hi @florentferry I'm using v1.1.4

Your example is indeed working but doesn't reflect my case as wrapper should be IN the component. (BTW having a comma after :div triggers an error)

= component "example"
// example component
= wrapper do
  = "Example"

This example also works. After some more investigation it seems the issue is related to the rails form_for helper and FormBuilder.

= form_for(@model, url: '#', method: :post) do |f|
  = component "example", f: f
  // example component
  = wrapper do
    = 'First example'

  = @f.field_wrapper do
    = 'Second example'
class CustomFormBuilder < ActionView::Helpers::FormBuilder
  def field_wrapper(&block)
    @template.content_tag(:div, @template.capture(&block))
  end
end

if First example => will be rendered once if Second example => twice

florentferry commented 6 years ago

Hello,

There are only one case where the behavior is not expected, when block is given to a form builder helper. I investigate into it, don't know why this behavior happens here. But you can bypass the behavior:

class CustomFormBuilder < ActionView::Helpers::FormBuilder
  def field_wrapper(&block)
    @template.content_tag(:div, class: 'field') do
      block.call
    end
  end

  def text_field(attr, options = {})
    field_wrapper do
      super(attr, options)
    end
  end
end
= form_for do |form|
 = component('text_field', form: form, attribute: :text)
_text_field.html.slim

= @f.text_field @attribute
pcriv commented 6 years ago

This scenario also shows duplicated content:

class CustomFormBuilder < ActionView::Helpers::FormBuilder
  def custom_select_datetime(datetime = Time.current, options = {}, html_options = {})
    yield ActionView::Helpers::DateTimeSelector.new(datetime, options, html_options)
  end
end
= f.custom_select_datetime(Time.current, options, class: "form-control")do |select|
  .select-date
    .select-wrapper
      = select.select_day
    .select-wrapper
      = select.select_month
    .select-wrapper
      = select.select_year
Spone commented 6 years ago

I think Komponent is not compatible with custom FormBuilders for now.

Personally, I use it with form_for:

= form_for [:admin, @item], html: {class: "admin-form"} do |f|
  = c "admin/heading", title: page_title
    = c "admin/button", text: "Back", href: @index_path, modifiers: "is-secondary as-link"
    = c "admin/button", icon: "save", text: @item.new_record? ? "Create" : "Save", type: "submit", modifiers: "is-primary has-outline is-large"

  = c "admin/form/group", label: "Title"
    = f.text_field :title

  = c "admin/form/group", label: "Description", help: "visible in search engines and social networks shares"
    = f.text_area :description

So the components are only wrappers around the fields.

I'll try to have a look at using a custom FormBuilder.

pcriv commented 6 years ago

That is an interesting approach! Thanks for sharing @Spone!

florentferry commented 6 years ago

Seems this issue has been resolved by a workaround, feel free to reopen if that solution is not appropriate for your use case.