Move views_ui classes from preprocess to templates

Here's a patch. TBH only template_preprocess_views_ui_display_tab_setting and template_preprocess_views_ui_display_tab_bucket needed this kind of love.

This is looking good to me. See what testbot says.

Makes sense, looks good. Thanks @lanchez!

Manually tested at /admin/structure/views/view/content also, looks great. Absolutely no changes.

+1

1. +++ b/core/modules/views_ui/templates/views-ui-display-tab-bucket.html.twig
   @@ -5,16 +5,25 @@
   + * - name: Element name.
   ...
   + * - overridden: If the "box" has been overridden.
   ...
   +    name ? name|clean_class,
   +    overridden ? 'overridden',
   

   Why not just use element.name and element.overridden instead of added new variables?

2. +++ b/core/modules/views_ui/templates/views-ui-display-tab-setting.html.twig
   @@ -15,7 +15,16 @@
   +    overridden ? 'overridden',
   
   +++ b/core/modules/views_ui/views_ui.theme.inc
   @@ -64,13 +53,10 @@ function template_preprocess_views_ui_display_tab_setting(&$variables) {
   -    $variables['attributes']['class'][] = 'overridden';
   +    $variables['overridden'] = $element['#overridden'];
   

   element.overridden instead?

@alexpott the short answer is because we haven't solved #2160611: Provide {{ item.item.alt }} Twig syntax for getting data from $item['#item']['alt']. But we could use the square bracket syntax here I suppose.

Here's what that would look like. What do we think?

Do we use the square bracket syntax elsewhere? It feels somewhat odd.

I agree, I'm not a fan. I don't think we use it anywhere else in core at this time.

I agree, I'm not a fan. I don't think we use it anywhere else in core at this time.

Well, I know that several people now uses short array syntax for every new line of code, but yeah we agreed that there should be no requirement
to use it.

Looks nice!

Unassigning :)

@dawehner I don't mean the short array syntax in PHP, I'm totally fine with that. It's the var['key'] syntax in Twig that I'm not a fan of but we need to use that to access hashed keys.

Shooting back over to Alex.

In #2229435: Clean up the way attributes are printed in field.html.twig, there was a decision to avoid using the bracket octothorpe syntax as a template variable. I think we need to clarify if we want to make the same call on this issue before it gets in. Adding an extra variable just to avoid syntax weirdness may feel like overkill, yes, but it also makes it easier to explain how to work with variables in templates to new folk.

Before, we were checking if the keys #name and #overridden existed before adding the classes. My guess is we need to do something similar in preprocess: default the variables to be empty, and content if the element keys exist?

Looks good!

+++ b/core/modules/views_ui/templates/views-ui-display-tab-bucket.html.twig
@@ -14,7 +14,14 @@
+    name ? name|clean_class,
+    overridden ? 'overridden',

+++ b/core/modules/views_ui/templates/views-ui-display-tab-setting.html.twig
@@ -15,7 +15,16 @@
+    defaulted ? 'defaulted',
+    overridden ? 'overridden',

These variables are not documented.

Only difference I found between title and name is that title is optional and name is mandatory

It seems like the issues raised by Alex have been addressed. A quick manual test does not show any visual regressions. Good work all!

Committed 4722170 and pushed to 8.0.x. Thanks!