Add element #type table and merge tableselect/tabledrag into it

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.

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.

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')?

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.

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.

hook_elements is for defaults, if you have no defaults, omit it

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:

  $type['item'] = array();

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...

I like Drumms version.

+1

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_...

Feature freeze requires this to be sent to D7.

maybe you could get some inspiration/code from: http://drupal.org/node/103171

subscribe. we'll *have* to get this in for D7

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.

subscribing

Lots of different implementations by various developers. Can we revive this and get a consensus on the best patch to take forward?

See #297893: #theme Form elements into tables ... I don't know if my patch is a duplicate of this, or something kinda different.

Anyone want to pick this up? we have #tableselect now, but thats a bit different.

Another related discussion with a nice patch is here: http://drupal.org/node/528932

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:

$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.

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.

@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.

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

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:

$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;

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!

@dman Actually the #attributes apply to the form element itself, here is a patch that will use '#cell_attributes' to apply attributes the

Example:

  $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;

+++ 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 into if ( 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.

@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.

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.

@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:

  for($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)),
      ),
    );
  }

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...

@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.

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!

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.

@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?

@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.

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:

• None at the moment, but I continue to test.

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:

• #caption: The table caption.
• #sticky: Make the header sticky.
• #header: The table header. Can be an array (what theme_table will handle) or the index of one of the table's children.
• #tabledrag: An array of tabledrag options. (@see drupal_add_tabledrag)
  • id - Optional, will use element id of it was not provided as an attribute.
  • action - Required.
  • relationship - Required
  • group - The class of the element that handle the weights. If an integer or array or integers is provided, a class will be generated and the numbers gived will be used to locate the field in the row. Ex: group => array(2,1) will mark the 3rd cell, 2nd element with the generated class.
  • subgroup - Optional
  • source - Optional
  • hidden - Optional (Default:TRUE)
  • limit - Optional

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:

• #draggable: Determines if the row is draggable via tabledrag (Default:TRUE)

tableform_cell

Provides a container for multiple FAPI elements to be included in the same cell of a table.

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.

@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.

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;

@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.

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.

subscribing

sub

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.

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 ?

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.

I see nothing here that can't live in contrib for Drupal 7. It's a nice new feature for Drupal 8.

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.

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.

Working implementation in Edge:
#991454: Add element #type table (with tableselect and tabledrag support)

Assigning to myself.

This would be awesome.

subscribing

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.

I just committed the latest known code to Edge: #991454-23: Add element #type table (with tableselect and tabledrag support)

Updated issue summary.

+1, This will help with 80+ admin screens for D8

@sun Where is s current state of the code?

For D8, needs responsive table support. I can help.

Here we go.

1. Ported #type 'table' from Edge module into core.
2. Converted filter_admin_overview() to #type table. (draggable showcase)
3. Converted node_admin_nodes() to #type table. (tableselect showcase)

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?

woops.

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?

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.

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 within theme_table() could be improved (potentially taking the same approach as the new theme_item_list()), but I do not consider that to be part of this issue.

Changed custom #attached handling into formal #tabledrag property.

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.

The #tabledrag move looks great, but how do we document the expected arguments then ?
(maybe just "@see drupal_add_tabledrag()" ?)

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.

Sure, we can proceed with this. I will add #responsive in a follow-up.

#74: type.table_.74.patch queued for re-testing.

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..

Nice patch. Happy to commit it once we're below thresholds again.

Awesome, I would like to go ahead and directly start using this over here: #1855402: Add generic weighting (tabledrag) support for config entities (DraggableListController)

Happy to commit it once we're below thresholds again.

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?

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.

#74: type.table_.74.patch queued for re-testing.

Would be great to get this in soon, so we can proceed with #1855402: Add generic weighting (tabledrag) support for config entities (DraggableListController), which in turn would help #599770: Clean up the contact forms listing UI: Allow to set the default category and weights on the listing page.

Agreed. #81 doesn't convince me that it's a feature. Reclassifying per #80.

#74: type.table_.74.patch queued for re-testing.

Another issue that is blocked on this: #1872870: Implement a RoleListController and RoleFormController

Committed and pushed to 8.x.

This will need a change notice.

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.

Change notice: http://drupal.org/node/1876710

Follow-up issues:

#1876712: [meta] Convert all 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'

I just hit this in D7 contrib again and I don't see any reason, why we cannot backport this. The patch that went in here is completely backportable without much effort and it would be a pure API addition. I wouldn't even suggest to convert core tables, only introduce the element for contrib use.

Here we go.

Oops, empty patch.

I just copied the code from the current D8 codebase, there's a small difference to the committed patch, but nothing that's not D7 compatible.

Ahh, I'm sorry. I messed that up again. #94 does not include the mentioned different from this patch and current D8. For context, this is the hunk:

--- a/includes/theme.inc
+++ b/includes/theme.inc
@@ -1876,7 +1876,15 @@ function drupal_pre_render_table(array $element) {
     foreach (element_children($element[$first]) as $second) {
       // Assign the element by reference, so any potential changes to the
       // original element are taken over.
-      $row['data'][] = array('data' => &$element[$first][$second]);
+      $column = array('data' => &$element[$first][$second]);
+
+      // Apply wrapper attributes of second-level elements as table cell
+      // attributes.
+      if (isset($element[$first][$second]['#wrapper_attributes'])) {
+        $column += $element[$first][$second]['#wrapper_attributes'];
+      }
+
+      $row['data'][] = $column;
     }
     $element['#rows'][] = $row;
   }

In #94 I also messed up the indentation in one line:

--- a/includes/form.inc
+++ b/includes/form.inc
@@ -3555,7 +3555,7 @@ function form_validate_table($element, &$form_state) {
       form_error($element, t('No items selected.'));
     }
   }
-elseif (!isset($element['#value']) || $element['#value'] === '') {
+  elseif (!isset($element['#value']) || $element['#value'] === '') {
     form_error($element, t('No item selected.'));
   }
 }

This time I'm really certain that the code is identical to the D8 code. Sorry for all the noise.

+1 from me, would like to see this in D7.

P.S.:

+++ b/includes/form.inc
@@ -3409,6 +3439,128 @@ function form_process_tableselect($element) {
+    // @todo D8: Rename into #select_all?

I don't see the use for this, seeing as it's already in D8 :)

Updated issue summary.

https://drupal.org/project/table_element, in case anyone is interested.

We've had a heck of a time with the conversions to #type=>table and ran into some issues that are prompting the dreaded question would it be easier to revert or find solutions to these. #2152193: Discuss the fate of #type table There has been quite a bit of work on the meta and maybe some of the people involved in the addition of this new type could weigh in on the problems?

Also, I don't mean to detract from the efforts nor hurt any feelings here. I've worked quite hard myself on a number of conversions and my hours would go out the window on a revert too, so this is a plea for help more then anything, though the way it looks now cutting our losses may unblock a bunch of issues.

I'm not sure whether I agree with backporting this... looks like a typical thing for https://drupal.org/project/elements to me?

agree with #100 that the backport to D7 is not required. This appears to already have been included in the linked module. Therefore, although it's not as cool as a core solution, contrib can reference that contrib module.