ATM it fails, because you need to add an image field and rewrite the results to use the alt text. Since juicebox currently tries to load fields based on the row name in views (which is IMO a whole other can of worms), it fails.
E.g. I have a content type with media reference field. The media bundle contains an image field (with singular cardinality). So I add a relationship and then the image field which is aptly called field_image.
To add the alt text, I add the field_image again, hidden and with the results rewritten with tokens {{ field_image_1__alt }}. The JuiceboxDisplayStyle tries to load the field field_image_1 from the entity, which doesn't exist.
Patch on the way
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | juicebox-views_properly_track_field_name-2855072-4.patch | 3.44 KB | rjacobs |
| #2 | impossible_to_add_image-2855072-2.patch | 1.03 KB | daften |
Comments
Comment #2
daften commentedNo test cases yet, but I guess I'd need to adapt one of the test views and add a field like this, set it as title and see if it's in the output correctly?
Comment #4
rjacobs commentedYea, I actually think the fact that we are using the view field name and entity field name interchangeably is the most important issue here. This all stems from the fact that I can't find a way to get entity field definition info via the views API directly. We need the field definition info in order to get the field type, and though this info is there internally within the views data objects, there's no way to get at it via a public interface. So instead we need manually load the entity info via our own entity manager, and then get the type. If it was not for this step we would never need to worry about the field name and whether-or-not it matches the way views keys its fields. This always bothered me. Anyway, unless there is in-fact a way to get the field type info directly from views (someone please tell me if there is!) I think we should at least focus on making sure we derive the correct entity field name for our manual lookup instead of assuming it's the same at the views field name (which clearly will not work in all cases).
@daften, it seems like you patch would address this by detecting the fact that "image_field_1" is not a real entity field, and would therefore not mark it as a potential image source field. Though this would probably work most of the time I think it still leaves the door open for other problems. I think the most robust approach is to treat all additions of an image (or file field) as potential image source fields, no matter if they are repeats, have rewriting, are added in a specific order, etc. This would be more consistent with the initial views integration intent for the module as well as the behavior in 7.x-2.x.
From what I can see the handler does store the proper entity field name as a public property so I think we can just use that instead of assuming the views field key matches. This way out logic will know that "image_field" is the entity field name for both views fields "image_field" and "image_field_1". I'm adding a patch that does this along with some other cleanup.
Comment #5
rjacobs commentedAlright, so it also occurs to me that the long explanation above does not actually address the use case in the OP :) There are really 2 issues here, the first (main) one described in #4 and the second one being how to deal with rewritten image fields as title/caption sources. As noted above I think the default behavior should be that all image and file fields added to a view are not marked as title/caption sources. The module has always used the raw fid data in these views fields anyway as there is just no way to easily detect which ones have rewriting that makes them safe strings, etc. So assuming we keep that design intent in place one possible solution for this case is to just add a non-image field to your view (say an ID field, or some generic global field) and then rewrite your rewritten image field (the one that outputs the alt value) into that. The final result can be used as your title or caption source in the Juicebox config.
In fact I don't think you even need to have 2 instances if your field_image on the view. Just add one, rewrite it to be
{{ field_image__alt }}and then use it as your image source in the Juicebox config. This works because the module ignores the rewritten output and just cares about the raw fid data represented via the field. Then add an ID field (or whatever dummy field you want), rewrite it to be{{ field_image }}and set that as your caption source. Views will render the alt text as a result of these rewrites into a field that Juicebox can happily use as a text (title/caption) source.It sounds a little hacky (rewrite of a rewrite), but I think it may technically be the cleanest way to go at this point. @daften, what do you think?
Comment #7
rjacobs commentedWhoops, had a newline character issue with the patch from #4. Here's the same one again.
Comment #8
daften commentedThanks for the quick feedback (and the long explanation :P )
I've tested the patch, and it does what it says, but there's one use case I think you've forgotten. What if you want to put the image title as title and the alt field as caption (or vice versa). You can't do that with the latest patch. My method was hacky, but that's also because of the problems you've described (I looked into getting field info through views and couldn't find it either atm).
The latest patch will at least not crash and burn when there's image view fields that don't have a corresponding entity field, so it's an improvement.
I'd also propose to add something to documentation about this approach.
Comment #9
rjacobs commentedI'm pretty sure you could do this by just repeating the setup from #4. In that case the view would have 2 additions of field_image and 2 "dummy" fields that are not image/file fields. Something like:
field_imagerewritten to{{ field_image__alt }}(and used as Juicebox image/thumb source)field_image_1rewritten to{{ field_image__title }}{{ field_image }}(and used as Juicebox caption source){{ field_image_1 }}(and used as Juicebox title source)Would that do the trick?
Comment #11
rjacobs commentedI went ahead and committed #7. Though the patch evolved a bit from #2 I still gave commit credit to daften as he got the ball rolling on the important variables. I hope that #9 addressed any missing use cases. I'll adjust the status but if something still appears missing here please feel free to re-open or create a new follow-up.