Form label not rendering correctly

[justinlittman]

Steps to reproduce

Given a component in which a form builder is being passed in and is being used to render a form label:

<%= form.label field_name, class: classes do %>
  <%= content || label_text %>
<% end %>

This is the actual component:

• https://github.com/sul-dlss/hungry-hungry-hippo/blob/upgrade_view_components/app/components/elements/forms/label_component.rb
• https://github.com/sul-dlss/hungry-hungry-hippo/blob/upgrade_view_components/app/components/elements/forms/label_component.html.erb

Here is a failing test:
https://github.com/sul-dlss/hungry-hungry-hippo/blob/upgrade_view_components/spec/components/elements/forms/label_component_spec.rb

which fails with:

  1) Elements::Forms::LabelComponent when label text is not provided creates label using label_text
     Failure/Error: expect(page).to have_css('label.form-label:not(.visually-hidden)', text: 'my label')
       expected to find visible css "label.form-label:not(.visually-hidden)" with text "my label" but there were no matches. Also found "\n", which matched the selector but not all filters.
     # ./spec/components/elements/forms/label_component_spec.rb:20:in `block (3 levels) in <top (required)>'

  2) Elements::Forms::LabelComponent when label is hidden creates label with hidden label
     Failure/Error: expect(page).to have_css('label.form-label.visually-hidden', text: field_name.to_s)
       expected to find visible css "label.form-label.visually-hidden" with text "use_citation" but there were no matches. Also found "\n", which matched the selector but not all filters.
     # ./spec/components/elements/forms/label_component_spec.rb:35:in `block (3 levels) in <top (required)>'

  3) Elements::Forms::LabelComponent when content is provided creates label using content
     Failure/Error: expect(page).to have_css('label.form-label:not(.visually-hidden)', text: 'my label')
       expected to find visible css "label.form-label:not(.visually-hidden)" with text "my label" but there were no matches. Also found "\n", which matched the selector but not all filters.
     # ./spec/components/elements/forms/label_component_spec.rb:27:in `block (3 levels) in <top (required)>'

  4) Elements::Forms::LabelComponent when label text or content is not provided creates label using field name
     Failure/Error: expect(page).to have_css('label.form-label:not(.visually-hidden)', text: field_name.to_s)
       expected to find visible css "label.form-label:not(.visually-hidden)" with text "use_citation" but there were no matches. Also found "\n", which matched the selector but not all filters.
     # ./spec/components/elements/forms/label_component_spec.rb:13:in `block (3 levels) in <top (required)>'

Expected behavior

The content or label text is rendered inside the label tag.

<label class="form-label" for="license">license</label>

Actual behavior

The content or label text is rendering outside the label tag.

license<label class="form-label" for="license\"></label>

Note that this previously worked in v3 with the compatibility patch enabled.

Interestingly, if the variables within the label are replaced with literal text, it renders correctly.

So:

<%= form.label field_name, class: classes do %>
  license
<% end %>

renders the expected.

System configuration

Rails version:
8.0.2

Ruby version:
3.3.4

Gem version:
4.0.0

[joelhawksley]

Thanks for the report! @camertron is going to have a look at this ❤️

[camertron]

Hey @justinlittman, thanks for the bug report! I've been able to dig in a bit and I think I've identified the issue. Creating an instance of FormBuilder manually and passing it a view context is a pretty advanced use-case and not one we've really designed for. Here's what's going on.

The problem

As you identified, part of the v4 release was to stop inheriting from ActionView::Base, which also obviated the need for the capture compatibility hack that made ViewComponent play more nicely with, among other things, Rails form helpers. The big idea was to re-use the view context (i.e. the instance of ActionView::Base) that Rails creates, rather than creating one per component render. Multiple view contexts confuse the form helpers because they retain a reference to the view context in which they were created, as your test demonstrates:

let(:form) { ActionView::Helpers::FormBuilder.new(nil, related_work_form, vc_test_controller.view_context, {}) }

Since each view context creates its own output buffer, you can get all kinds of funny out-of-order rendering issues depending on which view context appends to which buffer when.

Ok, so why is all that relevant? Well, since out-of-order rendering issues are caused by multiple view contexts, it follows that multiple view contexts are getting created in your test. I added some puts statements in ActionView::Base#initialize and confirmed the output appears twice. We can see why if we look at the implementation of the view_context method in lib/action_view/rendering.rb:

def view_context
  view_context_class.new(lookup_context, view_assigns, self)
