pantographe / view_component-form

Rails FormBuilder for ViewComponent
MIT License
206 stars 16 forks source link

Make it possible to override form tag helper output using a builder #82

Closed boardfish closed 2 years ago

boardfish commented 2 years ago

Closes #81

The aim of this PR will be to make it so that you can pass a form builder class to a tag helper to render the corresponding component. For example:

text_field_tag 'name', 'Simon', builder: ViewComponent::Form::Builder.for_tag

will call text_field on ViewComponent::Form::Builder.for_tag and pass the arguments correctly in order to render ViewComponent::Form::TextFieldComponent.

boardfish commented 2 years ago

I wasn't able to get the tests running locally. When I run rake spec, the following is returned:


An error occurred while loading spec_helper.
Failure/Error:
  Combustion.initialize! :action_controller, :action_view, :action_text do
    config.logger = ActiveSupport::TaggedLogging.new(Logger.new(nil))
    config.log_level = :fatal
  end

NameError:
  uninitialized constant Combustion::Bundler
# ./spec/spec_helper.rb:19:in `<top (required)>'
No examples found.
No examples found.

Finished in 0.00003 seconds (files took 1.61 seconds to load)
0 examples, 0 failures, 1 error occurred outside of examples

Finished in 0.00003 seconds (files took 1.61 seconds to load)
0 examples, 0 failures, 1 error occurred outside of examples

EDIT: Using the bundled rspec worked - bundle exec rspec.

boardfish commented 2 years ago

We don't currently have a component mapping for these, so their tests are failing:

hidden_field_tag
submit_tag
button_tag
image_submit_tag
field_set_tag

The last of these is particularly interesting: fieldset isn't something I've used personally yet, but it's got a lot of utility in allowing form fields to exist outside of a parent form tag.

Buttons and submit tags are some of the more interesting items here - if you check out Primer's implementation, you'll notice that it accepts a type. It's still visually a button in any case, but the rendered HTML differs - a button tag, an input type=submit tag, or an input type=reset (I assume). That's where the ComponentMapping class ought to get more interesting - it'd be good for it to accept lambdas so that we can pre-build components, similar to lambda slots in ViewComponent itself.

I suppose the default behavior, though, should be to assume that the user will configure that, unless we plan on adding a button component to the library built into the gem?

boardfish commented 2 years ago

I think I might've actually taken the wrong approach here. fields_for with the builder might actually have the same effect, assuming ViewComponent overcame whatever was causing problems with passing a form builder instance around. But I suppose there are still instances where folks might want to render fields without any model backing whatsoever, e.g. within a component, so perhaps it's still a valid concern.

Spone commented 2 years ago

:wave: @boardfish I think you're going much faster than us here :smile:

Would you be interested in a short call to discuss our options here? Please reach out

boardfish commented 2 years ago

Right, I think I've cracked it! There's actually nothing we need to do here. The fields helper is actually spot on for what I'm looking for here - it lets you use a builder without a backing model if you wish.

  <%= fields builder: DefaultFormBuilder do |f| %>
    <%= f.text_field :start_time %>
    <%= f.text_field :end_time %>
    <%= f.text_field :resources, multiple: true %>
  <% end %>

I'm a little surprised I'd never heard of it in the first instance, but here we are. Nothing needs to be done to solve this :sparkles: