Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

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

parndt
Copy link
Contributor

@parndt 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!

… default cols and rows options from textarea.
@josevalim
Copy link
Contributor

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

@carlosantoniodasilva
Copy link
Member

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?

@fxn
Copy link
Member

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.

@chriseppstein
Copy link
Contributor

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

@@ -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">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why leave the rows here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the example specifies :rows

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@josevalim
Copy link
Contributor

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

@parndt
Copy link
Contributor Author

parndt commented Mar 11, 2012

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

josevalim added a commit that referenced this pull request Mar 11, 2012
Fixes #5324 by removing default size options from input:text and default cols and rows options from textarea.
@josevalim josevalim merged commit 6c0d5a1 into rails:master Mar 11, 2012
myabc added a commit to opf/openproject that referenced this pull request May 1, 2015
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants