| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | forms system |
| Category: | task |
| Priority: | major |
| Assigned: | sun |
| Status: | closed (fixed) |
| Issue tags: | API addition, API clean-up |
Issue Summary
Having dealt with some tables in Drupal, I found them quite difficult to construct. That’s why I have written a Forms API implementation of tables.
Now, you can create tables directly in Forms API without the need for theme(‘table’) in custom theme functions.
<?php
$form = array(
'system' => array(
'#type' => 'table_table',
'#header' => array(
0 => NULL,
1 => array('data' => 'Name', 'field' => 'name', 'sort' => 'asc'),
),
0 => array(
'status' => array (
'#type' => 'checkbox',
'#default_value' => '1',
),
'name' => 'table',
),
1 => array(
'status' => array (
'#type' => 'table_cell',
'status' => array (
'#type' => 'checkbox',
'#default_value' => '1',
),
),
'name' => 'taxonomy',
),
),
);
?>In #type => 'table_table', you can specify the header of the table in the #header attribute. It takes a regular header structure as returned by tablesort_init(). Table sorting is fully supported (with pager_query and so on). You can now populate the rest of your #type => 'table_table with the rows. These rows (0, 1 in the example above) contain the cells (‘status’ and ‘name’ in the example).
There are three possibilities: If the cell is a string, it is expanded to array('#value' => $cell) and put into a table cell automatically. If it is an array (thus an arbitrary form element such as checkbox, radio or textfield), it is wrapped by a table cell. If it is a table cell, nothing specifically happens.
This behavior implies that you can place arbitrary form elements inside a table cell without writing specific theme functions that handle these. They are automatically generated by Forms API.
Attached is a module that implements the table. I’d like some people to check my code and if this approach makes sense. The module does also include a sample table at admin/settings/table that maps the SQL table ‘system’ to a table that can be browsed.
Related issues
- Edge: #991454: Add element #type table (with tableselect and tabledrag support)
- #950034: Using the same source for main/secondary with a custom menu doesn't work (original base for Edge module implementation)
- #453400: "-wrapper" HTML ID not output for #type checkboxes/radios (checkboxes automatically get an HTML ID, but radios do not)
- #1248940-3: "Manage fields" screens : broken table layout (empty cells)
- #1260860: Rework language list admin user interface (#header)
- #1823574: Improve the Views Bulk Operations (VBO) that is in core
Change records for this issue
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| table.module | 4.79 KB | Ignored: Check issue status. | None | None |
Comments
#1
I forgot to say that I will go on and convert this module to a patch for Drupal core that replaces theme('table') and converts all tables to this model removing the need for specific theme functions.
#2
Now minus the
var_export.#3
I agree with the theory, and tried it earlier this year
The result was a nice big demo and something very similar to what you did.
(I called the elements formtable, tr and td. It's not as tied to the tablebuilder as yours, it chooses Form API - like syntax instead ... but hence doesn't support tablesort.)
My implimentation (80% docs) is attached.
I'm using it in several places already.
Everything I glanced at in your code made sense (note glanced, it's past my bedtime), but the expand function looks a bit inelegant.
maybe I'll sleep on it.
#4
It sounds like you propose to remove them theme_table() function to which I say ouch. Not everything the uses that function is a form.
#5
I like the general idea a lot, but why not just extend FAPI so that drupal_render() knows to render the elements in each row and then call theme('table')?
#6
I had a conversation with chx yesterday evening and he told me that this data model (rows instead of columns) could result in quite some difficulties using
#type => checkboxes.@nevets you can build your tables using this model even without drupal_get_form(). Just construct the array in the same way and use drupal_render() to get the HTML representation.
#7
I like the idea and the implementation, but I doubt that it warrants a new core module. Maybe a .inc file? The table_elements() return value could then be added instead to system_elements(). We've already got tablesort.inc. Maybe that should be changed to table.inc, and these functions included there.
#8
Oh, I never intended it to be a module. That is just for development/reviewing purposes becasue it makes it easier to show what it does without the need to reroll patches all the time (see #2). If this is considered for inclusion in core, I will happily provide a patch. Note that this definitely won't make it into 4.8/5.0.
#9
Minor correction. The pain is with radio buttons and no matter what you do, if you use #type radio instead of #type radios you need advanced mojo.
'table_row' => array(),this is unnecessary. hook_elements is for defaults, if you have no defaults, omit it, noone checks whether a specific type is defined in any hook_elements.#10
It's true that defaults is our only current core use. But e.g. in formbulder.module we need a way to answer the question "What are all the defined form element types (so we can offer a UI for building them)?". It's best IMO if we return all eliment types, whether or not they have defaults.
And we seem to do this already in system_elements:
<?php$type['item'] = array();
?>
#11
THis should not be a separate module.
I wrote one of these awhile ago http://cvs.drupal.org/viewcvs/drupal/contributions/sandbox/drumm/extra_e...
#12
I like Drumms version.
#13
+1
#14
I have started developing a much more enhanced version which when finished will integrate into the formapi.
see http://cvs.drupal.org/viewcvs/drupal/contributions/sandbox/gordon/table_...
#15
Feature freeze requires this to be sent to D7.
#16
maybe you could get some inspiration/code from: http://drupal.org/node/103171
#17
subscribe. we'll *have* to get this in for D7
#18
I did some quick testing of drumm's form element and still works in 5 (it was built for 4.7). I only had to change form_render => druapl_render(). See his code at http://cvs.drupal.org/viewvc.py/drupal/contributions/sandbox/drumm/extra.... I think this is a nice, small implementation.
#19
subscribing
#20
Lots of different implementations by various developers. Can we revive this and get a consensus on the best patch to take forward?
#21
See #297893: #theme Form elements into tables ... I don't know if my patch is a duplicate of this, or something kinda different.
#22
Anyone want to pick this up? we have #tableselect now, but thats a bit different.
#23
Another related discussion with a nice patch is here: http://drupal.org/node/528932
#24
Here is a work in progress, proof-of-concept patch, that aims at properly implementing theme_table() in the renderable array model.
The basic structure of a table is now:
<?php$table = array(
'#type' => 'table',
'#columns' => array(
'col1' => array(
'#markup' => t('Column 1'),
),
'col2' => array(
'#markup' => t('Column 2'),
),
),
'row1' => array(
'col1' => array(
'#markup' => t('Cell 1'),
),
'col2' => array(
'#markup' => t('Cell 2'),
),
),
);
?>
The table is rendered in the order of the columns in the #columns parameter, and only columns that are in the #columns parameter are rendered. That should allow to add a column and reorder columns easily.
#25
The last submitted patch failed testing.
#26
Thanks for working on this.
- '#theme' => 'table',- '#header' => $header,
- '#rows' => $rows,
+ '#markup' => format_table($header, $rows),
The problem here is that we have a string by the time any alter hook comes along. exactly when is a module supposed to come in an add its extra column? if thats not possible, then this patch is a regression.
#27
@Moshe: all the format_table() are temporary back-ward compatible hacks. I have done a full conversion in node_revision_overview(), but I'm still trying to figure out if this is the correct API, from a DX perspective. Feedback is more then welcome.
#28
Thanks. I just looked at node_revision_overview(). Seems a bit verbose to me. The #tableselect syntax looks a bit more terse. See http://heine.familiedeelstra.com/new-tableselect-form-element-in-core
#29
subscribe.
I am closing http://drupal.org/node/528932 in favour of this thread.
I've written a patch which declares a new FAPI element call 'tableform'. It will theme a form structure using theme_table. My goal is to get this element included in Drupal 7, so any feedback will be greatly appreciated.
Example of usage:
<?php
$form = array(
'table' => array(
'#type' => 'tableform',
'#header' => array('col1', 'col2', 'col3'),
'#caption' => 'Hello',
'#tabledrag' => array(
'action' => 'order',
'relationship' => 'sibling',
'group' => 2,
'hidden' => false,
),
),
);
for($i = 0; $i < 10; $i++) {
$form['table']["row-$i"] = array(
'col1' => array(
'#type' => 'checkbox',
'#default_value' => rand(0,1),
),
'col2' => array(
'#type' => 'checkbox',
'#default_value' => rand(0,1),
),
'col3' => array(
'#type' => 'select',
'#default_value' => 0,
'#options' => array_combine(range(0,10),range(0,10)),
),
);
}
$form['table']['row-1']['#draggable'] = false;
?>
#30
Heh. That's not unlike what I did back in 2006.
Where do the cells #attributes end up? the code looks like it goes on the td, but we also need behaviour would be for it to go on the element.
It'll need a lot of testing to make it into D7, I'm thinking.
I like the idea, of course, but good luck!
#31
@dman Actually the #attributes apply to the form element itself, here is a patch that will use '#cell_attributes' to apply attributes the
Example:
<?php
$form = array(
'table' => array(
'#type' => 'tableform',
'#header' => array('col1', 'col2', 'col3'),
'#caption' => 'Hello',
'#tabledrag' => array(
'action' => 'order',
'relationship' => 'sibling',
'group' => 2,
'hidden' => false,
),
),
);
for($i = 0; $i < 10; $i++) {
$form['table']["row-$i"] = array(
'col1' => array(
'#type' => 'checkbox',
'#default_value' => rand(0,1),
'#attributes' => array(
'class' => array('test_field_class'),
),
'#cell_attributes' => array(
'class' => array('table_cell', 'test_cell_class'),
),
),
'col2' => array(
'#type' => 'checkbox',
'#default_value' => rand(0,1),
),
'col3' => array(
'#type' => 'select',
'#default_value' => 0,
'#options' => array_combine(range(0,10),range(0,10)),
),
);
}
$form['table']['row-1']['#draggable'] = false;
?>
#32
+++ includes/form.inc 28 Aug 2009 18:16:22 -0000@@ -2197,6 +2197,224 @@ function form_process_tableselect($eleme
+ if(is_int($tabledrag['group'])) {
All the
if(should be modified intoif (to comply with coding standards. Many occurences in the patch.+++ includes/form.inc 28 Aug 2009 18:16:22 -0000@@ -2197,6 +2197,224 @@ function form_process_tableselect($eleme
+ $attributes['id'],
+ $tabledrag['action'],
+ $tabledrag['relationship'],
+ $tabledrag['group'],
+ isset($tabledrag['subgroup']) ? $tabledrag['subgroup']:NULL,
+ isset($tabledrag['source']) ? $tabledrag['source']:NULL,
+ isset($tabledrag['hidden']) ? $tabledrag['hidden']:TRUE,
These array items have whitespace at the end of the line which should be removed.
#33
@TheRec Thanks for the feedback, here is the patch after its been run through the coder module. I've adjusted the styling to fit the coding standards.
#34
In a similar theme, the row will need #attributes also, I'm thinking.
And formbuilder really really expects every element_children() to have a #type, so it should be
$row[#type]='tablerow'I guess.Right now, it can be guessed from context, but all the other form recursion routines may be surprised to find an untyped element in the tree.
#35
@dman The patch provides functionality for the row to have #attributes (See example below). In my experience with working with the form api, any element that does not provide a #type will be assigned type #type of 'markup'. I've been creating form with empty child elements (who contain subchildren) for some time, it all seems to work so long as the empty child has the #tree property set.
My focus on this patch has been to make the process of placing a new/existing form in a table as simple as possible. I specifically chose to omit the creation of a 'tablerow' as (without tableform) would not provide any functionality and to keep the FAPI docs free of these types. What would the benefit of creating a 'tablerow' element?
Setting #attributes for a row:
<?phpfor($i = 0; $i < 10; $i++) {
$form['table']["row-$i"] = array(
'#attributes' => array(
'class' => array('rowclass'),
),
'col1' => array(
'#type' => 'checkbox',
'#default_value' => rand(0,1),
'#attributes' => array(
'class' => array('test_field_class'),
),
'#cell_attributes' => array(
'class' => array('table_cell', 'test_cell_class'),
),
),
'col2' => array(
'#type' => 'checkbox',
'#default_value' => rand(0,1),
),
'col3' => array(
'#type' => 'select',
'#default_value' => 0,
'#options' => array_combine(range(0,10),range(0,10)),
),
);
}
?>
#36
Cool on the #attributes then, I must have missed where that got passed in.
The row is not 'markup' and shouldn't assume it is. Markup has a #value, and does not have children. It may happen to work for you right now, but it leaves an assumption floating. Conceptually, I think this construct should be labelled clearly and exposed so form_alter, theming, and other code can see what's going on. For now, it's just a label (possibly redundant), but it is important enough to be explicit about. For code clarity.
Saying that a row is a row may be marginally more strict, and marginally less simple, but it's clearer and more consistent with the rest of FAPI. IMO.
And it can't hurt what you've already got...
#37
@dman I agree with your statement. The need for a tablerow type would not hurt, of course, I'll probably implement a tablecell type too as this is a cleaner solution than using the '#cell_attributes' property anyway.
I figure I'll have the process callback add the tableform_row and tableform_cell types as needed. This way we still get the flexibility I was looking for in the beginning and the extendability you speak of. I'll see if I can hammer this code out this weekend.
#38
Cool.
It would be OK if the tablecell was optional, if there is only a single form element to render. That will be the 'simple' common case, but if you want to nest more elements, or be explicit, then a tablecell wrapper would be handy. Like the tablebuilder currently does, allowing either a string or an array with data and attributes, depending on the need.
I think this functionality has been missing for ages (ever since I wanted it in 4.7!) so I hope it gets support!
#39
I'm not sure if everyone here knows that we already can put tables into a render() structure. Search for $page['tracker'], for example. I don't see much need for this patch anymore.
#40
@moshe weitzman: you can place tables in a render() structure, but you can't place a render() structure into a table. This issue is dedicated to that.
@vangorra: Why are you hijacking this issue? Have you seen that there is an already fairly advanced patch in #24?
#41
@Damien Tournoud: It is not my intent to hijack this issue. It was suggested to me to merge my issue with this one in order to get more eyes on the patch. I did see the patch in post #24 and see that we are solving the same problem with differing approaches.
It is my opinion that multiple suggestions to the same solutions helps foster open source development and in the end, can only benefit the project as a whole. I felt it necessary to post my patch as it compliments the existing theme functions rather than modifying them, a strategy that avoids the need to modify existing core and contrib modules.
#42
Taking dman's feedback into account, I have refactored the tableform code to better utilise the theme API. The patch includes 3 FAPI elements: tableform, tableform_row, tableform_cell. I'm looking forward to anybody's feedback.
Known Issues:
Rough Element Doc:
tableform
A form element that generates a table of other form elements. All children of a tableform should be of type tableform_row. If the type is ommited, it will be set. If the type is not a tableform_row, it will be removed.
Properties:
tableform_row
An element that represents a row for a tableform. Children can be a FAPI elements. If child type is not specified, then it assumed that it is a tableform_cell and set as such.
Properties:
tableform_cell
Provides a container for multiple FAPI elements to be included in the same cell of a table.
#43
Coding standards review... hopefully it will help :)
+++ includes/form.inc 31 Aug 2009 15:58:09 -0000@@ -2196,6 +2196,232 @@ function form_process_tableselect($eleme
+ * Format a table based on a form element
Should be "Formats"
+++ includes/form.inc 31 Aug 2009 15:58:09 -0000@@ -2196,6 +2196,232 @@ function form_process_tableselect($eleme
+ else if (isset($element[$element['#header']])) {
...
+ else if ($tabledrag['auto_config_group']) {
According to coding standards it should be elseif without a space.
+++ includes/form.inc 31 Aug 2009 15:58:09 -0000@@ -2196,6 +2196,232 @@ function form_process_tableselect($eleme
+ * Prepares a formtable and its rows to be themed. Also initializes the
+ * tabledrag options.
The short description of the function should be one sentence of less than 80 characters. You can be more precise under, after skipping a line.
+++ includes/form.inc 31 Aug 2009 15:58:09 -0000@@ -2196,6 +2196,232 @@ function form_process_tableselect($eleme
+ * @param $element
...
+ * @param $element
...
+ * @param $element
Missing parameter description.
+++ includes/form.inc 31 Aug 2009 15:58:09 -0000@@ -2196,6 +2196,232 @@ function form_process_tableselect($eleme
+ // Ensure that the children are tableform_row types
Missing a dot a the end of the line.
+++ includes/form.inc 31 Aug 2009 15:58:09 -0000@@ -2196,6 +2196,232 @@ function form_process_tableselect($eleme
+ // Set the #type for cell that should have it. (if the dev is being lazy)
"(if the dev is being lazy)", maybe this does not have its place in core ;)
This review is powered by Dreditor.
#44
@TheRec Thank you for the review. It looks like the coder module can't catch everything. Also, the Dreditor is pretty wicked.
I've made changes and cleaned the code a bit more in the process. Hopefully, I didn't stray away from the standards again.
#45
Note, with these patches #565496: Allow dynamic attaching of other types of stuff to render() structures and #564880: Change drupal_add_tabledrag() function parameters to an $options array, draggable tables might be built this way:
$table['#attached_tabledrag'][] = $tabledrag_options_array;#46
@dropcube: Both those patches you propose would really compliment the formtable as well as any other FAPI elements. Have you had a chance to work with the formtable patch? I would really appreciate your feedback.
#47
Since #565496: Allow dynamic attaching of other types of stuff to render() structures has been committed, I will implement a $tableform['#attachment']['tabledrag'] to the tableform element. On an additional note, this newest version of the patch cleans thing up just a bit more. Stay tuned for attachment support. :)
**Quick update. It is not elegant to implement the tabledrag '#attachment' property until #564880: Change drupal_add_tabledrag() function parameters to an $options array is approved.
#48
The last submitted patch failed testing.
#49
subscribing
#50
sub
#51
Tagging absolutely critical clean-ups for D7. Do not touch this tag. Please either help with this or one of the other patches having this tag.
#52
I'm new to this thread. This could help #616240: Make Field UI screens extensible from contrib - part II very much...
It looks like, since patch #44, the only pending remark is:
"we could add tabledrag through
$tableform['#attached']['tabledrag'] = $options;instead of$tableform['#tabledrag'] = $options;" - which requires #564880: Change drupal_add_tabledrag() function parameters to an $options array"(comment #47)
Is that really a blocker, or can it be changed in a followup ?
Is any of the people following this thread confident enough to mark this RTBC ?
#53
I'm also new to this thread and have not read it yet, but am subscribing.
As an FYI and crosslink, as of http://drupal.org/node/602522#comment-2219964, _theme_table_cell() now has it that if a cell is an array with a 'data' property, and the value of that 'data' property is an array, then it gets drupal_render()'d. This was needed for that issue, but maybe some of you have ideas for how that can be made better, especially with the work done on this issue.
#54
I see nothing here that can't live in contrib for Drupal 7. It's a nice new feature for Drupal 8.
#55
Except it would be really great for core Field UI - #616240: Make Field UI screens extensible from contrib - part II (required to let fieldgroup tie into the UI from contrib)
But yeah, doesn't seem like it will happen in D7 now.
#56
http://drupal.org/project/table
#57
I use a modified version of http://drupal.org/project/formtable (I modified it to work w/D6). It works well for me. I agree that there should be some sort of implementation of this functionality in core.
#58
Working implementation in Edge:
#991454: Add element #type table (with tableselect and tabledrag support)
Assigning to myself.
#59
This would be awesome.
#60
subscribing
#61
Last patch from 2009, probably needs a reroll.
Related to #1276908: Administrative tables are too wide for smaller screens that might add some more options to the new type being discussed here.
#62
I just committed the latest known code to Edge: #991454-23: Add element #type table (with tableselect and tabledrag support)
#63
+1, This will help with 80+ admin screens for D8
@sun Where is s current state of the code?
#64
For D8, needs responsive table support. I can help.
#65
Here we go.
#66
The last submitted patch, type.table_.65.patch, failed testing.
#67
Tested and works well :)
Adding the tabledrag code and all shouldn't that happen in a preprocess? I've got #1812374: Add preprocess to theme_table function to simplify it to add all the responsive table stuff and most of the processing out of the theme function. The preprocess would be easier to override for contrib no? Maybe there is an actual reason as to why it's not in a preprocess can I ask why?
#68
woops.
#69
re: #67 @nod_:
Thanks for testing! And yeah, I'm aware of the issue you pointed to and will get back to it soon. :)
Technically you're making a very good point, but for this issue/patch, it doesn't really matter, since
1) The main feature and aspect of #type 'table' is to introduce a form/render structure that is compatible with render arrays, which is easy/straightforward to work with from a DX perspective. While being there, also eliminate the secondary tableselect and tabledrag concepts that are separate currently and require specialized implementations for no good reason. As a developer, I want to be able to just simply specify #type 'table' and get my job done. :) If I'm doing this within a form context, then all I should need is a corresponding form submission handler for my form (like for any other form), and again, just be done with it. ;)
2)
theme_table()is actually one of the theme functions in core that is too generic (and too complex) to be useful. In my entire career as as Drupal themer, I never faced a single use-case in which it would have made sense to override that theme function. Simplifying it and moving stuff into preprocess might change that situation, but in the end, the ultimate question remains: What, exactly, could you override in a theme? ;) A table is a table, right? HTML tables haven't really changed since 1995, and you'd have to implement quite the horror of complex variable processing in order to do anything special about a particular table. ;) So in the end, the most I can make sense of from a theming perspective is to entirely swap out #type/#theme 'table' with something else, so as to not render a table where normally a table would appear. Thus, by introducing #type 'table' and inherently eliminating a ton of "table form theme functions" throughout core, I think we actually make a big leap forward towards that.That said, I did not verify yet, whether it is possible to simply swap out the theme function but still keep the form/render elements as well as tableselect stuff working. OTOH, that sounds like a nice follow-up issue, too. :)
In light of that, I think our actual goal should be to decouple the tabledrag functionality into a universal draggable functionality (potentially leveraging native HTML5 implementations if available), instead of trying to optimize where & how it happens for tables. :)
Makes sense? :)
Fixed tableselect validation of #type table needs to be limited to form buttons that rely on it.
What I think could be improved is the #attached[tabledrag] handling. Perhaps we should move that into a custom #tabledrag property, equivalent to the #tableselect property?
#70
Thanks for reviving this issue.
+++ b/core/includes/theme.inc@@ -1837,6 +1837,98 @@ function theme_breadcrumb($variables) {
+ * $form['table'] = array(
+ * '#type' => 'table',
+ * '#header' => array(t('Title'), array('data' => t('Operations'), 'colspan' => '1')),
+ * // Optionally, to add tableDrag support:
+ * '#attached' => array(
+ * 'tabledrag' => array(array('order', 'sibling', 'thing-weight')),
+ * ),
+ * );
I'm not a fan of having callers set #attached['tabledrag']. Why not have them set #tabledrag = TRUE? Thats what I would have thought after reading the hook_element_info() declaration. It should be drupal_pre_render_table() that converts that to #attached['tabledrag'] or whatever. Thats my first thought, anyway.
Whatever we do above for tabledrag, we should also do for #responsive. We currently add stylesheets for responsive and tabledrag from within a theme function and that completely breaks when render cache is used (i.e. #cache). So, I consider this issue a nice bug fix.
+++ b/core/includes/theme.inc@@ -1837,6 +1837,98 @@ function theme_breadcrumb($variables) {
+function drupal_pre_render_table(array $element) {
+ foreach (element_children($element) as $first) {
This function has nice Doxygen, but it doesn't elaborate on why we need to transform #rows. I'm surprised, because we attach #rows to #theme=table elements today and it seems to work fine.
#71
Yeah, we can do #tabledrag instead of the custom #attached.
However, we cannot do TRUE, since
drupal_add_tabledrag()1) requires additional arguments and 2) it needs to be invoked more than once with different arguments for some tables (mostly those that allow a tree-dragging; i.e., not only up/down, but also moving within hierarchies).I'm not up to speed with the responsive table columns yet. Since that is part of #theme table already, I wonder whether we actually need to do anything about that here. But I'll look into it.
Regarding the #pre_render transformation of #rows, that's required since
theme_table()only supports render array elements in the'data'subkey of table cells. This transformation is actually the core of this issue/patch. That's also the only way to specify attributes for the table cell. The processing and rendering withintheme_table()could be improved (potentially taking the same approach as the newtheme_item_list()), but I do not consider that to be part of this issue.Changed custom #attached handling into formal #tabledrag property.
#72
That change looks good to me.
@sun - if you look into #responsive and decide to defer it to a follow-up, thats fine. I'm happy to RTBC this with or without #responsive handling.
#73
The #tabledrag move looks great, but how do we document the expected arguments then ?
(maybe just "@see drupal_add_tabledrag()" ?)
#74
So. I looked into two things:
1) I hacked the filter admin table in a couple of ways to see whether it is possible to skip the table rendering; e.g., by overriding #theme to NULL. That worked as expected, and I was still able to submit the form. The tabledrag obviously did not work, since the entire tabledrag.js is bound to tables right now. As mentioned before, we should definitely consider to abstract this into a more generic draggable.js - ideally only as polyfill if native HTML5 support is not available. :)
2) Exploring 1) will lead us to a whole new problem space of horizontal extensibility of render elements that is not covered by core yet (even though render elements are extensible in all dimensions already, they are not "pluggable" in a "plugin" sense). In other words: Processing and applying #tabledrag and #tableselect for a table is one thing. Applying them to arbitrary render structures is a whole new, and very very interesting + fun thing. :)
3) I also looked into #1276908: Administrative tables are too wide for smaller screens, and from what I can see, the change only affected the #header column definition of tables. This definition is not changed by this patch here though - it is the #theme table that is still using #header, and #type table does not influence that.
In turn, I think that means we're feature-complete here. :)
Added docs for #tabledrag property.
#75
Sure, we can proceed with this. I will add #responsive in a follow-up.
#76
#74: type.table_.74.patch queued for re-testing.
#77
Wouldn't it make more sense to just focus on a #type=>table?
You could then add #type=>tabledrag and #type=>tableselect as implementations of #type=>table to Core, instantly providing two good examples of "how it's done" for other developers to look at.
It would also have the benefit of being 'clean': tabledrag and tableselect would be just 'an' implementation of render elements inside a table. The hook_element_info() will be cleaner and so will the functions behind it.
Thought I'd add this to the discussion before this patch goes into Core..
#78
Nice patch. Happy to commit it once we're below thresholds again.
#79
Awesome, I would like to go ahead and directly start using this over here: #1855402: Add generic weighting (tabledrag) support for config entities (ConfigEntityListController)
#80
I disagree with this being classified as a feature request. It's a conversion of something that existed before render arrays into a render array. I'm tempted to mark it a "task", except not doing so, cause maybe sun had a reason for the status change in #65?
#81
The functionality was denied post D7 feature freeze, and thus had to live in the contributed Edge module. On these grounds, I classified this as a feature, and ensured that there is a 100% ready patch on Dec 1st, 2012.
#82
#74: type.table_.74.patch queued for re-testing.
#83
Would be great to get this in soon, so we can proceed with #1855402: Add generic weighting (tabledrag) support for config entities (ConfigEntityListController), which in turn would help #599770: Clean up the contact category listing UI: Allow to set the default category and weights on the listing page.
#84
Agreed. #81 doesn't convince me that it's a feature. Reclassifying per #80.
#85
#74: type.table_.74.patch queued for re-testing.
#86
Another issue that is blocked on this: #1872870: Implement a RoleListController and RoleFormController
#87
#88
Committed and pushed to 8.x.
This will need a change notice.
#89
Just want to say thanks guys. We've needed this for a long time. This allows for a nice cleanup of #1833716: WYSIWYG: Introduce "Text editors" as part of filter format configuration.
From my initial analysis of the code, it doesn't look like this easily allows modules that form_alter() a table to easily add an additional column (other than at the end of the table), since the order of the table is based on the declaration order, rather than the order of the header or some weight system. Is this the case? If it's not, I may make a followup to make the column order weighted in some way.
#90
Change notice: http://drupal.org/node/1876710
Follow-up issues:
#1876712: [meta] Convert form tables in core to new #type 'table'
#1876714: Remove #type 'tableselect'
#1876718: Allow modules to inject columns into tables more easily
#1876720: Improve responsive table support for #type 'table'
#91
Automatically closed -- issue fixed for 2 weeks with no activity.