Skip to content

Commit 260d6f1

Browse files
npezza93rafaelfranca
authored andcommitted
Change form_with to generates ids by default
When `form_with` was introduced we disabled the automatic generation of ids that was enabled in `form_for`. This usually is not an good idea since labels don't work when the input doesn't have an id and it made harder to test with Capybara. You can still disable the automatic generation of ids setting `config.action_view.form_with_generates_ids` to `false.`
1 parent f76ca45 commit 260d6f1

File tree

12 files changed

+310
-225
lines changed

12 files changed

+310
-225
lines changed

Diff for: actionview/CHANGELOG.md

+11
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
* Change `form_with` to generates ids by default.
2+
3+
When `form_with` was introduced we disabled the automatic generation of ids
4+
that was enabled in `form_for`. This usually is not an good idea since labels don't work
5+
when the input doesn't have an id and it made harder to test with Capybara.
6+
7+
You can still disable the automatic generation of ids setting `config.action_view.form_with_generates_ids`
8+
to `false.`
9+
10+
*Nick Pezza*
11+
112
* Fix issues with `field_error_proc` wrapping `optgroup` and select divider `option`.
213

314
Fixes #31088

Diff for: actionview/lib/action_view/helpers/form_helper.rb

+3-24
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,8 @@ def apply_form_for_options!(record, object, options) #:nodoc:
478478

479479
mattr_accessor :form_with_generates_remote_forms, default: true
480480

481+
mattr_accessor :form_with_generates_ids, default: true
482+
481483
# Creates a form tag based on mixing URLs, scopes, or models.
482484
#
483485
# # Using just a URL:
@@ -640,16 +642,6 @@ def apply_form_for_options!(record, object, options) #:nodoc:
640642
#
641643
# Where <tt>@document = Document.find(params[:id])</tt>.
642644
#
643-
# When using labels +form_with+ requires setting the id on the field being
644-
# labelled:
645-
#
646-
# <%= form_with(model: @post) do |form| %>
647-
# <%= form.label :title %>
648-
# <%= form.text_field :title, id: :post_title %>
649-
# <% end %>
650-
#
651-
# See +label+ for more on how the +for+ attribute is derived.
652-
#
653645
# === Mixing with other form helpers
654646
#
655647
# While +form_with+ uses a FormBuilder object it's possible to mix and
@@ -746,7 +738,6 @@ def apply_form_for_options!(record, object, options) #:nodoc:
746738
# end
747739
def form_with(model: nil, scope: nil, url: nil, format: nil, **options)
748740
options[:allow_method_names_outside_object] = true
749-
options[:skip_default_ids] = true
750741

751742
if model
752743
url ||= polymorphic_path(model, format: format)
@@ -1044,16 +1035,6 @@ def fields_for(record_name, record_object = nil, options = {}, &block)
10441035
# or model is yielded, so any generated field names are prefixed with
10451036
# either the passed scope or the scope inferred from the <tt>:model</tt>.
10461037
#
1047-
# When using labels +fields+ requires setting the id on the field being
1048-
# labelled:
1049-
#
1050-
# <%= fields :comment do |fields| %>
1051-
# <%= fields.label :body %>
1052-
# <%= fields.text_field :body, id: :comment_body %>
1053-
# <% end %>
1054-
#
1055-
# See +label+ for more on how the +for+ attribute is derived.
1056-
#
10571038
# === Mixing with other form helpers
10581039
#
10591040
# While +form_with+ uses a FormBuilder object it's possible to mix and
@@ -1072,7 +1053,6 @@ def fields_for(record_name, record_object = nil, options = {}, &block)
10721053
# FormOptionsHelper#collection_select and DateHelper#datetime_select.
10731054
def fields(scope = nil, model: nil, **options, &block)
10741055
options[:allow_method_names_outside_object] = true
1075-
options[:skip_default_ids] = true
10761056

10771057
if model
10781058
scope ||= model_name_from_record_or_class(model).param_key
@@ -1676,7 +1656,7 @@ def to_model
16761656
def initialize(object_name, object, template, options)
16771657
@nested_child_index = {}
16781658
@object_name, @object, @template, @options = object_name, object, template, options
1679-
@default_options = @options ? @options.slice(:index, :namespace, :skip_default_ids, :allow_method_names_outside_object) : {}
1659+
@default_options = @options ? @options.slice(:index, :namespace, :allow_method_names_outside_object) : {}
16801660

