Convert theme_tableselect() to #theme table__tableselect

Converted the theme_tableselect to table__tableselect.

Patch attached

Awesome @JeroenT could you add a couple URLs to review this change to the issue summary's Steps to test? Maybe a couple paths that use table select?

I'll do a manual review for you;)

Added steps to the Issue Summary. Did a quick test and The responsive JS and table class were missing from the comments admin table select.

"core\/misc\/tableresponsive.js"
<table class="responsive-enabled">

May need
'#responsive' => TRUE, and possibly some other type=>table default properties.

@JoelPittet,

Thank you for the review. I added the responsive + added some missing classes to the table.

PSR-4 fix

This looks good, just tried it out again and reviewed. I'm just posting those changes in this patch. Let me know if you have any concerns with the changes?

1. +++ b/core/includes/form.inc
   @@ -1632,14 +1634,8 @@ function form_process_container($element, &$form_state) {
   +function form_pre_render_tableselect(array $element) {
   

   No other pre_renders type hint the $element so I'd rather not in this case unless you want to open an issue to do them all at once?

2. +++ b/core/modules/comment/src/Form/CommentAdminOverview.php
   @@ -202,15 +202,22 @@ public function buildForm(array $form, array &$form_state, $type = 'new') {
   +          'data' => drupal_render($username),
   

   You don't need to call drupal_render here. Just data => $username.

   Save the flattening.

thanks @steinmb and @JeroenT

Here's a manual testing. It's looking good, this patch fixes a responsive bug in rendering the comments form. See screenshots.

@joelpittet,

The responsive fixes come from RESPONSIVE_PRIORITY_MEDIUM and RESPONSIVE_PRIORITY_LOW:

	@@ -202,15 +202,22 @@ public function buildForm(array $form, array &$form_state, $type = 'new') {
             '#title' => $comment->getSubject(),
           ) + $comment_permalink->toRenderArray(),
         ),
-        'author' => drupal_render($username),
+        'author' => array(
+          'data' => $username,
+          'class' => array(RESPONSIVE_PRIORITY_MEDIUM),
+        ),
         'posted_in' => array(
           'data' => array(
             '#type' => 'link',
             '#title' => $commented_entity->label(),
             '#access' => $commented_entity->access('view'),
           ) + $commented_entity->urlInfo()->toRenderArray(),
+          'class' => array(RESPONSIVE_PRIORITY_LOW),
+        ),
+        'changed' => array(
+          'data' => $this->date->format($comment->getChangedTime(), 'short'),
+          'class' => array(RESPONSIVE_PRIORITY_LOW),
         ),
-        'changed' => $this->date->format($comment->getChangedTime(), 'short'),
       );
       $comment_uri_options = $comment->urlInfo()->getOptions();
       $links = array();

Thanks, yeah this is ready RTBC in my opinion, just checking if Cottser agrees to remove the profiling task as it's usually just a Admin UI item and we aren't using it in many places nor are we converting to twig.

I made changes to the comment of the function form_pre_render_tableselect.

Patch attached.

Wrong interdiff, this is the right one.

I outdented everything underneath @param.

Patch attached.

@Cottser, I hope this is the last update to the comments ;)

I also changed the comment of form_process_tableselect.

Patch attached.

18: 2152227-tableselect-suggestion-17.patch queued for re-testing.

Ok, I think I get it now.

I also removed the changes to the comment of form_process_tableselect. I thought i could also change this comment because it's also about the tableselect.

Patch attached.

Marking as needs review.

Did a manual test, and it works great. Only diff was it now has an ID and before it didn't. This is due to the addtion of drupal_pre_render_table I believe.

Re-rolled, something must have just changed slightly as I just reviewed it.

Committed 28ef6b2 and pushed to 8.0.x. Thanks!

Oops - we need a change record for this. If this isn't done in a couple of days I'll have to make this a beta blocker :(

Wanted to do this change record, and looked at the parent for possibly existing ones or using one as a template. Found that #2152205: Convert theme_date() to #theme input__date didn't have one. Maybe also needs one?

Drafted a change record, not sure it's really nessasary but maybe it's helpful?

In D7 we still had the #type=>tableselect, this just gets rid of the extra theme function and replaces it with a table template suggestion.
https://www.drupal.org/node/2322315

Actually all I think we need to do is add this issue to this CR https://www.drupal.org/node/1876710 since what we people need to know to do is this:

+++ b/core/modules/update/src/Form/UpdateManagerUpdate.php
@@ -249,12 +250,13 @@ public function buildForm(array $form, array &$form_state) {
-        '#type' => 'tableselect',
+        '#type' => 'table',
+        '#tableselect' => TRUE,

Sorry for the distraction.

I think https://www.drupal.org/node/2322315 does not contain the necessary detail if someone was actually using theme_tableselect().