x-govuk/govuk-form-builder

Conditional reveal behaviour change between Rails 7.0 and Rails 7.1

Closed this issue · 4 comments

I'm not sure if this is something where we would want to change how the gem works in 7.1 or not, but when upgrading to Rails 7.1 from Rails 7 we've found our conditional reveals break, specifically because we have a (SLIM) partial that does:

= form.govuk_radio_button field_name, ...
  = yield if some_condition

This works fine if some_condition is true, but when it is false we end up with the equivalent of:

= form.govuk_radio_button field_name, ...
  = nil

That = nil is treated differently in Rails 7.1 to 7.0. Specifically, suppose we have a typical 'Yes'/'No' radio button with a conditional reveal attached to the 'Yes'. In Rails 7.1, with an effective = nil passed to the 'No' option, we end up, very surprisingly, with this:

Screenshot 2023-10-10 at 10 59 16

Excitingly, the 'Yes' option is re-rendered as the conditional reveal of the 'No' option!

Sorry I missed this, I'll look into it right away!

I tried to recreate in Erb and with the following form I'm getting the correct behaviour with Rails 7.1.2:

<%= form_with(model: employee) do |form| %>
  <%= form.govuk_text_field :name %>

  <%= form.govuk_radio_buttons_fieldset(:department_id, legend: { text: "Department" }) do %>
    <%= form.govuk_radio_button(:department_id, 1, label: { text: 'IT' }) do %>
      <%= nil %>
    <% end %>
    <%= form.govuk_radio_button(:department_id, 3, label: { text: 'Sales' }) do %>
      <%= form.govuk_text_field(:other_department_name) %>
    <% end %>
    <%= form.govuk_radio_button(:department_id, 2, label: { text: 'Marketing' }) %>
  <% end %>

  <%= form.govuk_submit %>
<% end %>

Perhaps this is specific to Slim? Is it this, which is apparently fixed in 7.1.2.

Thanks for looking into this! Yes, I think you've found what the issue was. We're now on 7.1.2 and are no longer experiencing the problem with = nil, so can go back to one-line = yield if statements for our conditional reveals.

Great, thanks for confirming 🙂 Closing this but please reopen if it reappears.