Update "Action View Form Helpers" guide #31972

bogdanvlviv · 2018-02-12T21:42:09Z

Details

@@ -30,7 +30,7 @@ - Change "`` tag" to "form tag" - Change "`/home/index`" to "a home page" - Remove "(some line breaks added for readability)" since there aren't broken lines below.

@@ -40,9 +40,11 @@

Change "an input element" to "input elements" since there are
two hidden elements.
Put the sentence "This input is important, because non-GET
form cannot be successfully submitted without it." to paragraph
about authenticity_token.
Change title of the link to security.html#cross-site-request-forgery-csrf
from "Security Guide" to "Securing Rails Application"(full name of the guide)
and append the word "guide".

@@ -76,10 +78,6 @@

Remove useless sentence "Besides text_field_tag and submit_tag,
there is a similar helper for every form control in HTML." since
user is able to get this info from the guide below.
Remove useless suggestion "IMPORTANT: Always use "GET" as the method
for search forms. This allows users to bookmark a specific search
and get back to it. More generally Rails encourages you to use
the right HTTP verb for an action".

@@ -110,7 +108,7 @@

Change "in chapter 7 of this guide" to "in chapter
Understanding Parameter Naming Conventions of this guide".

@@ -142,7 +140,7 @@
@@ -151,7 +149,7 @@

<%= label_tag(:age_adult, "I'm over 21") %> outputs
<label for="age_adult">I'm over 21</label>
I changed "I'm" to "I am" in input/output for consistency with example
in order to correct output.

@@ -215,7 +213,7 @@

Change title of the link to security.html#logging
from "Security Guide" to "Securing Rails Application"(full name of the guide)
and append the word "guide".

@@ -233,10 +231,10 @@

Change "<input/>" to "<input />" for consistency with other examples
in the guide and readability.
Remove useless sentence "The params[:person] hash is suitable for
passing to Person.new or, if @person is an instance of Person,
@person.update. While the name of an attribute is the most common
second parameter to these helpers this is not compulsory.
In the example above, as long as person objects have a
name and a name= method Rails will be happy."

@@ -283,7 +281,7 @@

Change "in the parameter_names section" to "in chapter Understanding
Parameter Naming Conventions of this guide".

@@ -292,7 +290,7 @@

Fix the example of using fields_for with :contact_detail
argument to ensure that this will work if @person.contact_detail
is nil.

@@ -309,7 +307,9 @@

Remove useless and probably incorrect sentence
"(in fact form_for calls fields_for internally)"
We shouldn't express internals same as private API.
Add note about (and link to) existing method form_with.

@@ -319,7 +319,7 @@

Downcase the word "Form" in "Rails Routing From the Outside In"
(in order to consistency with the name of the guide) and
append the word "guide".

@@ -327,21 +327,21 @@

Change "same thing, short-style (record identification gets used):" to
"short-style:" for consistency and removing useless info.
Change passing of method: argument in the example of using form_for
from html: { method: "patch"} to method: "patch" since it is
more correct and preferable.
Remove useless and incorrect sentence "Record identification is smart
enough to figure out if the record is new by asking record.new_record?"
We shouldn't express internals same as private API.
Add missing comma.
Remove incorrect sentence "These attributes will be omitted for brevity in
the rest of this guide." since our examples show all attributes.
Fix warning about using form_for with STI:
Change the sentence "You will have to specify the model name, :url,
and :method explicitly." to "You will have to specify :url,
and :as (the model name) explicitly." since you'll have to specify
only :url and :as when use STI and form_for.

@@ -357,11 +357,11 @@

Change "the routing guide" to "Rails Routing From the Outside In guide"

@@ -369,7 +369,7 @@

Change "output:" to "Output:"
Fix example by removing "..." in order for it works.

@@ -405,74 +404,84 @@

Fix example by removing "..." in order for it works.
Change "output:" to "Output:"
Express output as HTML and remove redundant dots.
Remove useless sentence "Often this will be the id of a corresponding
database object but this does not have to be the case." and
useless example of "select_tag and options_for_select"
Remove complicated and incorrect sentence "When :include_blank or :prompt
are not present, :include_blank is forced true if the select attribute
required is true, display size is one and multiple is not true."
Change "Models" to "Model Objects" for consistency.
Change "database model" to "model" since form_for can work with model
that doesn't related to database.
Improve the example by adding output.

@@ -480,21 +489,24 @@

Fix example by removing "..." in order for it works.
Clarify example
Make warning about using select with an association more clear to express
what is important to remember. Remove useless and complicated explanation
what would happen if you do wrong in this case.

@@ -511,7 +523,7 @@

Make the sentence more clear.

@@ -520,16 +532,16 @@

Improve the example
Make the sentence more clear.
Change "TimeZone" to "ActiveSupport::TimeZone" and express it as a link

