Problem/Motivation

If you have a nested view-within-a-view, with the Masonry style applied to the container View. The nested view will then also get Masonry applied to it, regardless of what Views style is selected for it.

Reproduction steps

  • Enable Masonry Views module
  • Create a view listing all published content teaser. Use masonry style. Make sure this view is working OK.
  • Enable module FieldView
  • Add a fieldview to a content type and display it in the teaser
  • Create a second view listing 5 published content
  • Set one of your content appearing in the main view to use this second view in the teaser field

1: You will get the listing of nodes within the teaser being "masonry-processed" without the patch.
2: You should get the listing of nodes NOT being processed after patch.

Reviewing process

  • Follow reproduction steps and make sure the error is happening
  • Apply patch and make sure the inner view is no more processed (ie the "masonry-processed" class is not rendered).
  • Add a third Masonry view in a field of the teaser. Make sure this third inner view is masonry enabled but not the second.

Proposed resolution

Solution 1: Restraint the container (in template_preprocess_views_view_masonry() ) to the immediate inner ".view-content" of the view.
Solution 2: Build a unique ID for the view withing the template_preprocess_views_view_masonry() preprocess method and process that unique ID.

Remaining tasks

- make patch
- review patch
- commit

User interface changes

The inner views of a Masonry View will not be Masonry processed if not of masonry type themselves.

API changes

None

Impacts

Depending on if a user actually created his own template for the masonry view .tpl.php, solution 1 may break. Thus, solution 2 seems more efficient for that purpose.

CommentFileSizeAuthor
#17 nested_view_should_not_be-masonry_processed--2513922-17.patch1.39 KBDom.
#13 interdiff.txt617 bytesDom.
#13 nested_view_should_not_be-masonry_processed--2513922-13.patch1.4 KBDom.
#11 nested_view_should_not_be-masonry_processed--2513922-11.patch1.5 KBDom.
#1 nested_views_also_get-2513922-1.patch664 bytesLeon Kessler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Leon Kessler’s picture

Leon Kessler CreditAttribution: Leon Kessler commented
Status: Active » Needs review
FileSize
nested_views_also_get-2513922-1.patch664 bytes

Patch attached which will ensure masonry is only applied on the correct View. This is done using a child CSS selector - so this could break someones implementation if they are injecting any custom markup. I have tested this with views headers and it works fine.

Leon Kessler’s picture

Leon Kessler CreditAttribution: Leon Kessler commented
Issue summary: View changes
slcp’s picture

slcp CreditAttribution: slcp commented

Works a treat for my use case though I haven't tested it extensively.

Dom.’s picture

Dom. CreditAttribution: Dom. commented
Status: Needs review » Closed (won't fix)

I'm very sorry guys, I'm the new module maintainer and I don't intend to support multiple module versions.
Please update the 3.x (2.x will also deprecate when 3.x will be released) and try it.
Feel free to open a new issue if needed and provide a patch for 3.x version if your issue is still existing.
Thanks for your comprehension.

Leon Kessler’s picture

Leon Kessler CreditAttribution: Leon Kessler commented
Version: 7.x-1.x-dev » 7.x-3.x-dev
Status: Closed (won't fix) » Needs review

Look to me like the 7.x-3.x version is a straight copy of the 7.x-1.x version. When I opened this ticket 1.x was the only dev version around.

This patch still applies cleanly to the new version, no need to start a new ticket :-)

Dom.’s picture

Dom. CreditAttribution: Dom. commented

Argument accepted !
Could you explain how to reproduce for reviewing ? Are you talking about having a view B in the view A header or footer, or are you suggesting something more complex ?

Leon Kessler’s picture

Leon Kessler CreditAttribution: Leon Kessler commented

My use case was this:

I had a view that displayed teasers (as opposed to fields), this view had the Masonry style applied to it.

My teasers contained another view, which was using the list style. Masonry was being applied to both views and causing things to mess up.

Haven't tried it with views inside the header or footer, it may also be an issue, it depends on the markup.

However, re-visiting the patch, it might be better to create our own unique selector. And use that in masonry_apply(), rather than guessing the markup outputted by views.

Dom.’s picture

Dom. CreditAttribution: Dom. commented

How did you include a view in teader ? Display Suite ? Viewfield module ? Views Field View module ?

Leon Kessler’s picture

Leon Kessler CreditAttribution: Leon Kessler commented

It was with panelizer.
I'd imagine those other methods would produce similar results though (correct me if I'm wrong).

