When the search results are themed by providing odd / even classes (aka zebra stripes) the parity is often not correct. This is especially apparent when grouping the search results by type. In this case the zebra stripes will just keep alternating, instead of resetting to 'odd' when showing the first result of a group.

This can also occur when displaying non-grouped results: if a node is displaying before the search results, the first result will incorrectly be given the 'even' class.

To see this happening you will need to make sure the default zebra stripes are output in the node:

function THEMENAME_preprocess_node(&$variables) {
  // Add zebra striping to nodes.
  $variables['classes_array'][] = $variables['zebra'];
}
CommentFileSizeAuthor
#7 1264358-7-display_suite-zebra_striping.patch3.02 KBpfrenssen
#1 1264358-1-display_suite-zebra_striping.patch3.27 KBpfrenssen

Comments

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Active » Needs review
StatusFileSize
new 1264358-1-display_suite-zebra_striping.patch3.27 KB
swentel’s picture

swentel commented

Wuu, you're on a roll :) I don't have much to review this week, but I see no reason for this to not go in, so expect a commit next week or so.

pfrenssen’s picture

pfrenssen
Location Sofia
commented

No problem, you are already very much on the ball with reviewing and committing patches. Thanks for the great work, I really love Display Suite!!

swentel’s picture

swentel commented

Did a quick test and this works great!

Some thoughts about the template_preprocess_node() function:

  • I'm not sure if ds has to implement template_preprocess_node if a themer also needs to implement another one to get the zebra variable in. We have to document this - even if we don't implement it ourselves.
  • I've played with some code in ds_entity_variables looking for the parity variable so nobody has todo anything, but I'm not sure about that one either. Maybe zebra as the key would make more sense to look for in that function, but that could potentially break other implementations ?

I'll let this sink for a bit this week - need to test this quick with the apachesolr backend as well, although I don't expect any problems.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Needs review » Needs work

This patch needs work indeed. I made this backwards, starting from the template_preprocess_node().

If we simply replace the $node->ds_search_parity variable with $node->zebra there is possibly no need for template_preprocess_node(). I have to look into this though, it depends on how theme() handles the zebra stripes. If I remember correctly it is using an internal static variable to keep the count.

Using zebra stripes on nodes is not something that is often needed on regular pages, but I suppose this is rather common for search results. We could make this an option in the search configuration: 'Add odd/even CSS classes to the search results.' This would save themers the work to implement the hook.

Regarding breaking other implementations: this is tricky, but I suppose if there is a need for two separate sets of zebra-striped nodes on one page this would have to be done with custom functionality anyway. Since the number of search results can not be predicted, any subsequent nodes have a 50/50 chance of starting with 'odd' or 'even', depending on the number of results returned.

pfrenssen’s picture

pfrenssen
Location Sofia
commented

The zebra striping are done with a static variable so we have no way of overriding that (see template_preprocess()). This problem also affects other modules such as Views (see #360550: how to reset $zebra/$id for each instance of a view).

The only solution seems to be to provide a second variable as in the patch above. Providing our own template_preprocess_node() hook is indeed not desirable, as it will override any zebra striping on pages that display search results. We should leave this for the themer to implement.

A more recognisable variable name would also be nice (ds_search_zebra?).

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Needs work » Needs review
StatusFileSize
new 1264358-7-display_suite-zebra_striping.patch3.02 KB

Reworked patch. This adds a new 'ds_search_zebra' property to the node object.

A typical way to use this would be in template.php:

/**
 * Implements template_preprocess_node().
 */
function THEMENAME_preprocess_node(&$variables) {
  // Provide correct zebra striping for Display Suite search results.
  if(isset($variables['ds_search_zebra'])) {
    $variables['zebra'] = $variables['ds_search_zebra'];
  }

  // Add zebra stripe 'odd' and 'even' classes to all nodes.
  $variables['classes_array'][] = $variables['zebra'];
}
swentel’s picture

swentel commented
Status: Needs review » Fixed

Committed to restructure branch, thanks!

Status: Fixed » Closed (fixed)

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