@@ -537,21 +549,21 @@

Make the sentence more clear.
Add missing comma after "Instead"
Add Oxford comma
Clear the example by removing "..."

@@ -585,9 +600,12 @@

Clear the example by removing "..."

@@ -604,30 +622,28 @@

Remove useless note "In many cases the built-in date pickers are clumsy as
they do not aid the user in working out the relationship between
the date and the day of the week."
Add Oxford comma
Change "Time.now" to "Time.new(2009)" in the example to make
the explanation more clear by removing "if the current year is 2009"
Change "encoding MUST" to "enctype must" since it is more
clear.

@@ -631,38 +647,32 @@

Pass url: {action: :upload} to clarify the example since
below we use upload action
Remove useless sentence "The only difference with other helpers is that you
cannot set a default value for file inputs as this would have no meaning."
in order to pay attention of a reader to main info in this paragraph.
Fix and to simpify complicated sentence about instance that
represent uploaded file by refer to ActionDispatch::Http::UploadedFile.
Remove useless " (assuming the form was the one in the previous example)".
Change the name of the variable in the example from uploaded_io to uploaded_file.
Add mention about and reference to Active Storage.
Remove useless section "Dealing with Ajax" since we can rely on
"Active Storage Overview" guide.
Remove beginning of the sentence "As mentioned previously".
Change FormBuilder to ActionView::Helpers::FormBuilder and
express it as a link.

@@ -703,12 +713,12 @@

Remove beginning of the sentence "As you've seen in the previous sections".
Change FormBuilder to ActionView::Helpers::FormBuilder.

@@ -756,16 +766,19 @@

Change name of the input from "addresses[][line1]" to "person[addresses][][line1]"
for consistency.
Remove useless sentences "Rails decides to start accumulating values in a
new hash whenever it encounters an input name that
already exists in the current hash" and "When working with array
parameters this duplicate submission will confuse Rails since duplicate input
names are how it decides when to start a new array element.
It is preferable to either use check_box_tag or to use hashes instead of arrays.".

@@ -788,6 +801,7 @@

Clarify the example by adding "<input name="_method" type="hidden" value="patch" />"
since this example of edit form.

@@ -812,7 +826,7 @@

Fix the value of :index option, change address to address.id.

@@ -820,12 +834,12 @@

Capitalize the name of the city Bologna in the example.
Fix the value of :index option, change address to address.id.

@@ -862,7 +876,7 @@

Downcase "Or" since this isn't beginning of a new sentence.

@@ -948,7 +962,7 @@

Remove useless sentence "You may wish to do this if the autogenerated
input is placed in a location where an input tag is not valid HTML or when
using an ORM where children do not have an id".

@@ -979,9 +993,9 @@

Explain more correctly the value of _destroy.

@@ -1024,4 +1038,4 @@

Change "milliseconds after the epoch" to "milliseconds since the epoch" and
express as link the word "epoch" that can help to get to know about the "epoch".

[ci skip]

rails-bot · 2018-02-12T21:42:18Z

r? @eileencodes

(@rails-bot has picked a reviewer for you, use r? to override)

rafaelfranca · 2018-02-17T05:13:19Z

guides/source/form_helpers.md

@@ -76,10 +78,6 @@ This will generate the following HTML:

 TIP: For every form input, an ID attribute is generated from its name (`"q"` in above example). These IDs can be very useful for CSS styling or manipulation of form controls with JavaScript.

-Besides `text_field_tag` and `submit_tag`, there is a similar helper for _every_ form control in HTML.
-
-IMPORTANT: Always use "GET" as the method for search forms. This allows users to bookmark a specific search and get back to it. More generally Rails encourages you to use the right HTTP verb for an action.


Why is this useless?

I just returned this. I removed "Always" from the beginning of the sentence since it sounds like an obligation with this word.

rafaelfranca · 2018-02-17T05:14:29Z

guides/source/form_helpers.md

 ```

-Upon form submission the value entered by the user will be stored in `params[:person][:name]`. The `params[:person]` hash is suitable for passing to `Person.new` or, if `@person` is an instance of Person, `@person.update`. While the name of an attribute is the most common second parameter to these helpers this is not compulsory. In the example above, as long as person objects have a `name` and a `name=` method Rails will be happy.


Why is this useless?

We have another place where we completly explain how to use params, see https://edgeguides.rubyonrails.org/action_controller_overview.html#strong-parameters

rafaelfranca · 2018-02-17T05:15:08Z

guides/source/form_helpers.md

-The object yielded by `fields_for` is a form builder like the one yielded by `form_for` (in fact `form_for` calls `fields_for` internally).
+The object yielded by `fields_for` is a form builder like the one yielded by `form_for`.
+
+NOTE: There is method [`form_with`](http://api.rubyonrails.org/classes/ActionView/Helpers/FormHelper.html#method-i-form_with) that creates a form tag based on mixing URLs, scopes, or models.


what mixing means in this context?

rafaelfranca · 2018-02-17T05:17:11Z

guides/source/form_helpers.md

 # short-style:
 form_for(@article)
 ```

