This project is archived and is in readonly mode.
DEFAULT_*_OPTIONS cause inconsistencies in FormHelper
Reported by Victor Costan | August 8th, 2010 @ 12:33 AM
For example, DEFAULT_FIELD_OPTIONS is {'size' => 30}, which
causes text_field to emit a size="30" HTML attribute. The
inconsistencies are
in the RDoc; e.g., the 2nd and 3rd examples in texxt_field's RDoc
are wrong in relation to FormTagHelper, which doesn't use these
defaults; for example, un-styled text fields generated with
text_field look different from those generated with
text_field_tag
I would also argue that these defaults violate the principle of least surprise. They're also bad design practice, because they put presentation details in mark-up. Width and height should be specified via CSS.
I'll be happy to produce a patch that eliminates these defaults, if core likes the idea, and it would be accepted.
The DEFAULT*OPTIONS are constants and frozen hashes, so plug-ins shouldn't rely on being able to modify them. So I hope that removing them won't break too much 3rd party code.
Comments and changes to this ticket
-
Victor Costan August 8th, 2010 @ 01:55 AM
Added a patch that removes DEFAULT*OPTIONS in FormHelper. Will follow up with a patch that sets input field sizes in scaffold.css.
-
Victor Costan August 8th, 2010 @ 02:09 AM
The feature removed by the patch above is re-added by this patch, using CSS.
-
Victor Costan August 10th, 2010 @ 06:12 AM
The comment above is spam, please remove. The user also seems to be a serial spammer.
-
Victor Costan August 10th, 2010 @ 02:53 PM
- Tag changed from “actionview” to “actionview, patch”
Can someone please look at this ticket before it gets overrun with spam?
-
Andrea Campi October 10th, 2010 @ 11:21 PM
I like your idea so I will gladly upvote it, but the patch doesn't apply anymore. Can you reroll it?
-
-
-
Victor Costan October 11th, 2010 @ 01:12 AM
The CSS patch still applies cleanly for me. Is there anything else I should do?
-
Ryan Bigg October 11th, 2010 @ 02:59 AM
- Tag cleared.
- Importance changed from “” to “Low”
Automatic cleanup of spam.
-
-
Victor Costan October 11th, 2010 @ 03:01 AM
@Ryan: thank you for the clean-up!
Please let me know if there is anything else I can do in support of this patch!
-
-
Andrea Campi October 11th, 2010 @ 07:19 AM
+1; I verified this works by creating a new scaffold, and tests pass.
-
-
Santiago Pastorino February 2nd, 2011 @ 04:48 PM
- State changed from “new” to “open”
This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.
Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
-
Santiago Pastorino February 2nd, 2011 @ 04:48 PM
- State changed from “open” to “stale”
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
Andrea Campi
Jeremy Kemper
Ryan Bigg
Victor Costan