Hello, not sure if this is an Omega Issue or a DS issue but it's an issue that affects Omega none the less.

When using any DS layout I can't seem to get the classes to appear on the nodes ("node node-teaser" etc) that would usually appear there. I notice that with Omega 4 these classes are different than with other themes ("node node--page node--full node--page--full" is how they show up in Omega 4). This seems to be done in omega's hook_preprocess_node() located in node.preprocess.inc in the Omega base theme. I have tried other themes and the correct classes show up fine. I'm sure it's probably something simple but I haven't been able to figure out a tidy way of getting these to show up with Omega, does anyone have any ideas?

Thanks!

CommentFileSizeAuthor
#8 2059819-8.patch1.08 KBfubhy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fubhy’s picture

Yep, this is a known issue but I did not have time to look into it yet. It's not related to the node-specific preprocess hooks but is a problem with the omega_initialize_attributes() and omega_cleanup_attributes() (omega) and ds_entity_variables() (display suite). They both basically do the opposite things and DS is being very mean here as it afterwards unsets() our classes which causes all classes to disappear :(. I would say this is something we should fix in DS as blindly unsetting the class attribute like that is just mean. Also, I am 100% pro-attributes-array as I absolutely hate "classes_array". I don't even understand why we have that. Attributes should be used everywhere (and that's what we are doing in D8 too) so my claim would be that "Omega does this right" at this point and we should probably fix this in DS. :)

Not sure how we would fix that in Omega either... The whole switch to attributes_array is quite tricky (with that reference to classes_array to keep track of it, etc.).

freddybushboy’s picture

Ah I see - very interesting! I have posted pretty much the same issue in the DS issue queue so I'm curious to see what people had to say over there too. I'll have to look into a temporary work around in the mean time though - I'll post anything I find here :-) Thanks!

swentel’s picture

@fubhy - I remember you showing me this one day, let's talk this over in IRC when we both have a few minutes :)

fubhy’s picture

@swentel sure, will be on IRC in about an hour...

freddybushboy’s picture

Looking forward to hearing what comes of that chat. The temporary fix I am using at the moment for anyone who is interested is just to add the node classes to $variables['classes'] in template_process_node(&$variables) in my template.php. Not great but does the trick for now..

Here is the code I used if anyone is interested - I copied a lot of it straight from omega:

function THEMENAME_process_node(&$variables) {
  // Temporarily fixes a known bug with DS and Omega.
  $css_node_type = drupal_clean_css_identifier($vars['type']);
  $css_view_mode = drupal_clean_css_identifier($vars['view_mode']);
  // Add modifier classes for view mode.
  $variables['classes'] .= ' node';
  $variables['classes'] .= ' node--' . $css_view_mode;
  $variables['classes'] .= ' node--' . $css_node_type;
  $variables['classes'] .= ' node--' . $css_node_type . '--' . $css_view_mode;
}
fubhy’s picture

Please use $variables['attributes_array']['class'] instead of classes and classes_array.

freddybushboy’s picture

I tried adding these classes using first the attributes_array and then the classes_array - for example $variables['attributes_array']['class'][] = 'node';

Neither of these worked for me which is why I resorted to just adding it to $variabless['classes']

fubhy’s picture

Assigned: Unassigned » msmithcti
Status: Active » Needs review
FileSize
1.08 KB

This simple patch might solve the problem. Can you please test it? I did not have time to set up DS, etc. as I am leaving for a week of vacation :). If it works (and there are no other bugs introduced by this) please let @splatio commit it. Assigning to @splatio.

Status: Needs review » Needs work

The last submitted patch, 2059819-8.patch, failed testing.

fubhy’s picture

I had some time to test this now. The patch seems to work nicely. I did not find any regressions whatsoever and the DS layout classes are rendered properly. Going to commit this. Can you please still test this in case I missed anything?

fubhy’s picture

Assigned: msmithcti » Unassigned
Status: Needs work » Fixed

Committed 4555f2f

freddybushboy’s picture

Thanks, I changed to the dev version, and it resolves my problem just fine :-) I am however getting this unrelated error with the dev version. I notice the libraries folder doesn't come with the dev version but I still had most of the files from beta6 - I just needed html5shiv-printshiv.js to resolve the second part and I am no longer using livereaload.js so I dont actually get the first part anymore - just a heads up.

Warning: parse_url(:/livereload.js) [function.parse-url]: Unable to parse URL in omega_extension_development_preprocess_html() (line 36 of /Users/fredparke/sites/sandbox/sites/all/themes/omega/includes/development/development.inc).
Warning: file_get_contents(sites/all/themes/sandbox/libraries/html5shiv/html5shiv-printshiv.js) [function.file-get-contents]: failed to open stream: No such file or directory in _locale_parse_js_file() (line 1488 of /Users/fredparke/sites/sandbox/includes/locale.inc).
fubhy’s picture

Take a look at the libraries.make file that now comes with the Dev release.use that to download the libraries properly. There is already an issue for the parse URL bug with livereload with a patch that fixes it.

freddybushboy’s picture

Thanks :)

freddybushboy’s picture

These are the classes I get now when using a DS layout for the record:

ds-1col node node-product node-teaser view-mode-teaser node--teaser node----teaser clearfix

Quite a lot of classes are added for the view mode and the node----teaser one is a bit odd, I'm guessing its the one that is supposed to be something like node--node_type--view_mode from line 26 in the file omega/preprocess/node.preprocess.inc ?

Does that mean that this is still not working as intended?

freddybushboy’s picture

Status: Fixed » Active
fubhy’s picture

Version: 7.x-4.0-beta6 » 7.x-4.x-dev
Assigned: Unassigned » msmithcti

Yep. I asked @splatio to take a look.

msmithcti’s picture

Status: Active » Fixed

So I've tested this with the latest dev and the classes are working correctly. Have you pulled the latest version from Git? Have you cleared all caches? Are you able to reproduce this on a clean install with just Omega and Display Suite?

Setting this back to fixed. Feel free to reopen if you are able to give some clear steps for reproducing it.

Status: Fixed » Closed (fixed)

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

karlmc15’s picture

Status: Closed (fixed) » Patch (to be ported)

hi all brother
I'm apply this patch https://drupal.org/files/locale_file_exists.patch in threat https://drupal.org/node/1803330
and my site vnopsec.com fine :)

msmithcti’s picture

Status: Patch (to be ported) » Closed (fixed)

@karlmc15 - glad it is working for you.

Generally it is a bad idea to reopen an issue once it has been fixed unless you have an issue with the specific fix in that issue.

Please see [#156119] for guidelines on what issue statuses to use, as a general rule "patch (to be ported)" is the wrong status to use in 90% of situations.