-Notice how the short-style `form_for` invocation is conveniently the same, regardless of the record being new or existing. Record identification is smart enough to figure out if the record is new by asking `record.new_record?`. It also selects the correct path to submit to and the name based on the class of the object.


This is not internal and it is not useless. Where we would explain this behavior to the users?

Agree. I just returned this.

rafaelfranca · 2018-02-17T05:19:15Z

guides/source/form_helpers.md

 ```

-The first argument to `options_for_select` is a nested array where each element has two elements: option text (city name) and option value (city id). The option value is what will be submitted to your controller. Often this will be the id of a corresponding database object but this does not have to be the case.
-
-Knowing this, you can combine `select_tag` and `options_for_select` to achieve the desired, complete markup:


The flow here was better before with this phrase. The guides should not be mechanical but have a nice flow

rafaelfranca · 2018-02-17T05:19:35Z

guides/source/form_helpers.md

 ```

-The first argument to `options_for_select` is a nested array where each element has two elements: option text (city name) and option value (city id). The option value is what will be submitted to your controller. Often this will be the id of a corresponding database object but this does not have to be the case.


I think the Often here was a nice explanation and we should keep.

rafaelfranca · 2018-02-17T05:20:19Z

guides/source/form_helpers.md

 ```

 Whenever Rails sees that the internal value of an option being generated matches this value, it will add the `selected` attribute to that option.

-WARNING: When `:include_blank` or `:prompt` are not present, `:include_blank` is forced true if the select attribute `required` is true, display `size` is one and `multiple` is not true.


If it is complicated and incorrect we fix, we don't have to remove.

rafaelfranca · 2018-02-17T05:22:15Z

guides/source/form_helpers.md


 ```erb
 <%= time_zone_select(:person, :time_zone) %>
 ```

 There is also `time_zone_options_for_select` helper for a more manual (therefore more customizable) way of doing this. Read the [API documentation](http://api.rubyonrails.org/classes/ActionView/Helpers/FormOptionsHelper.html#method-i-time_zone_options_for_select) to learn about the possible arguments for these two methods.

-Rails _used_ to have a `country_select` helper for choosing countries, but this has been extracted to the [country_select plugin](https://github.com/stefanpenner/country_select). When using this, be aware that the exclusion or inclusion of certain names from the list can be somewhat controversial (and was the reason this functionality was extracted from Rails).
+Rails _used_ to have a `country_select` helper for choosing countries, but this has been extracted to the [country_select gem](https://github.com/stefanpenner/country_select).


plugin is correct.

rafaelfranca · 2018-02-17T05:23:29Z

guides/source/form_helpers.md


 Uploading Files
 ---------------

-A common task is uploading some sort of file, whether it's a picture of a person or a CSV file containing data to process. The most important thing to remember with file uploads is that the rendered form's encoding **MUST** be set to "multipart/form-data". If you use `form_for`, this is done automatically. If you use `form_tag`, you must set it yourself, as per the following example.
+A common task is uploading some sort of file, whether it's a picture of a person or a CSV file containing data to process. The most important thing to remember with file uploads is that the rendered form's enctype **must** be set to "multipart/form-data". If you use `form_for`, this is done automatically. If you use `form_tag`, you must set it yourself, as per the following example.


either encoding as it was before or enctype attribute.

bogdanvlviv · 2018-08-13T02:15:54Z

#31972 (comment)

WARNING: When :include_blank or :prompt are not present, :include_blank is forced true if the select attribute required is true, display size is one and multiple is not true.

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

Lines 144 to 161 in 15774d2

    
           def select_content_tag(option_tags, options, html_options) 
        
             html_options = html_options.stringify_keys 
        
             add_default_name_and_id(html_options) 
        
             if placeholder_required?(html_options) 
        
               raise ArgumentError, "include_blank cannot be false for a required field." if options[:include_blank] == false 
        
               options[:include_blank] ||= true unless options[:prompt] 
        
             end 
        
             value = options.fetch(:selected) { value() } 
        
             select = content_tag("select", add_options(option_tags, options, value), html_options) 
        
             if html_options["multiple"] && options.fetch(:include_hidden, true) 
        
               tag("input", disabled: html_options["disabled"], name: html_options["name"], type: "hidden", value: "") + select 
        
             else 
        
               select 
        
             end 
        
           end

It only works for select:

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

Line 28 in 15774d2

select_content_tag(option_tags, @options, @html_options)

but doesn't for select_tag:

rails/actionview/lib/action_view/helpers/form_tag_helper.rb

Lines 135 to 158 in 15774d2

    
           def select_tag(name, option_tags = nil, options = {}) 
        
             option_tags ||= "" 
        
             html_name = (options[:multiple] == true && !name.to_s.ends_with?("[]")) ? "#{name}[]" : name 
        
             if options.include?(:include_blank) 
        
               include_blank = options.delete(:include_blank) 
        
               options_for_blank_options_tag = { value: "" } 
        
               if include_blank == true 
        
                 include_blank = "" 
        
                 options_for_blank_options_tag[:label] = " " 
        
               end 
        
               if include_blank 
        
                 option_tags = content_tag("option".freeze, include_blank, options_for_blank_options_tag).safe_concat(option_tags) 
        
               end 
        
             end 
        
             if prompt = options.delete(:prompt) 
        
               option_tags = content_tag("option".freeze, prompt, value: "").safe_concat(option_tags) 
        
             end 
        
             content_tag "select".freeze, option_tags, { "name" => html_name, "id" => sanitize_to_id(name) }.update(options.stringify_keys) 
        
           end

This commit is the next work after rails#33523. Also, this commit removes mention about hidden `utf8` input. Since form helpers don't generate this input by default since rails#32125. Note that I also had created PR rails#31972 with improvements to "Action View Form Helpers" guide, but I'll rebase it after merging the current PR.

bogdanvlviv · 2018-08-27T16:28:11Z

I've just rebased with master branch after applied significant changes to this guide - #33523, #33727.

rails-bot assigned eileencodes Feb 12, 2018

bogdanvlviv force-pushed the update-form_helpers-guide branch 2 times, most recently from 9a6b9cb to 21915f2 Compare February 12, 2018 21:54

rafaelfranca requested changes Feb 17, 2018

View reviewed changes

bogdanvlviv force-pushed the update-form_helpers-guide branch 5 times, most recently from e8e7f74 to 97c9a8d Compare August 13, 2018 02:10

bogdanvlviv force-pushed the update-form_helpers-guide branch from 97c9a8d to 02b56ee Compare August 13, 2018 02:30

bogdanvlviv mentioned this pull request Aug 27, 2018

Follow up #33523 [ci skip] #33727

Merged

bogdanvlviv force-pushed the update-form_helpers-guide branch 3 times, most recently from 99e9e86 to c03dd15 Compare August 27, 2018 16:12

Update "Action View Form Helpers" guide [ci skip]

841fc18

bogdanvlviv force-pushed the update-form_helpers-guide branch from c03dd15 to 841fc18 Compare August 27, 2018 16:25

rafaelfranca merged commit 3218a66 into rails:master Aug 27, 2018

bogdanvlviv deleted the update-form_helpers-guide branch August 27, 2018 16:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update "Action View Form Helpers" guide #31972

Update "Action View Form Helpers" guide #31972

bogdanvlviv commented Feb 12, 2018 •

edited

rails-bot commented Feb 12, 2018

rafaelfranca Feb 17, 2018

bogdanvlviv Aug 13, 2018

rafaelfranca Feb 17, 2018

bogdanvlviv Aug 13, 2018

rafaelfranca Feb 17, 2018

rafaelfranca Feb 17, 2018

bogdanvlviv Aug 13, 2018 •

edited

rafaelfranca Feb 17, 2018

rafaelfranca Feb 17, 2018

rafaelfranca Feb 17, 2018

rafaelfranca Feb 17, 2018

rafaelfranca Feb 17, 2018

bogdanvlviv commented Aug 13, 2018 •

edited

bogdanvlviv commented Aug 27, 2018

		```

		Upon form submission the value entered by the user will be stored in `params[:person][:name]`. The `params[:person]` hash is suitable for passing to `Person.new` or, if `@person` is an instance of Person, `@person.update`. While the name of an attribute is the most common second parameter to these helpers this is not compulsory. In the example above, as long as person objects have a `name` and a `name=` method Rails will be happy.

		```

		The first argument to `options_for_select` is a nested array where each element has two elements: option text (city name) and option value (city id). The option value is what will be submitted to your controller. Often this will be the id of a corresponding database object but this does not have to be the case.

Update "Action View Form Helpers" guide #31972

Update "Action View Form Helpers" guide #31972

Conversation

bogdanvlviv commented Feb 12, 2018 • edited

rails-bot commented Feb 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bogdanvlviv Aug 13, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bogdanvlviv commented Aug 13, 2018 • edited

bogdanvlviv commented Aug 27, 2018

bogdanvlviv commented Feb 12, 2018 •

edited

bogdanvlviv Aug 13, 2018 •

edited

bogdanvlviv commented Aug 13, 2018 •

edited