diff options
author | Kasper Timm Hansen <kaspth@gmail.com> | 2016-11-21 21:48:38 +0100 |
---|---|---|
committer | Kasper Timm Hansen <kaspth@gmail.com> | 2016-12-13 21:02:36 +0100 |
commit | a6d065e39f9285119d75a05ee46c659f8b3c0db8 (patch) | |
tree | 80239fbd5fe2ddd7dfeaec8c6fe7bf49383cbbdf /actionview/lib | |
parent | 34a1c7e284e9d307f61070dfb30f0d6ab34c74d2 (diff) | |
download | rails-a6d065e39f9285119d75a05ee46c659f8b3c0db8.tar.gz rails-a6d065e39f9285119d75a05ee46c659f8b3c0db8.tar.bz2 rails-a6d065e39f9285119d75a05ee46c659f8b3c0db8.zip |
form_with/fields: Don't output ids by default
Continuing 67f81cc where we decided not to output ids
by default in the new form helpers.
Went with @dhh's suggestion of just requiring ids on
fields being labelled:
https://github.com/rails/rails/issues/25197#issuecomment-231797117
Seems okay enough.
Diffstat (limited to 'actionview/lib')
-rw-r--r-- | actionview/lib/action_view/helpers/form_helper.rb | 31 | ||||
-rw-r--r-- | actionview/lib/action_view/helpers/tags/base.rb | 14 | ||||
-rw-r--r-- | actionview/lib/action_view/helpers/tags/label.rb | 4 |
3 files changed, 43 insertions, 6 deletions
diff --git a/actionview/lib/action_view/helpers/form_helper.rb b/actionview/lib/action_view/helpers/form_helper.rb index e7ea267211..55389950ed 100644 --- a/actionview/lib/action_view/helpers/form_helper.rb +++ b/actionview/lib/action_view/helpers/form_helper.rb @@ -595,6 +595,16 @@ module ActionView # # Where <tt>@document = Document.find(params[:id])</tt>. # + # When using labels +form_with+ requires setting the id on the field being + # labelled: + # + # <%= form_with(model: @post) do |form| %> + # <%= form.label :title %> + # <%= form.text_field :title, id: :post_title %> + # <% end %> + # + # See +label+ for more on how the +for+ attribute is derived. + # # === Mixing with other form helpers # # While +form_with+ uses a FormBuilder object it's possible to mix and @@ -690,6 +700,8 @@ module ActionView # form_with(**options.merge(builder: LabellingFormBuilder), &block) # end def form_with(model: nil, scope: nil, url: nil, format: nil, **options) + options[:skip_default_ids] = true + if model url ||= polymorphic_path(model, format: format) @@ -986,6 +998,16 @@ module ActionView # or model is yielded, so any generated field names are prefixed with # either the passed scope or the scope inferred from the <tt>:model</tt>. # + # When using labels +fields+ requires setting the id on the field being + # labelled: + # + # <%= fields :comment do |fields| %> + # <%= fields.label :body %> + # <%= fields.text_field :body, id: :comment_body %> + # <% end %> + # + # See +label+ for more on how the +for+ attribute is derived. + # # === Mixing with other form helpers # # While +form_with+ uses a FormBuilder object it's possible to mix and @@ -1003,7 +1025,8 @@ module ActionView # to work with an object as a base, like # FormOptionHelper#collection_select and DateHelper#datetime_select. def fields(scope = nil, model: nil, **options, &block) - # TODO: Remove when ids and classes are no longer output by default. + options[:skip_default_ids] = true + if model scope ||= model_name_from_record_or_class(model).param_key end @@ -1469,7 +1492,7 @@ module ActionView private def html_options_for_form_with(url_for_options = nil, model = nil, html: {}, local: false, skip_enforcing_utf8: false, **options) - html_options = options.except(:index, :include_id, :builder).merge(html) + html_options = options.except(:index, :include_id, :skip_default_ids, :builder).merge(html) html_options[:method] ||= :patch if model.respond_to?(:persisted?) && model.persisted? html_options[:enforce_utf8] = !skip_enforcing_utf8 @@ -1603,7 +1626,7 @@ module ActionView def initialize(object_name, object, template, options) @nested_child_index = {} @object_name, @object, @template, @options = object_name, object, template, options - @default_options = @options ? @options.slice(:index, :namespace) : {} + @default_options = @options ? @options.slice(:index, :namespace, :skip_default_ids) : {} convert_to_legacy_options(@options) @@ -1910,6 +1933,8 @@ module ActionView # See the docs for the <tt>ActionView::FormHelper.fields</tt> helper method. def fields(scope = nil, model: nil, **options, &block) + options[:skip_default_ids] = true + convert_to_legacy_options(options) fields_for(scope || model, model, **options, &block) diff --git a/actionview/lib/action_view/helpers/tags/base.rb b/actionview/lib/action_view/helpers/tags/base.rb index cf8a6d6028..b8c446cbed 100644 --- a/actionview/lib/action_view/helpers/tags/base.rb +++ b/actionview/lib/action_view/helpers/tags/base.rb @@ -13,6 +13,7 @@ module ActionView @object_name.sub!(/\[\]$/, "") || @object_name.sub!(/\[\]\]$/, "]") @object = retrieve_object(options.delete(:object)) + @skip_default_ids = options.delete(:skip_default_ids) @options = options @auto_index = Regexp.last_match ? retrieve_autoindex(Regexp.last_match.pre_match) : nil end @@ -81,9 +82,12 @@ module ActionView def add_default_name_and_id(options) index = name_and_id_index(options) options["name"] = options.fetch("name") { tag_name(options["multiple"], index) } - options["id"] = options.fetch("id") { tag_id(index) } - if namespace = options.delete("namespace") - options["id"] = options["id"] ? "#{namespace}_#{options['id']}" : namespace + + unless skip_default_ids? + options["id"] = options.fetch("id") { tag_id(index) } + if namespace = options.delete("namespace") + options["id"] = options["id"] ? "#{namespace}_#{options['id']}" : namespace + end end end @@ -154,6 +158,10 @@ module ActionView def name_and_id_index(options) options.key?("index") ? options.delete("index") || "" : @auto_index end + + def skip_default_ids? + @skip_default_ids + end end end end diff --git a/actionview/lib/action_view/helpers/tags/label.rb b/actionview/lib/action_view/helpers/tags/label.rb index b31d5fda66..cab15ae201 100644 --- a/actionview/lib/action_view/helpers/tags/label.rb +++ b/actionview/lib/action_view/helpers/tags/label.rb @@ -73,6 +73,10 @@ module ActionView def render_component(builder) builder.translation end + + def skip_default_ids? + false # The id is used as the `for` attribute. + end end end end |