Convert theme_user_admin_roles

Sorry, I have changed the status of the task by a mistake. Changed back

+++ b/core/modules/user/user.admin.inc
@@ -942,6 +944,44 @@ function theme_user_admin_roles($variables) {
+    $rows[] = array('data' => $row, 'attributes' => new Attribute(array('class' => array('draggable'))));

I believe you should change it to $rows[] = array('data' => $row, 'attributes' => array('class' => array('draggable')));

Cell attributes should be handled by template_preprocess_table(&$variables), core/includes/theme.inc

Looks good for me!

thanks!

commited / pushed to front-end

It looks like the template is missing from front-end, so re-applying patch.

Also, some notes:

As a general rule, we are not calling theme() functions from the preprocess layer. Instead we include the template file table directly from this new template. This will make it much easier for theme devs to find where the code they want to change is coming from.

The theme_table function (or template) needs the variables header, rows, and attributes, so we expose those in preprocess here so they can be passed straight through to the table.html.twig file. Then we also need to call template_preprocess_table() from our preprocess function, to make sure that the table variables gets formatted correctly before being passed to that template.

Also, attributes need to use the new Attribute class.

I'm going to make these changes, test, and commit this file.

this is what the template now looks like:

{#
/**
 * @file
 * Default theme implementation for the role order and new role form.
 *
 * Available variables
 * - header: The table headers. Each element of the array can be accessed.
 * - rows: The table rows. 
 * - attributes: An array of html attributes to apply to the table tag.
 * - form: Remaining form elements for user roles table.
 *
 * @see template_preprocess()
 * @see template_preprocess_user_admin_roles()
 *
 * @ingroup themeable
 */
 @todo: update this once merged with head, and table.twig.html updated.
 @todo: update paths to include files once http://drupal.org/node/1777532 is resolved.
#}
{% include "core/themes/stark/templates/theme.inc/table.html.twig" %}
{{ form }}

feel free to check the changes in preprocess too, if you are curious.
Tested and pushed. marking as fixed.

As a general rule, we are not calling theme() functions from the preprocess layer. Instead we include the template file table directly from this new template. This will make it much easier for theme devs to find where the code they want to change is coming from.

I understand the reasoning here, but it seems to me this approach doesn't work from a technical point of view.

The theme_table function (or template) needs the variables header, rows, and attributes, so we expose those in preprocess here so they can be passed straight through to the table.html.twig file. Then we also need to call template_preprocess_table() from our preprocess function, to make sure that the table variables gets formatted correctly before being passed to that template.

What if my module implements:

function MYMODULE_preprocess_table(&$variables) {
} 

Then my preprocess function will get bypassed with the approach above. Calling template_preprocess_table() directly is kind of a hack and is only meant to be called by the theming internals. Also, I want to point out that behavior will be different with this approach vs. using theme().

Another situation where this "general rule" does not work is when you want to incorporate not one, but two or more arbitrary templates in your template, e.g.:

$variables['table1'] = theme('table', ...);
$variables['table2'] = theme('table', ...);

How do you handle that with the approach above?

I propose that this rule gets revised and/or another solution for the actual implementation of it is sought.

Updated issue summary.