fkelly12054 / juicebox-kelly

Juicebox moving towards Drupal 9
GNU General Public License v2.0
1 stars 1 forks source link

views display of juicebox content type #23

Open fkelly12054 opened 3 years ago

fkelly12054 commented 3 years ago

Drupal 9 testing went fair. I could use a content type that had image fields in it, create a view to display them based on the instructions of the Drupal site and the webwash article, set the view to output to a page and then call up that page by it's url.

The content type had five images. #2 through 5 should show as thumbnails and do when I retrieve that content type. However, in the view only one image displays and then that same image is used as thumbnails in positions 2 through 5. x main image

y thumbnail 1 (main image x) z thumbnail 2 main image x a thumbnail main image a b thumbnail main image b

I'll try a ping capture later

fkelly12054 commented 3 years ago

On drupal 8 site, I have a juicebox related content type with three images. Looking at the content type itself: no views, I get image 1

thumbnails row image 1 image 2 image 3
(so image 1 is repeated down on the thumbnail row

Putting them into views and following the instructions using "display all values in the same row" I get something like: Image 1 thumbnails row Image1 repeated then image 3 (image 2 left out)

However if I uncheck the "display all values in the same row" setting I get

                                  image 1

thumbnail area image 1 image 1 image 1 image 3 image 3

Not sure how this is working on your system Dave. The instructions on the Drupal Juicebox module documentation and the webwash article are at least 3 years old and some go back 7 or 8 years.

I'm not sure I want to be chasing after this. Maybe as a bug fix for a later release? What do you think? I may try it on my production site which is code prior to any of our changes.

fkelly12054 commented 3 years ago

On my production site with Juicebox 8.x-2.0 beta 4 I get:

Exception building Juicebox embed code for view: !message in Drupal\juicebox\Plugin\views\style\JuiceboxDisplayStyle->getItems() (line 359 of /home/fkelly5/public_html/drupal8/modules/juicebox/src/Plugin/views/style/JuiceboxDisplayStyle.php).

after creating a view to access a Juicebox content type that has a couple of images in it. There is all kind of flakiness in even building the view with warnings to include an image field (when there is already one there).

I'm open to suggestions but my comment about not chasing after this stands. Maybe if there is are users who want to pursue it we can help them as a follow on.

fkelly12054 commented 3 years ago

back to testing our new code with Drupal 8 in my virtual host (locally) ... the function getItems in JuiceBoxDisplayStyle.php is returning a fouled up array of items. These items do correspond to what is shown on the resulting Gallery built from views, but basically show 3 of the first image and then 2 of the next. var_dumps confirm this The variable $source_type is set to "file_field" which is what I think we want. But then it is reading in an incorrect list of files/images. Throw in a var_dump at line 377;

foreach ($view->result as $row_index => $row) {

// var_dump($row);

and then another at 410 after the array $items is loaded and you will see:

// var_dump($items); return $items;`

Conclusion so far, views is returning an incorrect list of images when built on with a juiceboxformat. More research needed.

fkelly12054 commented 3 years ago

First I set up a new content type and put a couple of images in it. Didn't use any Juicebox features, just plain old images. Set up a view. Could retrieve that content type and show the images going down the page. Other formatting is of course possible but just wanted to set a baseline.

Then went into my view for jbox-images. In the FORMAT section I made it be an unformatted list rather than a Juicebox Gallery. Now, when I retrieve that page that I generate from the view, it works almost perfectly. I get the main image, then any other images that are in that content type show as thumbnails. I can click on them and they show in the main image area.

I said "almost perfectly". On my test system I have two content items of the jbox-image content type. Both show (vertically) on the page because my selection criteria (filter in views speak) is the whole content type. That could be worked around easily enough.

Time to see what having the format be Juicebox Gallery versus unformatted list is doing differently.

fkelly12054 commented 3 years ago

back to debugging. So, to repeat, when I set the FORMAT section to an unformatted list in the view and the view is set to have a page display named jbox-image the page(s) display properly. Setting the FORMAT section to Juicebox Gallery results in an error.

"Error: Wrong parameters for Exception([string $message [, long $code [, Throwable $previous = NULL]]]) in Exception->__construct() (line 368 of modules\juicebox\src\Plugin\views\style\JuiceboxDisplayStyle.php).

Exception->__construct('Empty or invalid field source @source detected for Juicebox view-based gallery.', Array) (Line: 368) Drupal\juicebox\Plugin\views\style\JuiceboxDisplayStyle->getItems('') (Line: 282) Drupal\juicebox\Plugin\views\style\JuiceboxDisplayStyle->buildGallery(Object) (Line: 257) Drupal\juicebox\Plugin\views\style\JuiceboxDisplayStyle->render(Array) (Line: 2170

The field source comes from the confGetFieldSources function so now looking at that.

fkelly12054 commented 3 years ago

So with view Format set to Juicebox.. the $items array in confGetFieldSources contains:

array (size=3) 'field_options_images' => array (size=1) 'field_jbox_image' => string 'Content: Jbox image*' (length=20) 'field_options_images_type' => array (size=1) 'field_jbox_image' => string 'file_field' (length=10) 'field_options' => array (size=0) empty

but with Format set to unformatted list it doesn't appear that confGetFieldSources ever gets called. Throwing in a die() just to confirm. Yep ... I don't think it ever gets used. So a different path through the code if the view has an unformatted list.

So, the key here is that the so-called "source-type" with a value of "file-field" is getting passed back from confGetFieldSources to getItems when the FORMAT is set to Juicebox_Gallery and not when it is set to unformatted list. And we recall that the list of files generated in the getItems function is messed up. And double-confirm, when the view is set to unformatted list, we never even execute the getItems function, so we never get into the screwed up file lookup area there.

fkelly12054 commented 3 years ago

Working backwards, and looking at the function buildGallery, we can confirm that when FORMAT is set to unformatted list, we never even get to the buildGallery function. So where does views go? Humm..

D4KO commented 3 years ago

If I have View Format: unformatted list and field format Juicebox Gallery it works for me. I can see all the images. If I have View Format: Juicebox and field format Juicebox Gallery I can see only one image. If I have View Format: Juicebox and field format Render entity I can see only one image. Only the first scenario works for me.

On Sat, Dec 19, 2020 at 8:36 PM fkelly12054 notifications@github.com wrote:

Working backwards, and looking at the function buildGallery, we can confirm that when FORMAT is set to unformatted list, we never even get to the buildGallery function. So where does views go? Humm..

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fkelly12054/juicebox-kelly/issues/23#issuecomment-748515884, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJQHTBH4BEI7PBVKDBZGDWLSVT6E7ANCNFSM4U3JSXHQ .

D4KO commented 3 years ago

I think ff you use unformatted list view format then the unformatted list formatter is be called. It renders the classic formatted list (ul li) with images inside. Then if Juicebox field formatter is set, JuiceboxFieldFormatter.php is called and build the gallery.

If you use Juicebox view format, JuiceboxDisplayStyle.php formatter should be called and build the gallery.

On Sat, Dec 19, 2020 at 8:52 PM DK koutas@gmail.com wrote:

If I have View Format: unformatted list and field format Juicebox Gallery it works for me. I can see all the images. If I have View Format: Juicebox and field format Juicebox Gallery I can see only one image. If I have View Format: Juicebox and field format Render entity I can see only one image. Only the first scenario works for me.

On Sat, Dec 19, 2020 at 8:36 PM fkelly12054 notifications@github.com wrote:

Working backwards, and looking at the function buildGallery, we can confirm that when FORMAT is set to unformatted list, we never even get to the buildGallery function. So where does views go? Humm..

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fkelly12054/juicebox-kelly/issues/23#issuecomment-748515884, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJQHTBH4BEI7PBVKDBZGDWLSVT6E7ANCNFSM4U3JSXHQ .

D4KO commented 3 years ago

it builds it but only with the first image in my cast. Somethings buggy

On Sat, Dec 19, 2020 at 9:04 PM DK koutas@gmail.com wrote:

I think ff you use unformatted list view format then the unformatted list formatter is be called. It renders the classic formatted list (ul li) with images inside. Then if Juicebox field formatter is set, JuiceboxFieldFormatter.php is called and build the gallery.

If you use Juicebox view format, JuiceboxDisplayStyle.php formatter should be called and build the gallery.

On Sat, Dec 19, 2020 at 8:52 PM DK koutas@gmail.com wrote:

If I have View Format: unformatted list and field format Juicebox Gallery it works for me. I can see all the images. If I have View Format: Juicebox and field format Juicebox Gallery I can see only one image. If I have View Format: Juicebox and field format Render entity I can see only one image. Only the first scenario works for me.

On Sat, Dec 19, 2020 at 8:36 PM fkelly12054 notifications@github.com wrote:

Working backwards, and looking at the function buildGallery, we can confirm that when FORMAT is set to unformatted list, we never even get to the buildGallery function. So where does views go? Humm..

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fkelly12054/juicebox-kelly/issues/23#issuecomment-748515884, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJQHTBH4BEI7PBVKDBZGDWLSVT6E7ANCNFSM4U3JSXHQ .

D4KO commented 3 years ago

9pm here. See you later ;)

On Sat, Dec 19, 2020 at 9:05 PM DK koutas@gmail.com wrote:

it builds it but only with the first image in my cast. Somethings buggy

On Sat, Dec 19, 2020 at 9:04 PM DK koutas@gmail.com wrote:

I think ff you use unformatted list view format then the unformatted list formatter is be called. It renders the classic formatted list (ul li) with images inside. Then if Juicebox field formatter is set, JuiceboxFieldFormatter.php is called and build the gallery.

If you use Juicebox view format, JuiceboxDisplayStyle.php formatter should be called and build the gallery.

On Sat, Dec 19, 2020 at 8:52 PM DK koutas@gmail.com wrote:

If I have View Format: unformatted list and field format Juicebox Gallery it works for me. I can see all the images. If I have View Format: Juicebox and field format Juicebox Gallery I can see only one image. If I have View Format: Juicebox and field format Render entity I can see only one image. Only the first scenario works for me.

On Sat, Dec 19, 2020 at 8:36 PM fkelly12054 notifications@github.com wrote:

Working backwards, and looking at the function buildGallery, we can confirm that when FORMAT is set to unformatted list, we never even get to the buildGallery function. So where does views go? Humm..

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fkelly12054/juicebox-kelly/issues/23#issuecomment-748515884, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJQHTBH4BEI7PBVKDBZGDWLSVT6E7ANCNFSM4U3JSXHQ .

fkelly12054 commented 3 years ago

The way the arrays are built and used in JuiceboxDisplayStyle.php is definitely messed up. I don't see how it could ever have worked properly. Unformatted list works okay, but it doesn't go through this code. I have the program torn apart but I haven't been able to figure out exactly how to put it back together. To be continued. The switch statement in getItems should be retrieving the image files, but the input to it is not correct. It will need more work. I'll be back tomorrow.

fkelly12054 commented 3 years ago

More frustration during Sunday's debug session. After numerous dead ends I thought "the Drupal 9 version" (separate site, separate code from same code base) but the view sort of worked, with exception that the base image shows four times in the thumbnail area ... what's the difference. So debugging side by side. The options returned by the view in Drupal 8 contains:

   'image_field' => string '' (length=0)
      'image_field_style' => string 'juicebox_multisize' (length=18)

The same array in drupa9 contains:

      'image_field' => string 'field_jimage' (length=12)
      'image_field_style' => string 'juicebox_multisize' (length=18)

and there is the start of our problem. Because, that little field_image text becomes the $source_field that is passed to the getItems function and without it being correct getItems() can't find image files it needs. Or course Drupal 9 is still returning an incorrect list of files for the thumbnails so that's another bridge we'll have to cross.

But the views in the two versions are set up identically (I think) so first step is to find out why the Drupal 8 version is not returning 'image_field' properly from the view.

fkelly12054 commented 3 years ago

Aha. So, when creating the view, on the line where you choose Juicebox Gallery for the FORMAT there is a settings field. Picking "settings" you get a page style options screen. There you can select the image source and thumbnail source. On Drupal 9 I had it set to the image field I was using. On Drupal 9, I did not. I bet that's going to solve this immediate problem. If so, we should probably kick off some sort of error message at an appropriate early point in JuiceboxDisplayStyle.php and have clear directions in the documentation.

And, yes, early indications are that this works! Changing the settings for the Juicebox Format in view to point to the image field does result in the proper settings getting sent over to JuiceboxDisplayStyle ... now to undo some of the damage my debugging may have done and see if we can get a good set of files returned.

And hooray. After synching up the code between Drupal 8 and Drupal 9 sites, I have the same bug in both. The view based galleries are displayed: just with the wrong list of images. That should be a walk in the park :)

fkelly12054 commented 3 years ago

Noting that a view created this way will only return one instance of a content type. You can affect this by selecting sort criteria (say for instance authored ascending or descending) but to really control it you would need to add other fields to the content type to select on in the view.

fkelly12054 commented 3 years ago

Oh joy. It appears that in the getItems function we just do the so-called "bulk load" of file entities at the bottom of the function. Strange since it will not be targeted on any one content item.

fkelly12054 commented 3 years ago

An update: first was reading the Drupal issue queue, particularly; https://www.drupal.org/project/juicebox/issues/1955704

I think there may be some intrinsic problems in what we are attempting. First, the thread above (from 7 years ago) recommends to put one image per node. That's not what we are attempting here. And we are having a "mess" with retrieving the "target-ids" properly from the view to use in the Juicebox gallery based view.

Second, my experiment includes creating a content type with two separate content items. Views will retrieve all of these content items. It could "filtered" if I put another field, say a tag each item, then set the filter in views to use that tag. And made the value of the tag unique for each content item. As it is, I think views is returning a mish-mash of data to the JuiceBoxDisplayStyle program and keeping it from building a proper list of fids (file ID's). You can return so called target ids from the views object but I don't think that doing a var_dump on the views array is giving us a full picture of what is stored in views itself. With an unfiltered view returning a bunch of content type items, you are likely to get a mess of returned target ids that is going to build a mess of a list of files to use. I think.

fkelly12054 commented 3 years ago

I finally got a view to work by totally cannibalizing the code in getItems. The root of the problem (I think) is that views returns more than one target_id ... perhaps in the field: public 'node__field_jbox_image_field_jbox_image_target_id' => string '255' (length=3)

(that is part of a much longer returned array). That is implied (to me at least) in the oblique comment in the original authors code: // The target IDs value comes in a mixed format depending on // cardinality. We can only use one ID as each view row can only // reference one image (to ensure appropriate matching with the // thumb/title/caption data already specified on the row).

In my test case, I only had two images in the content item so I restricted the foreach loop to one pass (index=0) and still was able to retrieve two target_ids which were used to retrieve two files. The var_dump doesn't help with diagnosing this ... in fact it's misleading you to think that there is only one target_id being retrieved.

To re-purpose an early pun, I'm not sure the juice is worth the squeeze in chasing this further. You can create a totally useful gallery with a simple content type without using any views. If there was some way to reliably retrieve a list of target_ids (which become file ids easily) then this might be worth pursuing. Maybe I'll learn enough about views structure along the way to make this possible.

Probably should post something in the Drupal issue queue saying "don't use views for this purpose at this time) and refer them to the 7 year old discussion about views and Juicebox (while noting that the Node Gallery module mentioned in that discussion has been obsolete for YEARS.

fkelly12054 commented 3 years ago

Funny thing. I just added a third image to my test content type (the one being referenced in the view). Just looking at the content type outside of views, it works perfectly: one main image and three thumbnails. To my surprise it also worked in the view. So, the view must be returning target ids's with one pass ... I have the foreach loop "crippled" to only work if: if ($row_index == 0) {

yes it must be getting all three target id's from that one pass. It that's really true, we might not even need a foreach loop at all. We need a better way of knowing what is hidden in: public 'node__field_jbox_image_field_jbox_image_target_id' => string '255' (length=3) that is retrieved from the view.

fkelly12054 commented 3 years ago

added a couple more images, to the test content type. They work immediately with the view. Which leads me to think that we only need one "pass" at the foreach loop ... or maybe some syntax that does away with the loop entirely.

fkelly12054 commented 3 years ago

just for kicks. I removed the filter on the tag for the view. I have two content type items, each with different tags. The first item has 3 images. They were displaying fine when the filter for their tag was on. The second item had two images. Taking the tag out adds those two images to the display (giving 5). This could be a way to build flexible galleries ... and the tag filters could be exposed to user choice. And this looks like it is much more efficient than rummaging through the whole foreach loop looking for fids. I'm sure the logic can be streamlined further ... probably don't need a foreach loop at all.

I'm not sure this will work under all conditions ... work in progress.