rails
rails copied to clipboard
`form_tag` and `form_with`: close tag when block omitted
Motivation / Background
Prior to this change, calling a form_tag
or form_with
without
passing a block generates a <form>
element without a closing </form>
tag.
The form_for version of building a <form>
element requires a block
to execute, so omitting the closing </form>
element isn't a
possibility.
A current work-around to achieve the desired behavior is to declare the
form_tag
or form_with
call with an empty block:
<%= form_tag "https://example.com" do %>
<% end %>
<%= form_with url: "https://example.com" do %>
<% end %>
Unfortunately, being unaware of this issue will likely yield HTML that
is silently invalid, which can cause surprising nesting and behavior,
such as elements declared after the <form>
becoming descendants
instead of siblings.
Detail
Introduce the config.action_view.closes_form_tag_without_block
configuration value.
By default (in versions prior to 7.2), that value is false
and
preserves backwards compatibility. When set to true
, calls to
form_with
and form_tag
without a block will result in elements that
close themselves.
Checklist
Before submitting the PR make sure the following are checked:
- [x] This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
- [x] Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex:
[Fix #issue-number]
- [x] Tests are added or updated if you fix a bug or add a feature.
- [x] CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.
Sounds reasonable, and the change looks good. Just wanted to bump this PR 🙃
This is the described intent of the method, per #26976 -- it has a non-block form so that it can be support the form_tag
use case as well as the form_for
one.
I'm doubtful that entirely-empty-form-tag is a common enough use case that people are likely to call this blocklessly and expect/want that result... is that a frequent pattern in a context I'm unfamiliar with?
is that a frequent pattern in a context I'm unfamiliar with?
The use case that got me digging was inline editing:
<%# app/views/articles/_inline_edit.html.erb %>
<% frame_id = dom_id(model, "#{method}_turbo_frame") %>
<%= form_with model: model, data: { turbo_frame: frame_id } do %>
<turbo-frame id="<%= frame_id %>" class="contents group inline-edit">
<%= yield %>
<%= link_to edit_article_path(model) do %>
Edit <%= model.class.human_attribute_name(method) %>
<% end %>
</turbo-frame>
<% end %>
Rendering the partial with a block would wrap the contents (through the <%= yield %>
line) within a <form>
generated by form_with
. It's invalid HTML to nest a <form>
within another <form>
, so to prevent accidents, the original version of the partial rendered the <turbo-frame>
and <form>
as siblings, and rendered an empty <form>
with an [id]
attribute:
<% frame_id = dom_id(model, "#{method}_turbo_frame") %>
<%= form_with model: model, id: "#{method}_form", data: { turbo_frame: frame_id } %>
<turbo-frame id="<%= frame_id %>" class="contents group inline-edit">
<%= yield %>
<%= link_to edit_article_path(model) do %>
Edit <%= model.class.human_attribute_name(method) %>
<% end %>
</turbo-frame>
This version would require callers to render fields with a [form]
attribute that referred to the <form>
element by [id]
(i.e. "#{method}_form"
. This way, there's no risk of doubly-nested <form>
elements. Once I discovered that I needed to call the form_with
with an empty block or with a hard-coded </form>
tag as a sibling, it worked as intended.
I'd never deployed the pattern before. It's certainly an edge case that's extremely uncommon. I think my surprise stems from the overlap between empty <form>
elements being so uncommon and the form_with
use case that depends on the omission of the trailing </form>
tag being so uncommon. The form_with documentation doesn't mention this behavior, and there aren't any tests that assert the omission.
If ensuring the </form>
tag is a step in the wrong direction, should there be an alternative PR that documents the behavior and adds tests to guard against introducing a regression in the future?
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
@matthewd I'm sorry to have let this PR drift so far behind main
. I've rebased it.
To re-iterate my main motivation for this change:
I think my surprise stems from the overlap between empty <form>
elements being so uncommon and the behavior of a block-less form_with
being somewhat difficult to predict.
Even if it isn't intended as an end-result, a call to <%= form_with ... %>
without a block includes all subsequent elements as its children, since it isn't self-closing. Accidents that introduce an open <form>
element early in the page are what I'm most afraid of.
@matthewd I've expanded this work to include a new config.action_view.renders_void_form_elements
configuration value and 7.2 default.
Does that level on granular control improve this change's backwards compatibility? I'm hopeful that it could also provide an avenue toward deprecation, then maybe global support in the next major release.
@rafaelfranca could you review these changes?
Another use case for an empty form, is a form with the submit button outside the form element:
<form id="create_like" action="likes" method="POST" >
<input type="hidden" name="authenticity_token" value="zZEbm" />
</form>
<button type="submit" form="create_like">:+1</button>
This would be useful when the button is already in a form:
<form id="create_like" action="likes" method="POST" >
...
</form>
<form action="comment" method="POST">
....
<button type="submit" form="create_like">:+1</button>
</form>
@skipkayhil are you able to review this proposal?