Merge pull request #29439 from npezza93/set-skip-default-ids

Allow skip_default_ids option to be set in form_with

actionview/CHANGELOG.md

+*   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.`
+
+    *Nick Pezza*
+
 *   Fix issues with `field_error_proc` wrapping `optgroup` and select divider `option`.
 
     Fixes #31088

actionview/lib/action_view/helpers/form_helper.rb

@@ -478,6 +478,8 @@ def apply_form_for_options!(record, object, options) #:nodoc:
 
       mattr_accessor :form_with_generates_remote_forms, default: true
 
+      mattr_accessor :form_with_generates_ids, default: true
+
       # Creates a form tag based on mixing URLs, scopes, or models.
       #
       #   # Using just a URL:
@@ -640,16 +642,6 @@ def apply_form_for_options!(record, object, options) #:nodoc:
       #
       # 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
@@ -746,7 +738,6 @@ def apply_form_for_options!(record, object, options) #:nodoc:
       #   end
       def form_with(model: nil, scope: nil, url: nil, format: nil, **options)
         options[:allow_method_names_outside_object] = true
-        options[:skip_default_ids] = true
 
         if model
           url ||= polymorphic_path(model, format: format)
@@ -1044,16 +1035,6 @@ def fields_for(record_name, record_object = nil, options = {}, &block)
       # 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
@@ -1072,7 +1053,6 @@ def fields_for(record_name, record_object = nil, options = {}, &block)
       # FormOptionsHelper#collection_select and DateHelper#datetime_select.
       def fields(scope = nil, model: nil, **options, &block)
         options[:allow_method_names_outside_object] = true
-        options[:skip_default_ids] = true
 
         if model
           scope ||= model_name_from_record_or_class(model).param_key
@@ -1676,7 +1656,7 @@ def to_model
       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, :skip_default_ids, :allow_method_names_outside_object) : {}
+        @default_options = @options ? @options.slice(:index, :namespace, :allow_method_names_outside_object) : {}
 
         convert_to_legacy_options(@options)
 
@@ -1985,7 +1965,6 @@ def fields_for(record_name, record_object = nil, fields_options = {}, &block)
       # See the docs for the <tt>ActionView::FormHelper.fields</tt> helper method.
       def fields(scope = nil, model: nil, **options, &block)
         options[:allow_method_names_outside_object] = true
-        options[:skip_default_ids] = true
 
         convert_to_legacy_options(options)
 

actionview/lib/action_view/helpers/tags/base.rb

@@ -15,7 +15,6 @@ def initialize(object_name, method_name, template_object, options = {})
 
           @object_name.sub!(/\[\]$/, "") || @object_name.sub!(/\[\]\]$/, "]")
           @object = retrieve_object(options.delete(:object))
-          @skip_default_ids = options.delete(:skip_default_ids)
           @allow_method_names_outside_object = options.delete(:allow_method_names_outside_object)
           @options = options
 
@@ -97,7 +96,7 @@ def add_default_name_and_id(options)
             index = name_and_id_index(options)
             options["name"] = options.fetch("name") { tag_name(options["multiple"], index) }
 
-            unless skip_default_ids?
+            if generate_ids?
               options["id"] = options.fetch("id") { tag_id(index) }
               if namespace = options.delete("namespace")
                 options["id"] = options["id"] ? "#{namespace}_#{options['id']}" : namespace
@@ -183,8 +182,8 @@ def name_and_id_index(options)
             end
           end
 
-          def skip_default_ids?
-            @skip_default_ids
+          def generate_ids?
+            ActionView::Helpers::FormHelper.form_with_generates_ids
           end
       end
     end

actionview/lib/action_view/helpers/tags/collection_check_boxes.rb

@@ -12,7 +12,6 @@ class CheckBoxBuilder < Builder # :nodoc:
           def check_box(extra_html_options = {})
             html_options = extra_html_options.merge(@input_html_options)
             html_options[:multiple] = true
-            html_options[:skip_default_ids] = false
             @template_object.check_box(@object_name, @method_name, html_options, @value, nil)
           end
         end

actionview/lib/action_view/helpers/tags/collection_radio_buttons.rb

@@ -11,7 +11,6 @@ class CollectionRadioButtons < Base # :nodoc:
         class RadioButtonBuilder < Builder # :nodoc:
           def radio_button(extra_html_options = {})
             html_options = extra_html_options.merge(@input_html_options)
-            html_options[:skip_default_ids] = false
             @template_object.radio_button(@object_name, @method_name, @value, html_options)
           end
         end

actionview/lib/action_view/helpers/tags/label.rb

@@ -75,10 +75,6 @@ def render(&block)
           def render_component(builder)
             builder.translation
           end
-
-          def skip_default_ids?
-            false # The id is used as the `for` attribute.
-          end
       end
     end
   end

actionview/lib/action_view/helpers/tags/select.rb

@@ -8,7 +8,7 @@ def initialize(object_name, method_name, template_object, choices, options, html
           @choices = block_given? ? template_object.capture { yield || "" } : choices
           @choices = @choices.to_a if @choices.is_a?(Range)
 
-          @html_options = html_options.except(:skip_default_ids, :allow_method_names_outside_object)
+          @html_options = html_options.except(:allow_method_names_outside_object)
 
           super(object_name, method_name, template_object, options)
         end

actionview/lib/action_view/railtie.rb

@@ -22,8 +22,15 @@ class Railtie < Rails::Engine # :nodoc:
     initializer "action_view.form_with_generates_remote_forms" do |app|
       ActiveSupport.on_load(:action_view) do
         form_with_generates_remote_forms = app.config.action_view.delete(:form_with_generates_remote_forms)
-        unless form_with_generates_remote_forms.nil?
-          ActionView::Helpers::FormHelper.form_with_generates_remote_forms = form_with_generates_remote_forms
+        ActionView::Helpers::FormHelper.form_with_generates_remote_forms = form_with_generates_remote_forms
+      end
+    end
+
+    initializer "action_view.form_with_generates_ids" do |app|
+      ActiveSupport.on_load(:action_view) do
+        form_with_generates_ids = app.config.action_view.delete(:form_with_generates_ids)
+        unless form_with_generates_ids.nil?
+          ActionView::Helpers::FormHelper.form_with_generates_ids = form_with_generates_ids
         end
       end
     end

guides/source/configuring.md

@@ -578,6 +578,8 @@ Defaults to `'signed cookie'`.
 
 * `config.action_view.form_with_generates_remote_forms` determines whether `form_with` generates remote forms or not. This defaults to `true`.
 
+* `config.action_view.form_with_generates_ids` determines whether `form_with` generates ids on inputs. This defaults to `true`.
+
 ### Configuring Action Mailer
 
 There are a number of settings available on `config.action_mailer`:

railties/lib/rails/application/configuration.rb

@@ -72,6 +72,9 @@ def load_defaults(target_version)
 
           self.ssl_options = { hsts: { subdomains: true } }
 
+          if respond_to?(:action_view)
+            action_view.form_with_generates_ids = false
+          end
         when "5.1"
           load_defaults "5.0"
 
@@ -82,7 +85,6 @@ def load_defaults(target_version)
           if respond_to?(:action_view)
             action_view.form_with_generates_remote_forms = true
           end
-
         when "5.2"
           load_defaults "5.1"
 
@@ -106,6 +108,10 @@ def load_defaults(target_version)
             action_controller.default_protect_from_forgery = true
           end
 
+          if respond_to?(:action_view)
+            action_view.form_with_generates_remote_forms = true
+            action_view.form_with_generates_ids = true
+          end
         else
           raise "Unknown version #{target_version.to_s.inspect}"
         end

railties/test/application/configuration_test.rb

@@ -757,6 +757,68 @@ def index
       assert_match(/label/, last_response.body)
     end
 
+    test "form_with can be configured with form_with_generates_ids" do
+      app_file "config/initializers/form_builder.rb", <<-RUBY
+      Rails.configuration.action_view.form_with_generates_ids = false
+      RUBY
+
+      app_file "app/models/post.rb", <<-RUBY
+      class Post
+        include ActiveModel::Model
+        attr_accessor :name
+      end
+      RUBY
+
+      app_file "app/controllers/posts_controller.rb", <<-RUBY
+      class PostsController < ApplicationController
+        def index
+          render inline: "<%= begin; form_with(model: Post.new) {|f| f.text_field(:name)}; rescue => e; e.to_s; end %>"
+        end
+      end
+      RUBY
+
+      add_to_config <<-RUBY
+        routes.prepend do
+          resources :posts
+        end
+      RUBY
+
+      app "development"
+
+      get "/posts"
+
+      assert_no_match(/id=('|")post_name('|")/, last_response.body)
+    end
+
+    test "form_with outputs ids by default" do
+      app_file "app/models/post.rb", <<-RUBY
+      class Post
+        include ActiveModel::Model
+        attr_accessor :name
+      end
+      RUBY
+
+      app_file "app/controllers/posts_controller.rb", <<-RUBY
+      class PostsController < ApplicationController
+        def index
+          render inline: "<%= begin; form_with(model: Post.new) {|f| f.text_field(:name)}; rescue => e; e.to_s; end %>"
+        end
+      end
+      RUBY
+
+      add_to_config <<-RUBY
+        routes.prepend do
+          resources :posts
+        end
+      RUBY
+
+      app "development"
+
+      get "/posts"
+
+      assert_match(/id=('|")post_name('|")/, last_response.body)
+    end
+
     test "form_with can be configured with form_with_generates_remote_forms" do
       app_file "config/initializers/form_builder.rb", <<-RUBY
       Rails.configuration.action_view.form_with_generates_remote_forms = false