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.

Sign up for GitHub

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

Jump to bottom

Collection field components with block support #124

Open
rowofpixels opened this issue Apr 27, 2022 · 9 comments · May be fixed by #148
Open

Collection field components with block support #124

rowofpixels opened this issue Apr 27, 2022 · 9 comments · May be fixed by #148
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@rowofpixels
Copy link
Contributor

I saw this comment in a past PR: #29 (comment) about adding collections without the block support first. I read through a number of issues in https://github.com/github/view_component related to form fields and it seems like it's not a simple problem to solve.

I'm curious whether there is a plan to support these fields when used with a block like so:

<%= f.collection_check_boxes :ids, MyModel.all, :id, :name do |b| %>
  <%= b.check_box %>
  <%= b.label %>
<% end %>

Right now the issue is that b is the component object rather than a builder object. I'm not sure what the right approach would be in a view component form builder.
@nicolas-brousse nicolas-brousse added the enhancement New feature or request label Apr 27, 2022
@nicolas-brousse
Copy link
Member

We did something similar for LabelComponent but we missed it on collections.

For LabelComponent we did this:

view_component-form/app/components/view_component/form/label_component.rb

Lines 31 to 39 in 2803648

# See: https://github.com/rails/rails/blob/83217025a171593547d1268651b446d3533e2019/actionview/lib/action_view/helpers/tags/label.rb#L48
def builder
@builder ||= begin
tag_value = options.delete("value")
ActionView::Helpers::Tags::Label::LabelBuilder.new(@view_context, object_name, method_name, object, tag_value)
end
end
delegate :translation, to: :builder

view_component-form/spec/view_component/form/label_component_spec.rb

Lines 45 to 72 in d38c459

context "with a block and translation param" do
let(:block) do
proc do |component|
"<span class=\"translated-label\">#{component.translation}</span>".html_safe
end
end
it do
expect(component).to eq_html <<~HTML
<label for="user_first_name"><span class="translated-label">First name</span></label>
HTML
end
end
context "with a block and builder param" do
let(:block) do
proc do |component|
"<span class=\"translated-label#{" has-error" if component.builder.object.errors.include?(:first_name)}\">" \
"#{component.builder.translation}</span>".html_safe
end
end
it do
expect(component).to eq_html <<~HTML
<label for="user_first_name"><span class="translated-label">First name</span></label>
HTML
end
end

@nicolas-brousse nicolas-brousse self-assigned this Apr 27, 2022
@nicolas-brousse nicolas-brousse removed their assignment May 5, 2022
@nicolas-brousse nicolas-brousse added the good first issue Good for newcomers label Jun 28, 2022
@wooly
Copy link
Contributor

wooly commented Mar 23, 2023

We've just come up against this today and it would be super useful to have. I started poking around to see if I could thread things through, but I can't quite wrap my head around it as in the collection checkbox case there's a new builder instantiated for each item in the collection here: https://github.com/rails/rails/blob/83217025a171593547d1268651b446d3533e2019/actionview/lib/action_view/helpers/tags/collection_helpers.rb#L89 which I'm guessing would need to be yielded somehow. Will try to workaround it for now.

@Spone
Copy link
Collaborator

Spone commented Mar 23, 2023

@wooly I don't know if you're aware of the new compatibility module available in ViewComponent, to fix problems with blocks that we had in the past: ViewComponent/view_component#1650

Let us know if you figure out a workaround, it will be useful to fix this issue. And feel free to open a PR if you have time!

@wooly
Copy link
Contributor

wooly commented Mar 23, 2023 •
edited
Loading

I managed to get it working, but not in a super-idiomatic way. I've pushed up an example PR here: #142

The idea is to pass an additional proc option to use as the block for the render call, then strip that out of the options prior to render so the component can be initialized with the options-less-proc hash, then pass the proc as the block to render.

@wooly
Copy link
Contributor

wooly commented Apr 3, 2023

@nicolas-brousse any feedback on that PR to validate that it's the right approach?

@nicolas-brousse
Copy link
Member

Hi @wooly, Thank you for your interest in this.
I'll try to found a bit of time this week or the next one to have a look on this.

@wooly
Copy link
Contributor

wooly commented Apr 3, 2023

Thanks! I’m happy to knock it into shape with any feedback and flesh out some specs too.

@Spone Spone linked a pull request May 1, 2023 that will close this issue
Draft
@Spone
Copy link
Collaborator

Spone commented May 1, 2023

@wooly 👋 we reviewed your PR. We spent some time trying to find a way closer to Rails' syntax, but it seems to be quite a difficult task. For now, let's go with your suggestion!

@coder2000
Copy link

coder2000 commented Dec 26, 2023 •
edited
Loading

I found a need for this solution in my application and I used code similar to #148 for my radio buttons. I didn't need to change my view at all and it works just the same. Only issue I have found is the text fields while I call titleize on them in my view appear as all lower case letters.

I was wrong. The code I was using was essentially the default collection with no block rendering. I think the issue is calling the components in the block properly because I get parameter errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support for block in collections (checkboxes & radio buttons) pantographe/view_component-form
5 participants
@coder2000 @Spone @wooly @nicolas-brousse @rowofpixels