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.

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

Conditional reveal behaviour change between Rails 7.0 and Rails 7.1 #448

Closed
patrick-laa opened this issue Oct 10, 2023 · 4 comments
Closed
Labels
bug Something isn't working

Comments

@patrick-laa
Copy link

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!

@peteryates
Copy link
Member

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

@peteryates peteryates added the bug Something isn't working label Dec 8, 2023
@peteryates
Copy link
Member

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.

@patrick-laa
Copy link
Author

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.

@peteryates
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants