Move classes out of the preprocess functions and into the Twig templates. Use the addClass() attribute method to add classes in the template. Use the clean_class filter to filter class names, if necessary. Maintain all existing functionality and ensure all existing class names are still in the markup, even ones that are inherited.

See the following issues for more detailed examples:
#2217731: Move field classes out of preprocess and into templates
#2254153: Move node classes out of preprocess and into templates

See this change record for information about using the addClass() method:
https://www.drupal.org/node/2315471

See this change record for more information about the phase 1 process of moving class from preprocess to templates:
https://www.drupal.org/node/2325067

Preprocess Functions Modified

template_preprocess_views_ui_display_tab_setting
template_preprocess_views_ui_display_tab_bucket
template_preprocess_views_ui_view_info
template_preprocess_views_ui_view_preview_section

Twig Templates Modified

views-ui-display-tab-setting.html.twig
views-ui-display-tab-bucket.html.twig
views-ui-view-info.html.twig
views-ui-view-preview-section.html.twig

Files: 
CommentFileSizeAuthor
#28 2329919-28.patch4.07 KBlauriii
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,533 pass(es). View
#28 interdiff-2329919-23-28.txt1.5 KBlauriii

Comments

davidhernandez’s picture

Issue tags: +FUDK
lanchez’s picture

Assigned: Unassigned » lanchez
lanchez’s picture

Status: Active » Needs review
FileSize
3.45 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,075 pass(es). View

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.

mdrummond’s picture

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

Cottser’s picture

Status: Needs review » Reviewed & tested by the community

Makes sense, looks good. Thanks @lanchez!

davidhernandez’s picture

Issue summary: View changes

Just confirming the others are erroneous and striking them out of the summary.

Cottser’s picture

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

dawehner’s picture

+1

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
  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?

Cottser’s picture

@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.

Cottser’s picture

Status: Needs work » Needs review
FileSize
2.88 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Setup environment: Test cancelled by admin prior to completion. View
1.75 KB

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

mdrummond’s picture

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

Cottser’s picture

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

dawehner’s picture

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.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Looks nice!

lauriii’s picture

Assigned: lanchez » Unassigned

Unassigning :)

Cottser’s picture

@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.

webchick’s picture

Assigned: Unassigned » alexpott

Shooting back over to Alex.

mdrummond’s picture

Status: Reviewed & tested by the community » Needs work

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.

lauriii’s picture

Status: Needs work » Needs review
FileSize
1.06 KB
3.13 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,845 pass(es), 0 fail(s), and 1,987 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 20: 2329919-20.patch, failed testing.

mdrummond’s picture

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?

lauriii’s picture

Status: Needs work » Needs review
FileSize
3.2 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,985 pass(es). View
731 bytes

The last submitted patch, 11: 2329919-11.patch, failed testing.

lauriii’s picture

Assigned: alexpott » Unassigned
rteijeiro’s picture

Status: Needs review » Reviewed & tested by the community

Looks good!

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ 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.

lauriii’s picture

Status: Needs work » Needs review
FileSize
1.5 KB
4.07 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,533 pass(es). View
davidhernandez’s picture

What is the difference between the title and name of the bucket? The documentation uses the same examples.

lauriii’s picture

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

LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community

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

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 4722170 and pushed to 8.0.x. Thanks!

  • alexpott committed 4722170 on 8.0.x
    Issue #2329919 by lauriii, Cottser, lanchez | davidhernandez: Move...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.