Add space before columns’ extra classes, in the layout templates [#2543172]

I was trying to hack some extra (bootstrap like) classes into columns for DS layouts.... and it appears you can do that in an alter hook, without having to copy the templates just to add your own classes. (example.)

However... that only works if you make sure you start the theme-variable for the classes with a space. That smells like a bug, not a feature. (See also the first line of all templates; the $classes variable for the whole layout does not have this issue.)

Apparently noone reported this before, so apparently this is not considered a big issue. But... this may be a good time to also fix it in D8, where the issue is still present, looking at the templates. (I will be happy to roll a patch for D8 - but less happy to have to test it atm.)

Comment	File	Size	Author
#3	ds-layout-template-column-classes-D8-2543172-3.patch	10.61 KB	roderik
#2	ds-layout-template-column-classes-D8.patch	10.43 KB	roderik
	ds-layout-template-column-classes.patch	12.36 KB	roderik

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

aspilicious CreditAttribution: aspilicious commented 31 July 2015 at 11:15

interesting... I'll look at it, please also make a D8 version.

Comment #2

roderik

Dutch

Amsterdam,NL / Budapest,HU

CreditAttribution: roderik as a volunteer commented 31 July 2015 at 15:19

Version:

7.x-2.x-dev

» 8.x-2.x-dev

File	Size
ds-layout-template-column-classes-D8.patch	10.43 KB

For completeness sake: I didn't install/run D8, only edited templates on visual inspection.

Comment #3

roderik

Dutch

Amsterdam,NL / Budapest,HU

CreditAttribution: roderik as a volunteer commented 31 July 2015 at 15:31

File	Size
ds-layout-template-column-classes-D8-2543172-3.patch	10.61 KB

1 file was hidden/shown/deleted

File	Size
ds-layout-template-column-classes-D8.patch	10.43 KB

Forgot one.

Comment #4

aspilicious CreditAttribution: aspilicious commented 1 August 2015 at 15:51

Well, actually it's strange you have issues because I found this in the D8 code.

$variables[$region_name . '_classes'] = !empty($configuration['layout']['settings']['classes'][$region_name]) ? ' ' . implode(' ', $configuration['layout']['settings']['classes'][$region_name]) : '';

It's the same for D7.

So there shouldn't be any problem. I verified it myself. But I agree this should be changed. We shouldn't add the space in the preprocess function for D8, not for D7.

EDIT: AAAH you're overwriting the preprocess function, ok makes sense now.

Comment #5

roderik

Dutch

Amsterdam,NL / Budapest,HU

CreditAttribution: roderik as a volunteer commented 2 August 2015 at 23:20

I'm not sure if I'm supposed to still answer anything here, but I checked the code you quoted in #4, in D7 - which is in ds_entity_variables().

The issue with my code seems to be

1)
I'm adding the classes to $vars in a hook_ds_pre_render(), which is executed after the above code snippet .

2)
I'm using hook_ds_pre_render() only because.... I failed at using a MYTHEME_preprocess() function.

By looking at the ds_theme() code / template name, I first figured I should add those extra classes in MYTHEME_preprocess_ds_3col_equal_width(). But that function is not executed. By step debugging, I discovered that theme('entity') is choosing the DS layout template for rendering... but does not execute any hook_preprocess_LAYOUT_NAME(). (It only executes preprocess functions primarily linked to 'entity'.)

I was kinda stuck there. If I should be using another preprocess function instead, I don't know it / could not find any info. So I figured hook_ds_pre_render() was the right place to modify $left_classes et al.