Convert theme_views_ui_expose_filter_form() to Twig

Split from meta.

Don't know why the tag got removed... adding it back in and removing the whitespace.

This looks good to me from a code style perspective. Ready for somebody to start manually testing the markup for :) don't forget to write out the steps for testing in the issue summary.

The only thing is that there's an awful lot of undocumented variables with @todo in this patch - are we going to open a followup ticket for these?

Can't make it work in reality - the template is not getting called. 'views_ui_expose_filter_form' - is this the correct name for it? My DPM gives me the name 'views_ui_config_item_form' on the exposed filter config page.

@alexrayu not sure what you mean? If you go to views for front page, and open up the filter dialog and click expose.
/admin/structure/views/view/frontpage/edit
Content: Promoted to front page status

I found some random things in the form that I don't know why they got there and added some todo cleanup.

+++ b/core/modules/views/views_ui/views_ui.theme.incundefined
@@ -75,43 +75,58 @@ function theme_views_ui_view_info($variables) {
+  $variables['form_description'] = $form['form_description'];
+  $variables['expose_button'] = $form['expose_button'];
+  $variables['group_button'] = $form['group_button'];
+  unset($form['form_description']);
+  unset($form['expose_button']);
+  unset($form['group_button']);

See my comment in #1963978-12: Convert theme_views_ui_build_group_filter_form() to Twig, I think we should do a similar thing here.

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

Here's a new patch now that we have without() filter we don't need the preprocess.

I gave it a manual test locally and it seems to work out nicely. Could use a second pair of eyes.

Patch looks good, also manually tested successfully.

There is some code in FilterPluginBase that seems like it should belong in the template, but I guess this is a separate issue:

       // Increase the width of the left (operator) column.
      $form['operator']['#prefix'] = '<div class="views-group-box views-left-40">';
      $form['operator']['#suffix'] = '</div>';
      $form['value']['#prefix'] = '<div class="views-group-box views-right-60">';
      $form['value']['#suffix'] = '</div>';

+++ b/core/modules/views_ui/templates/views-ui-expose-filter-form.html.twig
@@ -0,0 +1,68 @@
+ * - group_button: @todo.
...
+ * - operator: @todo.
+ * - value: @todo.
+ * - use_operator: @todo.
+ * - has_left_column: @todo.
...
+ * - more: @todo.

Shouldn't we do?

+++ b/core/modules/views_ui/templates/views-ui-expose-filter-form.html.twig
@@ -0,0 +1,68 @@
+{% if form.use_operator is not empty %}
...
+{% if form.operator['#type'] %}

Shouldn't these if's be the same? Looks very weird to me. That said this weirdness is in the original .tpl.php

@longwave Want to give the other postponed one a go? It's very similar to this one. Likely easier to start from scratch mostly as I did here. #1963982: Convert theme_views_ui_rearrange_filter_form() to Twig

Sorry for leaving all the @todos in there.

@alexpott re #13.2 I tried to match the originals. is not empty allows for 0's where as the second one just checks for a truthy value.

The check for {{ form.required }} isn't needed because it doesn't have a wrapper div to concern itself with.

{{ form.use_operator }} could be a 0. Though maybe unlikely I wasn't confident in that decision. If someone else can confirm that 0's can be safely disregarded (aka !empty() vs isset()) Then I'd be glad to change it. #type was !empty() which is simply {% if var %}

Tried to fill out the @todos to the best of my ability (non-existent docs before).

Sorry that interdiff was useless, here it is again.

I wonder if we should just remove this template. The form structure is complicated and very variable depending on the filter in use and the various options that it may have, and I can't see why anyone would need to override this form in their theme; also it seems like it depends on a lot of #prefix and #suffix attributes that are simply hardcoded. The existing template logic can be moved to FilterPluginBase.

Also, the output already appears to be subtly broken, before this patch - edit the default Content view, click the "Published status" filter, and change "Filter type to expose" to Grouped. Perhaps this is due to the SafeMarkup feature, though.

I couldn't see any difference before and after this patch. Likely don't need this template file, and that was the original intent of postponing, but I just want to get rid of the theme function. Removing this template will just further break the layout even more than it already is.

I'm fine with it being here and if someone wants to abstract it out entirely that's fine too, but the goal is just to get it converted so it's easier to maintain a a template instead of HTML in PHP strings.

The amount of drupal_renders() removed alone is a win to me.

In that case I think this is fine. I am pretty sure it is safe to check form.use_operator is not empty in both the if statements as use_operator will either be a checkbox element or unset, never 0. However if we are going to stay with the 1:1 conversion and clean this up later, then the documentation needs a minor fix:

+ * - operator: The operators for how the filters value should be treated.
+ * - value: The filters available values.
+ * - use_operator: Checkbox to allow the user to expose the operator.
+ *   - #type: The operator type.

#type belongs to operator, not use_operator.

I think this is ready for RTBC if we fix the minor documentation bug in #20.

@longwave thanks for the poke I forgot to make the quick fix.

Actually I agree with it being ok to remove the is not empty thanks, for got that in #22

ok let's clean this up later...

Committed 14b58a4 and pushed to 2317865. Thanks!

Committed 14b58a4 and pushed to 2317865. Thanks!

Huh? Not committed to 8.0.x anyway, that I can see.

Committed 2ff3381 and pushed to 8.0.x. Thanks!