Posted by yched on February 7, 2009 at 1:41am
5 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | field system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Attached patch reintroduces field styling (field.css was not being called anymore, and still had node-specific selectors).
It also overhauls a little the HTML generated for multiple fields on edit forms (d-n-d table + add more button), to something more inline with the other form elements on the page (proper .form-item class for consistent margins, <label> tag instead of just a styled <th>)
+ lol for the ahah-new-content class that got renamed to ahah-new-field during the content.module -> field.module renaming spree :-)
See screenshots for a visual hint.
Comments
#1
Dude, where are my file attachments ?
Retry.
#2
Hem - last attempt for tonight.
#3
#4
The last submitted patch failed testing.
#5
#6
sigh, wrong patch.
#7
The last submitted patch failed testing.
#8
If testbot is being dense and I'm being a jerk for consistently mixing my own patches, we'll get nowhere pretty fast.
#9
Move CSS inclusion to hook_init() (like node.module does). Including it at form build time doesn't work when the form is taken out of the cache after failed validation.
#10
rerolled, just in case
#11
Rerolled, including the clear-block / clearfix rename.
+ bump - that patch is not a biggie, would be cool to have it in :-)
#12
I found a few more related fixes and a typo and missing widget settings for the Text module. This fix is needed to get the CCK UI working right so we can test the other field patches in the UI.
#13
The last submitted patch failed testing.
#14
Patch applies and seems to work. Frankly, Karen and yched are the only two people who *really* understand this part of the code, so if they say it is right, I'm not sure who else's review matters.
Clearly we need to train more people on the details of Field API form handling.
#15
Ooops. When I said the patch applies, I meant "after I accounted for the fact that Karen rolled it from the wrong directory." :-)
Re-rolled.
#16
I might be wrong but I believe that ...
+ '#prefix' => '<div id="' . $field_name_url_css . '-wrapper">',+ '#suffix' => '</div>',
#17
(We should totally add XHTML validation to SimpleTest.)
#18
(Re #17: Yes, automatic XHTML validation in SimpleTest would be great. Also, we should run it automatically on the redesigned d.o during development and post-launch.)
re #16: It does not appear to put a div inside a span. Here is the XHTML generated for a multi-value text widget on an edit form:
<?php<div id="field-sample-wrapper"><div class="form-item"><table class="sticky-header" style="position: fixed; top: 0px; width: 1010px; left: 290.5px; visibility: hidden;"><thead><tr><th class="field-label" colspan="2" style="width: 997.833px;"><label>Sample: </label></th> </tr></thead></table><table class="field-multiple-table sticky-enabled tabledrag-processed sticky-table" id="field_sample_values">
<thead class="tableHeader-processed"><tr><th class="field-label" colspan="2"><label>Sample: </label></th> </tr></thead>
<tbody>
<tr class="draggable odd"><td class="field-multiple-drag"><a class="tabledrag-handle" href="#" title="Drag to re-order"><div class="handle"> </div></a></td><td><div id="edit-field-sample-0-value-wrapper" class="form-item">
<input type="text" class="form-text text" value="sample field" size="60" id="edit-field-sample-0-value" name="field_sample[0][value]" maxlength="255"/>
</div>
</td><td class="delta-order" style="display: none;"><div id="edit-field-sample-0--weight-wrapper" class="form-item">
<select id="edit-field-sample-0--weight" class="form-select field_sample-delta-order" name="field_sample[0][_weight]"><option value="-3">-3</option><option value="-2">-2</option><option value="-1">-1</option><option selected="selected" value="0">0</option><option value="1">1</option><option value="2">2</option><option value="3">3</option></select>
</div>
</td> </tr>
<tr class="draggable even"><td class="field-multiple-drag"><a class="tabledrag-handle" href="#" title="Drag to re-order"><div class="handle"> </div></a></td><td><div id="edit-field-sample-1-value-wrapper" class="form-item">
<input type="text" class="form-text text" value="value 2" size="60" id="edit-field-sample-1-value" name="field_sample[1][value]" maxlength="255"/>
</div>
</td><td class="delta-order" style="display: none;"><div id="edit-field-sample-1--weight-wrapper" class="form-item">
<select id="edit-field-sample-1--weight" class="form-select field_sample-delta-order" name="field_sample[1][_weight]"><option value="-3">-3</option><option value="-2">-2</option><option value="-1">-1</option><option value="0">0</option><option selected="selected" value="1">1</option><option value="2">2</option><option value="3">3</option></select>
</div>
</td> </tr>
<tr class="draggable odd"><td class="field-multiple-drag"><a class="tabledrag-handle" href="#" title="Drag to re-order"><div class="handle"> </div></a></td><td><div id="edit-field-sample-2-value-wrapper" class="form-item">
<input type="text" class="form-text text" value="value 3" size="60" id="edit-field-sample-2-value" name="field_sample[2][value]" maxlength="255"/>
</div>
</td><td class="delta-order" style="display: none;"><div id="edit-field-sample-2--weight-wrapper" class="form-item">
<select id="edit-field-sample-2--weight" class="form-select field_sample-delta-order" name="field_sample[2][_weight]"><option value="-3">-3</option><option value="-2">-2</option><option value="-1">-1</option><option value="0">0</option><option value="1">1</option><option selected="selected" value="2">2</option><option value="3">3</option></select>
</div>
</td> </tr>
<tr class="draggable even"><td class="field-multiple-drag"><a class="tabledrag-handle" href="#" title="Drag to re-order"><div class="handle"> </div></a></td><td><div id="edit-field-sample-3-value-wrapper" class="form-item">
<input type="text" class="form-text text" value="" size="60" id="edit-field-sample-3-value" name="field_sample[3][value]" maxlength="255"/>
</div>
</td><td class="delta-order" style="display: none;"><div id="edit-field-sample-3--weight-wrapper" class="form-item">
<select id="edit-field-sample-3--weight" class="form-select field_sample-delta-order" name="field_sample[3][_weight]"><option value="-3">-3</option><option value="-2">-2</option><option value="-1">-1</option><option value="0">0</option><option value="1">1</option><option value="2">2</option><option selected="selected" value="3">3</option></select>
</div>
</td> </tr>
</tbody>
</table>
<div class="clearfix"><input type="submit" class="form-submit field-add-more-submit ahah-processed" value="Add another item" id="edit-field-sample-field-sample-add-more" name="field_sample_add_more"/>
</div></div></div>
?>
I don't see a span anywhere. The prefix and suffix are wrapping another div.
NOTE: There is currently a bug in the CCK UI; you can't create a multi-value field with it. I worked around the bug by changing the 'cardinality' column of a field in field_config table directly. :-)
#19
The bug in CCK UI is fixed and you can now use it to test these patches.
I found one more css change, the last few lines in this patch. I needed to make the css rule a bit more specific to get the width of text and number fields to display in the right width instead of spanning the whole width of the page.
I already know this patch will fail the bot's testing because it doesn't appear to come from the right location, but I have tried re-rolling it from every possible place I can think of and I get the same result. It is either a problem with Tortoise or my understanding of how to roll a patch, so I'll let someone else re-roll it into something the bot will like.
#20
The last submitted patch failed testing.
#21
Most of these changes just got committed in http://drupal.org/cvs?commit=181304. My last change to the .css got missed but it looks like everything else in this patch got committed there. I'll post a new patch with the incremental change in a moment.
#22
Here's another patch that will fail because it has the wrong root. I just spent an hour trying to find a way to get Tortoise to show the path in the file name and have given up, so someone will need to re-roll this patch for me.
Once re-rolled into a format the bot likes it should be a very simple no-brainer. The change is needed so the fields will obey the widget settings for the size of the input field.
#23
The last submitted patch failed testing.
#24
Same as #22, rolled from root.
I didn't participated in that followup patch, so I can set to RTBC.
#25
The version of this patch accidentally committed with #392686: Switch to serial primary keys added 'widget_settings' to the structure returned by hook_field_info() (at least in text.module) which is only supposed to return info about field types, not widgets.
#26
The last issue affects only the text module and is fixed in #372743: Body and teaser as fields, so if that gets in this is fixed.
#27
Fixed now.
#28
Automatically closed -- issue fixed for 2 weeks with no activity.