Convert theme_views_ui_rearrange_filter_form() to Twig

Split from meta.

whoops, removed a '+'

Review for #4:

+ * Default theme implementation for views ui rearrange filter form.

Views UI

+ * - grouping: A boolean for if there is more than one group.

We're not including PHP data types in Twig template docs.

-      $row[] = array('colspan' => 5, 'data' => $instructions . drupal_render($form['remove_groups'][$group_id]));
+      $row[] = array('colspan' => 5, 'data' => $instructions . $form['remove_groups'][$group_id]);

Does this really work? $instructions would be a string and presumably $form['remove_groups'][$group_id] is a renderable array, right?

+ '#theme' => 'link',

We're not supposed to be doing this unless #1922454: Gut l(), fix theme('link'), have a working framework for consistently generated links inside and outside of Twig lands. Don't know if we need to change anything, but it's worth noting that this patch is essentially waiting on that.

+{% endif %}
+<div class="scroll">
+  {% if grouping %}

Shouldn't the div be indented two spaces?

This issue needs manual steps for testing to be added to the issue summary and for somebody to follow the steps and verify the markup manually.

Yeah that $instructions concatenation was a blunder thanks @thedavidmeister

Left theme link conversion because it's only really active links that are affected there. Let me know if my assumption is wrong?

hmmm, how did that get in there, fixed array line length as well.

#9: 1963982-9-twig-views-ui-rearrange-filter-form.patch queued for re-testing.

#7 -

Left theme link conversion because it's only really active links that are affected there. Let me know if my assumption is wrong?

Currently theme_link() skips the active class handling and also this sanitization:

  // Remove all HTML and PHP tags from a tooltip. For best performance, we act only
  // if a quick strpos() pre-check gave a suspicion (because strip_tags() is expensive).
  if (isset($options['attributes']['title']) && strpos($options['attributes']['title'], '<') !== FALSE) {
    $options['attributes']['title'] = strip_tags($options['attributes']['title']);
  }

+++ b/core/modules/views/views_ui/views_ui.theme.incundefined
@@ -276,22 +294,62 @@ function theme_views_ui_rearrange_filter_form(&$vars) {
+          '#theme' => 'link',

Normally in render arrays we use '#type' => 'link'

Otherwise, this looks great!

#13 - '#type' => 'link' is for form elements I thought.

No, it's a render element. All render elements are also form elements, but they can be used on their own. #type => container, for example.

Unpostpoing as there has been a while waiting, let's do this conversion for now.

@lauriii have a look at #1963980: Convert theme_views_ui_expose_filter_form() to Twig Would you mind doing the same here? Removing the preprocess entirely if possible, and attempt at filling out the @todos?

Yeah, I'll take look on that.

Thanks @lauriii here's the changes since #20

Mind doing an manual testing?

This fixes at least the remove button to show up because #theme link doesn't exist but #type link does.

For some reason the the action buttons have an extra "Create new filter group". I'm not really sure why at the moment...

This patch fixes all of the Twig autoescaping issues with this theme. See screenshot.

@laurii Want to take a stab at getting that action link to do what it did before (I assume hide or JS hide...)
And maybe a markup comparison?

The patch applied here solves issue described on #24.

I did markup comparison and found few differences:
1) Default group filter is visible all the time because its missing .tabledrag-hide. Patch applied here should fix this.

Before:

After:

2) .views-messages element is visible even if there's no messages.

Create new filter group functionality doesn't work for some reason, so I couldn't test what happens when there's multiple groups.

@joelpittet found some more changes on the markup so here's a fix for them:
1) Empty markup have changed so I rolled it back for how it used to be.
2) Elements on the remove col wasn't arranged correctly.

Ok so we fixed the escaping issues with this function, which is a plus.

I manually fixed the before patch markup > < to accurately to a comparison.
@lauriii fixed the minor ordering issue with a #weight and and the empty markup was moved back to where it was because #empty didn't seem to work on the #type table so no need to make that change in this conversion issue.

I've just added a couple minor changes still, removed Javascript:void() from the link to match and got #instructions to show up in the markup with #markup.

Markup Review Screenshots:

Committed and pushed to 8.x. Thanks!