Dom.’s picture

Dom. CreditAttribution: Dom. commented
Issue summary: View changes
Status: Needs review » Needs work

I have updated the issue summary. Also I will come with a patch for solution 2 which seems better for users with custom view templates that may not include .view-content and other depending markup.

Dom.’s picture

Dom. CreditAttribution: Dom. commented
Status: Needs work » Needs review
FileSize
nested_view_should_not_be-masonry_processed--2513922-11.patch1.5 KB

Here is a patch with solution 2. It does not depend on any upper template (views-view.tpl.php for example). Only the masonry template within the module.
Please have a try at it and let me know.

Leon Kessler’s picture

Leon Kessler CreditAttribution: Leon Kessler commented
Status: Needs review » Needs work 
+++ b/masonry_views.module
@@ -22,8 +22,12 @@ function template_preprocess_views_view_masonry(&$vars) {
+  $container = '.view-' . drupal_clean_css_identifier($view->name) . '.view-display-id-' . $view->current_display . ' .' . $unique_masonry_container;

This could probably just be $unique_masonry_container no? There's no benefit of having the class names infront of it, and could lead to issues if someone is hacking their markup.

Dom.’s picture

Dom. CreditAttribution: Dom. commented
Status: Needs work » Needs review
FileSize
nested_view_should_not_be-masonry_processed--2513922-13.patch1.4 KB
interdiff.txt617 bytes

I was wondering if needed in case of a view with multiple Masonry display.
Correction patch attached.

Leon Kessler’s picture

Leon Kessler CreditAttribution: Leon Kessler commented
Status: Needs review » Needs work

Looks good, one small change...

+++ b/masonry_views.module
@@ -22,8 +22,12 @@ function template_preprocess_views_view_masonry(&$vars) {
+  $container = '.' . $unique_masonry_container;

Should be $container = '#' . $unique_masonry_container;

Dom.’s picture

Dom. CreditAttribution: Dom. commented 
+++ b/views-view-masonry.tpl.php
@@ -5,9 +5,10 @@
+<div class="<?php print $container_class; ?>">

Thus this needs to be an ID.
Is it correct semantically speaking to have an ID here (ie within the .view-content) ?

Leon Kessler’s picture

Leon Kessler CreditAttribution: Leon Kessler commented

Ah yes that needs to be changed too.

No problems having an ID here.

Dom.’s picture

Dom. CreditAttribution: Dom. commented
Status: Needs work » Needs review
FileSize
nested_view_should_not_be-masonry_processed--2513922-17.patch1.39 KB

Done it.

Leon Kessler’s picture

Leon Kessler CreditAttribution: Leon Kessler commented
Status: Needs review » Reviewed & tested by the community

Reviewed and tested. Looks good!

  • Dom. committed e68dea1 on 7.x-3.x 
    Issue #2513922 by Dom., leon.nk: Nested Views also get Masonry applied...
Dom.’s picture

Dom. CreditAttribution: Dom. commented
Status: Reviewed & tested by the community » Fixed

Thanks @leon.nk

Dom.’s picture

Dom. CreditAttribution: Dom. commented
Status: Fixed » Needs work

This correction needs to be reverted : it breaks Views Infinite Scroll compatibility :
#1808018: Make work with Views Infinite Scroll
Actually other modules may expect the markups to be standard, ie .view-content to contain only the rows, not in a nested markup.

Dom.’s picture

Dom. CreditAttribution: Dom. commented
Status: Needs work » Fixed

Applied first patch instead.

  • Dom. committed 5324bba on 7.x-3.x 
    Issue #2513922 by Dom.: revert initial correction on #2513922 to support...

Status: Fixed » Closed (fixed)

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

stevenx’s picture

stevenx CreditAttribution: stevenx commented

#1 works for me
thanks