Add element #type table (with tableselect and tabledrag support)

Neato.

Note that tabledrag is a bit tricky though - because on failed validation, you want to redisplay the table in the updated order. Field UI is dealing with for its own tables in field_ui_table_pre_render().

Yup, field_ui_table_pre_render() builds the tree from parenting and weight values ($row['parent_wrapper']['parent']['#value'], $row['weight']['#value']), and deduces rendering order. This relies on the the hardcoded presence of the 'parent' and 'weight' elements, though.

Handling of region rows is more complex still (relies on PHP callbacks per 'row type' + JS code...), and is probably best left aside.

Just a plug to note that currently http://drupal.org/project/table exists. There's also http://drupal.org/project/elements. Since Table hasn't seen commits since last March, maybe at least contact @kkaefer to mark it obsolete. On the other hand, development could also move there, and Edge would simply introduce a dependency on Table. Either way, efforts should be consolidated.

Also, now looking at the code, I saw that 'colspan' is currently not supported for a cell. I'll see if I can roll a patch, and open a new issue, if so.

Actually by 'the code' I meant HEAD, I just mentioned it here, because this is the AFAIK the (one-and-)only issue for #type table.

If we're adding new FAPI elements, might you consider adding them to http://drupal.org/project/elements?

Here's my Dreditor review. I have yet to try out the code. Will do that, though, in the next couple days.

+++ edge.module	6 Jan 2011 20:38:48 -0000
@@ -13,8 +13,20 @@ function edge_element_info() {
+    '#value_callback' => 'edge_type_table_value',

Not your fault, but:

do {
 $me = ":'(";
} while ('This is nowhere to be found on http://api.drupal.org')

+++ edge.module	6 Jan 2011 20:38:48 -0000
@@ -85,6 +97,139 @@ function edge_css_alter(&$css) {
+ *   The data that will appear in the $element_state['values'] collection

You mean $form_state['values'] right?

+++ edge.module	6 Jan 2011 20:38:48 -0000
@@ -85,6 +97,139 @@ function edge_css_alter(&$css) {
+    // @todo Why?

You can read my mind... :) (The comment below that is explaining that the keys of #default_value are used, not the values).

+++ edge.module	6 Jan 2011 20:38:48 -0000
@@ -85,6 +97,139 @@ function edge_css_alter(&$css) {
+    // Advanced selection behaviour make no sense for radios.

Typo: make -> makes

+++ edge.module	6 Jan 2011 20:38:48 -0000
@@ -85,6 +97,139 @@ function edge_css_alter(&$css) {
+    // Add a "Select all" checkbox column to the header.
+    if ($element['#js_select']) {

Is this naming consistent with somewhere in core? If not, maybe rename #js_select with #select_all?

+++ edge.module	6 Jan 2011 20:38:48 -0000
@@ -85,6 +97,139 @@ function edge_css_alter(&$css) {
+    // Add an empty header column for radio buttons or a "Select all" checkbox
+    // is not desired.

The sentence doesn't make sense. or -> when?

+++ edge.module	6 Jan 2011 20:38:48 -0000
@@ -85,6 +97,139 @@ function edge_css_alter(&$css) {
+        // Determine option label; either an assumed 'title' column, or the
+        // first available column containing a #title or #markup.

Just popped into my mind, might make sense, might not: What about a #title_key attribute, which defaults to 'title'. That would be more (/as ?) flexible and remove the need for the code below (finding an element with #title set).

+++ edge.module	6 Jan 2011 20:38:48 -0000
@@ -85,6 +97,139 @@ function edge_css_alter(&$css) {
+          '#attributes' => $element['#attributes'],

Is there any specific reason we're applying the top-level attributes to each checkbox/radio? There probably is, but I don't see it.

+++ edge.module	6 Jan 2011 20:38:48 -0000
@@ -85,6 +97,139 @@ function edge_css_alter(&$css) {
+        if ($element['#multiple']) {
+          $element[$key]['select']['#default_value'] = isset($value[$key]) ? $key : NULL;
+          $element[$key]['select']['#parents'] = $element_parents;
+        }
+        else {
+          $element[$key]['select']['#default_value'] = ($element['#default_value'] == $key ? $key : NULL);
+          $element[$key]['select']['#parents'] = $element['#parents'];
+          $element[$key]['select']['#id'] = drupal_html_id('edit-' . implode('-', $element_parents));
+        }

It seems as though checkboxes automatically get an HTML ID, but radios don't. If that's so, isn't that a core bug?

Powered by Dreditor.

Now I've tried it out. It's pretty awesome, because it's literally a one-line change in your code and you get a whole lot for it.

Two things:
1. The #element_validate callback checks that you have select at least one row. Why is that? I don't see a reason for that. Maybe if #required => TRUE or something...

2. If you don't key your table-row-elements with proper string-names, but just use an indexed array, the first value will be 0 (integer). So that means if the row is checked the value will be '0' (string) and if unchecked the value will be 0 (integer) (like all unchecked rows), which both cast as false to boolean. I don't know if we should do anything about that, but I wanted to mention it, because it will probably confuse people down the road.

For reference, the last thing mentioned in #15 is: #453400: "-wrapper" HTML ID not output for #type checkboxes/radios
(Might be good to add a @todo or something in the code?)

Ehh, actually that's a Drupal 6 issue. #17 is bogus.

I have started working on two custom forms (in D6 for now) that I hope to contribute back to the Drupal community. The first allows selective pruning of the dblog table on possible combinations of time and count values. The second does the same for the aggregator tables.

@sun your code here and that of @yched in the field_ui module are great examples of integrating sortable forms into themeable tables with possible js support. Using these two examples (and possibly others), does it make sense to make an effort to add a new #table element to the FAPI for Drupal 8? If so, I am interested in contributing to the effort as this is an element I believe many contributor modules could well use.

FWIW : considerations about 'empty cells' in #1248940-3: "Manage fields" screens : broken table layout