Comments

handkerchief created an issue. See original summary.

omar alahmed’s picture

StatusFileSize
new1.97 KB

Thanks - The patch is working, however, the entity reference field name should not be hard-coded like this: $item = $item->entity->field_media_image[0];

The patch is re-rolled to be applicable from the module root not the site root.

ammar qala’s picture

Add support for fieldable file entities

ammar qala’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: add-the-support-for-fieldable-file-entities-2996745-3.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

The last submitted patch, 2: add-support-for-fieldable-file-entities-2996745-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

ammar qala’s picture

fixing operator

Status: Needs review » Needs work

The last submitted patch, 7: add-the-support-for-fieldable-file-entities-2996745-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

muckermarc’s picture

The patch in #7 worked for me against the latest dev branch, thanks!.

However I did have to adjust the entity reference field name in line 88 of the patch as Omar suggested from:
$item = $item->entity->field_media_image[0];
to:
$item = $item->entity->field_image[0];

Is there anyway we can avoid hard-coding this field name?

bobemoe’s picture

I've just migrated all my images to the media entity and I think this may be why I cant use JuiceBox to show them. As media entity becomes more core this will become more important.

mellowtothemax’s picture

Patch worked for me too, however instead of referencing the media view mode (render entity), I had to choose the juicebox formatter widget directly in the node's manage display and not in the media type's manage display.

Note this works with Drupal commerce variations

bobemoe’s picture

Just got the patch working for me too but had to use #3 not as #7 didn't apply. I did not need to manually modify any lines.

Drupal 8.8.1

bobemoe’s picture

Although this is working brilliantly for "images", it (perhaps unsurprisingly) errors out if I have a "remote video" in my media field.

Call to a member function isDisplayed() on null in .../juicebox/src/Plugin/Field/FieldFormatter/JuiceboxFieldFormatter.php on line 279

bobemoe’s picture

From the JuiceBox site: "Juicebox can load JPGs, GIFs (including animated GIFs) and PNGs. Any other file types (text, video etc.) are not supported." so it looks like its a no-go to get rendered entities into the sideshow. Shame :(

I would suggest adding a check to make sure the referenced media entity is of bundle "image" and skip adding it to the gallery if it is not.

bobemoe’s picture

This patch no longer applies on 2.0.0-beta4 :(

bobemoe’s picture

I've refactored patch #3 to apply on beta4.

fkelly12054@gmail.com’s picture

Not to rain on this parade, but it seems to me like we need to prioritize. We need a version of Juicebox that will work with Drupal 9, which also means eliminating the use of the contrib module "libraries" and using core facilities instead and fixing all deprecated PHP code that won't work in Drupal 9. When we have a stable Drupal 8/9 version it will be time to take on other projects such as this. If multiple people are trying to apply patches to differing versions of the code, chaos will rule.

D4K0’s picture

The patch in #16 works for me on Drupal 8.9.6/Juicebox 8.x-2.0-beta4

D4K0’s picture

Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
D4K0’s picture

Status: Reviewed & tested by the community » Needs work

I've changed the issue status back to 'Needs work' as the patch #16 test failed.

D4K0’s picture

fkelly's right #17
We need a version of Juicebox that will work with Drupal 8/9.

fkelly12054@gmail.com’s picture

@dk40. Thank you. Most of what I'm proposing is summarized in https://www.drupal.org/project/juicebox/issues/3184078 (Moving forward from 8.x-2.x.dev (with Library and D9). Our maintainer agreed to a release that will move us toward that a few weeks back. Since then I've been working incrementally on making the code more Drupal standards compliant. I laid out a couple of issues remaining in the 3184078 issue. I have a couple of object oriented coding text books out and and working my way through them. There's really just two "dependency injection" issues remaining. I can run my code on a Wamp localhost (virtual host) system with Drupal 9. It works to view albums and to configure Juicebox and to create new gallery items and edit existing ones. But that's not a sufficient test I know.

My version of the code is at: https://github.com/fkelly12054/juicebox-kelly. Anybody who wants to try it ON A TEST SYSTEM, is welcome. I'm working on the existing issues there. As time permits I'll see if the code in the patch in this issue will work there, but I really want to nail down the dependency injection issues first. I think they could cause problems with Drupal 9.

It would be helpful if a fully qualified developer (maintainer) could review my changes. Then I think we need a beta type release that removes the library dependency and (hopefully) works with Drupal 9. Then a period of testing. Then go gold!

D4K0’s picture

fkelly12054@gmail.com’s picture

A couple of us have a test version of the code up and running on a separate github account. We've tried to apply the patches listed here.

In order to test the results I need to know what steps you are taking. My initial effort was to:

create a content type, I called it jbox media
add two fields: a body and an media image
the "widget" for the media image is media library

on the structure/content type screen if I set the format column to "rendered entity" and the gear icon I choose "rendered as media library" then it "sort of" works. I can view the content type with my three images in it.

But I don't really understand what the purpose of this is. Doing it this way seems to have no relationship to the Juicebox module.

Could we step back? What are you trying to accomplish with this patch and capability? What type of Juicebox gallery do you want to create? How do you plan to get images in it? If I know that I can perhaps test.

Thanks.

D4K0’s picture

This patch was handy for me in this use case:

In Content Type - Manage Fields
I have one multi-value media image reference field.
In Content Type - Manage Display
I displays the field in 'JuiceBox gallery' format.

Whithout this patch, Juicebox galley format simply didn't work for me in this setup with media image multi-value reference field.
It worked only with classic image fields.

D4K0’s picture

The patch just fixed my specific case that time. I have no idea about its eventual other consecvences.

D4K0’s picture

I have content type 'Product' with the multi-value media imafe reference field. The field is displayed with Juicebox format.

D4K0’s picture

Version: 8.x-2.0-beta3 » 8.x-3.x-dev
Assigned: Unassigned » D4K0
Status: Needs work » Fixed

The patch #16 was incorporated into 8.x-3.x branch and introduced in 8.x-3.0-alpha1 version.

neslee canil pinto’s picture

+1, @D4K0 thanks

neslee canil pinto’s picture

Assigned: D4K0 » Unassigned

Status: Fixed » Closed (fixed)

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