Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixes #5324 by removing default size options from input:text and default cols and rows options from textarea. #5366

Merged
merged 3 commits into from Mar 11, 2012

Conversation

Projects
None yet
6 participants
Contributor

parndt commented Mar 10, 2012

I'm sure this will be a controversial change :-)

But this reduces hardcoded HTML and makes default HTML easier to add CSS to (in my opinion) and has the added benefit of reducing the amount of HTML that is sent to the browser.

See issue #5324 for more reasoning.

Thanks!

Contributor

josevalim commented Mar 10, 2012

I think this is a step forward. /cc @chriseppstein @fxn @jeremy

Sounds good :)

Only one question: should the options be completely removed, or should the options be cleared to allow the user to set the same defaults back, in case they rely on - or want - them?

Owner

fxn commented Mar 10, 2012

It may break the look & feel of existing apps, but I think this is OK for a major version.

The only thing I'd add is a note about this in the upgrading guide, so that people know they have to watch out for this change, rather than magically have their views looking weird.

Contributor

chriseppstein commented Mar 11, 2012

Agree with @fxn and that this is not something that should be in the rails helper.

@vijaydev vijaydev and 1 other commented on an outdated diff Mar 11, 2012

actionpack/lib/action_view/helpers/form_helper.rb
@@ -757,7 +757,7 @@ def file_field(object_name, method, options = {})
# # </textarea>
#
# text_area(:application, :notes, :cols => 40, :rows => 15, :class => 'app_input')
- # # => <textarea cols="40" rows="15" id="application_notes" name="application[notes]" class="app_input">
+ # # => <textarea rows="15" id="application_notes" name="application[notes]" class="app_input">
@vijaydev

vijaydev Mar 11, 2012

Member

why leave the rows here?

@parndt

parndt Mar 11, 2012

Contributor

Because the example specifies :rows

@parndt

parndt Mar 11, 2012

Contributor

Ah it also specifies :cols my bad, will fix.

Contributor

josevalim commented Mar 11, 2012

This also need a CHANGELOG entry. Please let me know once everything is into place.

Contributor

parndt commented Mar 11, 2012

@josevalim I've just pushed a change to the actionpack changelog. Hope it's appropriate! :-)

@josevalim josevalim added a commit that referenced this pull request Mar 11, 2012

@josevalim josevalim Merge pull request #5366 from parndt/fix_issue_5324
Fixes #5324 by removing default size options from input:text and default cols and rows options from textarea.
6c0d5a1

@josevalim josevalim merged commit 6c0d5a1 into rails:master Mar 11, 2012

@myabc myabc added a commit to opf/openproject that referenced this pull request May 1, 2015

@myabc myabc Update FormBuilder specs for removed default opts.
Rails 4 no longer sets default values for `size`, `cols` and `rows`
on `input`, `textarea` tags.

See rails/rails@3384ee2
(part of rails/rails#5366)

Signed-off-by: Alex Coles <alex@alexbcoles.com>
a2e02d8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment