Needs review
Project:
Edge
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
18 Feb 2011 at 13:56 UTC
Updated:
25 May 2011 at 02:11 UTC
Jump to comment: Most recent file
I haven't been able to look at the latest patch in #991454: Add element #type table (with tableselect and tabledrag support) but I promise to review it once I have the time, in case it hasn't gone in until then.
For now, here is a rather simple patch that adds the ability to retain attributes such as colspan for cells.
It definitely works, because I'm currently testing this with the 7.x-2.x branch of Clock module.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | cell_row_attributes.patch | 1.18 KB | tstoeckler |
| #7 | cell_row_attributes.patch | 1.18 KB | tstoeckler |
| #3 | cell_attributes.patch | 953 bytes | tstoeckler |
| #1 | cell_attributes.patch | 953 bytes | tstoeckler |
Comments
Comment #1
tstoecklerAnd the patch...
Comment #2
sunhm. That's definitely the simplest approach, but it duplicates the #attributes of the cell content markup, in case the cell's content is "simple"; i.e.,
$element[$first][$second]contains the cell content directly (no further sub-keys).That might be acceptable, but perhaps not. It's definitely not acceptable when considering
#attributes['id']...An alternative approach would be to support a custom
#wrapper_attributesor#cell_attributesproperty.Comment #3
tstoecklerWhile #wrapper_attributes definitely sounds nice, I'd say let's go with a simple approach that works for this now.
Introducing #cell_attributes. I also went for #row_attributes for consistency. The risk of confusion/duplication isn't very big for rows, but I thought why not. I'll reroll without that, though, if you don't like it.
I tested the basic functionality and it works.
Comment #4
tstoecklerJust to make that clear, I think #wrapper_attributes is the right direction in the long run, I just don't know how to implement it in a generic fashion. Or of course, we could just call what I called #row_attributes and #cell_attributes #wrapper_attributes, I don't know. I'm not really 100% comfy with the Form/Render API so I'll basically roll whatever you say...
Comment #5
tstoecklerWow, I just realized I uploaded the same patch twice...
Don't know if I can still find the right one, otherwise I'll have to re-code. Anyway, needs work.
Comment #6
tstoecklerAlso, assigning so that I don't forget.
Comment #7
tstoecklerHere we go.
Comment #8
tstoecklerRetesting