16811661
convert_to_legacy_options(@options)
16821662

@@ -1985,7 +1965,6 @@ def fields_for(record_name, record_object = nil, fields_options = {}, &block)
19851965
# See the docs for the <tt>ActionView::FormHelper.fields</tt> helper method.
19861966
def fields(scope = nil, model: nil, **options, &block)
19871967
options[:allow_method_names_outside_object] = true
1988-
options[:skip_default_ids] = true
19891968

19901969
convert_to_legacy_options(options)
19911970

Diff for: actionview/lib/action_view/helpers/tags/base.rb

+3-4
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ def initialize(object_name, method_name, template_object, options = {})
1515

1616
@object_name.sub!(/\[\]$/, "") || @object_name.sub!(/\[\]\]$/, "]")
1717
@object = retrieve_object(options.delete(:object))
18-
@skip_default_ids = options.delete(:skip_default_ids)
1918
@allow_method_names_outside_object = options.delete(:allow_method_names_outside_object)
2019
@options = options
2120

@@ -97,7 +96,7 @@ def add_default_name_and_id(options)
9796
index = name_and_id_index(options)
9897
options["name"] = options.fetch("name") { tag_name(options["multiple"], index) }
9998

100-
unless skip_default_ids?
99+
if generate_ids?
101100
options["id"] = options.fetch("id") { tag_id(index) }
102101
if namespace = options.delete("namespace")
103102
options["id"] = options["id"] ? "#{namespace}_#{options['id']}" : namespace
@@ -183,8 +182,8 @@ def name_and_id_index(options)
183182
end
184183
end
185184

186-
def skip_default_ids?
187-
@skip_default_ids
185+
def generate_ids?
186+
ActionView::Helpers::FormHelper.form_with_generates_ids
188187
end
189188
end
190189
end

Diff for: actionview/lib/action_view/helpers/tags/collection_check_boxes.rb

-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ class CheckBoxBuilder < Builder # :nodoc:
1212
def check_box(extra_html_options = {})
1313
html_options = extra_html_options.merge(@input_html_options)
1414
html_options[:multiple] = true
15-
html_options[:skip_default_ids] = false
1615
@template_object.check_box(@object_name, @method_name, html_options, @value, nil)
1716
end
1817
end

Diff for: actionview/lib/action_view/helpers/tags/collection_radio_buttons.rb

-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ class CollectionRadioButtons < Base # :nodoc:
1111
class RadioButtonBuilder < Builder # :nodoc:
1212
def radio_button(extra_html_options = {})
1313
html_options = extra_html_options.merge(@input_html_options)
14-
html_options[:skip_default_ids] = false
1514
@template_object.radio_button(@object_name, @method_name, @value, html_options)
1615
end
1716
end

Diff for: actionview/lib/action_view/helpers/tags/label.rb

-4
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,6 @@ def render(&block)
7575
def render_component(builder)
7676
builder.translation
7777
end
78-
79-
def skip_default_ids?
80-
false # The id is used as the `for` attribute.
81-
end
8278
end
8379
end
8480
end

Diff for: actionview/lib/action_view/helpers/tags/select.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ def initialize(object_name, method_name, template_object, choices, options, html
88
@choices = block_given? ? template_object.capture { yield || "" } : choices
99
@choices = @choices.to_a if @choices.is_a?(Range)
1010

11-
@html_options = html_options.except(:skip_default_ids, :allow_method_names_outside_object)
11+
@html_options = html_options.except(:allow_method_names_outside_object)
1212

1313
super(object_name, method_name, template_object, options)
1414
end

Diff for: actionview/lib/action_view/railtie.rb

+9
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ class Railtie < Rails::Engine # :nodoc:
2828
end
2929
end
3030

31+
initializer "action_view.form_with_generates_ids" do |app|
32+
ActiveSupport.on_load(:action_view) do
33+
form_with_generates_ids = app.config.action_view.delete(:form_with_generates_ids)
34+
unless form_with_generates_ids.nil?
35+
ActionView::Helpers::FormHelper.form_with_generates_ids = form_with_generates_ids
36+
end
37+
end
38+
end
39+
3140
initializer "action_view.logger" do
3241
ActiveSupport.on_load(:action_view) { self.logger ||= Rails.logger }
3342
end

0 commit comments

Comments
 (0)