end

Notice the complete lack of any memoization. That means a new view context gets created whenever you call vc_test_controller.view_context and, crucially, does not get re-used by the ViewComponent test helpers. This is because the test helpers also call vc_test_controller.view_context and mint a brand new view context of their own.

So, why not memoize the view context? Because render_inline can get called multiple times per test. The view context should be thrown away at the end of each render so the output from the previous render doesn't interfere with assertions run on the output from the current render.

The solution

Ok, so that's the bad news. The good news: it should be fairly straightforward to fix, although it will require a small change to ViewComponent itself. I'm proposing the test helpers define another method, vc_test_view_context, that will allow tests and the VC test helpers to refer to a shared view context. However, the shared view context will be reset after each call to render_inline, meaning you'll need to re-create any FormBuilder instances if you want to call render_inline multiple times in a test.

Here's a PR: #2410

[justinlittman]

Awesome. I can verify that fixed all of my component specs. (Now, on to my feature specs, which I suspect will require a bunch of prefixing with helpers..)

[doits]

Just want to tune in the problem is not only present in test but for me in development, too.

I have a component that renders a shared view like this:

# component.html.erb

<%= form_with(model: ...) do |form| %>
  <%= render 'shared/input', form:, attr: 'something' do %>
    <% form.text_field :something %>
  <% end %>
<% end %>

# shared/_input.html.erb

<%= form.label attr do %>
  Generate some label here

  <%= yield %>
<% end %>

It should generate:

<label>
  Generate some label here

  <input type="text" ...>...</input>
</label>

but instead it generates

Generate some label here

<label>
  <input type="text" ...>...</input>
</label>

Looks like the yield is placed wrong.

It worked correctly on v3.x with config.view_component.capture_compatibility_patch_enabled = true. Broke with upgrade to v4.

Rails version:
8.0.2

Ruby version:
3.3.5

Gem version:
4.0.0

[doits]

A minimal reproducible example (without forms):

# app/components/example_component.rb

class ExampleComponent < ViewComponent::Base
end

<%# app/components/example_component.html.erb %>

<%= render "shared/partial" do %>
  abc
<% end %>

<%# app/views/shared/_partial.html.erb %>

def <%= yield %>

And output (with annotations by rails) is:

<!-- BEGIN app/components/example_component.html.erb -->
  abc
<!-- BEGIN app/views/shared/_partial.html.erb -->def 
  abc

<!-- END app/views/shared/_partial.html.erb --><!-- END app/components/example_component.html.erb -->

abc (component html) is output twice 🤔

Afaik it should be more like (not sure about line breaks):

<!-- BEGIN app/components/example_component.html.erb -->

<!-- BEGIN app/views/shared/_partial.html.erb -->def 
  abc

<!-- END app/views/shared/_partial.html.erb --><!-- END app/components/example_component.html.erb -->

[camertron]

Hey @doits, thanks for bringing this up. I was able to reproduce the problem, but I don't immediately understand why it's happening and I don't think it's related to the OP's issue. I've filed this new issue instead: #2423

[robbevp]

I'm still noticing some related issues when using form.button inside the component. Note that this never really worked properly in 3.x either - I had a comment from months ago to try and figure this out, but never got around to it.

The following component renders differently in dev/test:

<div>
  <%= form.button :foo, value: :bar, class: '' do %>
    Bar
  <% end %>
  <%= form.button :foo, value: :baz, class: '' do %>
    <%= "baz" %>
  <% end %>
  <%= form.button :foo, value: :foobar, class: '' do %>
    <% "foobar" %>
  <% end %>
</div>

Results in the following html in when testing the component directly:

<div>
    Bar
<button name="user[foo]" type="submit" id="user_foo" value="bar" class="">
    Bar
</button>  
    baz
<button name="user[foo]" type="submit" id="user_foo" value="baz" class="">
</button>  
<button name="user[foo]" type="submit" id="user_foo" value="foobar" class="">foobar</button>
</div>

But renders correctly in dev:

<div>
  <button name="user[foo]" type="submit" id="user_foo" value="bar" class="">
    Bar
</button>
<button name="user[foo]" type="submit" id="user_foo" value="baz" class="">
    baz
</button>
<button name="user[foo]" type="submit" id="user_foo" value="foobar" class="">foobar</button>
</div>

We worked around this by not printing the tags inside form.button. I'm happy to provide a more complete reproduction or move this to a different issue