The dubious amount of field formatters for Lightbox2 had been bugging me for quite some while.
I'm talking about the multitude of "lightbox/lightshow__image style__lightbox style" style formatters.
Imagine having 10 image styles (that's not so far fetched, I think). That will lead to no less than a hundred formatters in the display list!

I propose these obviously config level settings to be handled by not additional formatters, but the field_formatter_settings_form hook (as such settings should be).
I've implemented such a single formatter with the ability to configure the following settings per field instance:
- Lightbox type: lightshow or single lightbox.
- Image style: a select list of image styles. What to display as a thumbnail.
- Lightbox style: a select list of image styles. What to display inside the lightbox itself.
- Caption field: a list of image entity fields to choose which field to display as a caption (this one could be later replaced by some more refined display type, e.g. an added 'lightbox' display mode for image file types, etc.).

I've attached a patch including these features, hope it can be ported a.s.a.p.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Karsa’s picture

I fixed a very minor bug in the settings form (double array enveloped options, LOL).
I also attached some screenshots of how the feature works.
Note how where there previously were dozens of formatters, there is now only a single one.
I believe configuring a lightbox2 display with this newly added settings form is much more instinctive to use.

I need feedback on this!

futurist’s picture

Great improvement, thank you.

However, the patch didn't apply properly (against 7.x-1.0-beta1):

patching file lightbox2.formatter.inc
patching file lightbox2.module
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] y
Hunk #1 FAILED at 786.
Hunk #5 FAILED at 1049.
Hunk #6 succeeded at 1065 (offset -3 lines).
Hunk #7 succeeded at 1116 (offset -3 lines).
2 out of 7 hunks FAILED -- saving rejects to file lightbox2.module.rej
Karsa’s picture

Strange, I think this is because of CRLF vs. plain LF newlines, it's stupid that git doesn't support that.
I attached a plain LF diff file, it works fine for me on 7.x-1.0-beta1.

(Sorry for being gone so long :) ).

Karsa’s picture

Status: Patch (to be ported) » Needs review
yukare’s picture

This is a good improviment, i too do not like the way it is now. I will try the path and make some tests if it do not affect anything else I will commit it.

yukare’s picture

+    default:
+        $caption_display = array(
+            'label' => 'hidden',
+            );
+        $entity = entity_load('file', array($item['fid']));
+        $entity = $entity[$item['fid']];
+        $rel.=drupal_render(field_view_field('file', $entity, $caption, $caption_display));
+        break;
+  }

Can you explain to me what this does and when it is used? Because when we set the image field to use lightbox, it chooses hidden for caption, there is not a default or unset value. If this is the default caption by drupal we must then add a new option to caption field to let the user choose it.

When we resolve this simple question, the path will be commited.

yukare’s picture

Status: Needs review » Fixed

Commited the path to git: http://drupalcode.org/project/lightbox2.git/commit/aa5ddcc

Thanks by your work on this Karsa, but can we work on question in #6? With this path commited i can now work on another important issue for me(grouping).

If you will work on any change about this, please get the version on git and create a new path.

Karsa’s picture

That branch of the switch handles formatting CCK fields as the caption.
As I said, it probably would be preferable to add these options to the caption display instead of every field as an option:

  • use a field: another select would appear from which the field to use could be selected.
  • use a rendered entity: another select would appear from which the view mode could be selected.

I shall try to code this ASAP.

Karsa’s picture

Status: Fixed » Needs review
FileSize
10.68 KB

OK, coded it, this patch adds the functionality of adding a rendered entity as the caption of the lightbox.
You can now select the caption type as: hidden / filename / rendered entity / field
When selecting 'rendered entity' you can specify a view mode, when selecting 'field', you can select a field from the image entity type.

Needed to add some additional allowed tags and toy with the javascript a little to match possibly complex html in the caption attribute.

Karsa’s picture

Version: 7.x-1.0-beta1 » 7.x-1.x-dev

Oh, and - as requested - this patch applies to branch 7.x-1.x-dev.

gifad’s picture

Status: Needs review » Needs work

This new functionality is interesting, but, please, do not remove the “File title” option !
Thanks,

Karsa’s picture

Status: Needs work » Needs review

Adding it in the first place was a mistake, file entities don't have titles, only filenames. :)
(The file entity label is the filename, it doesn't have a separate "title" unless you add one - as a field, which this functionality fully supports).
See entity_get_info('file'):

entity keys (Array, 5 elements)
     id (String, 3 characters ) fid
     label (String, 8 characters ) filename
     revision (String, 0 characters )
     bundle (String, 4 characters ) type
     uuid (String, 4 characters ) uuid
gifad’s picture

This issue is about fields, not files...
“File title” is not a mistake, perhaps it just could be better named “Image title” (or “Title text”, as other popular field formatters name it).
The description in drupal-7.22/modules/image/image.install is “Image title text, for the image's 'title' attribute.”
And it's a good candidate for the lightbox caption, too

Karsa’s picture

The 'title text' and 'alt text' of the image entity are fields. Programmatically added fields, but fields nonetheless. I don't think there's any need to support them not as such, outside the already available field-as-caption functionality.

gifad’s picture

“already available field-as-caption” ?

On a fresh install I can't get it working :
“Rendered file” option gives an empty “view mode” select
“Field” option gives an empty “Field” select

after firing twice this php notice :

Location http://.../system/ajax
Message Warning: Invalid argument supplied for foreach() in form_select_options() (line 2674 of .../drupal-7.22/includes/form.inc).

Am I missing something ?

Karsa’s picture

Status: Needs review » Needs work

Ah, you must not be using the file_entity module. Ok, we could probably add this functionality when file_entity is missing AND hide the field functionality as well.
Can you maybe work on this? I won't be able to code&fix this in the next couple of weeks or so.

gifad’s picture

Status: Needs work » Needs review
FileSize
11.33 KB

Made Karsa's improvements “Rendered entity as caption” at #9 conditioned by presence of file_entity module;
If file_entity is not present, choice is given to use title or alt fields of image, as the caption.

gifad’s picture

Fixed a php notice if file_entity module not enabled.

HyperGlide’s picture

  • yukare committed aa5ddcc on 8.x-1.x authored by Karsa
    Fix for issue #1751666 by Karsa: Decrease the number of field formatters
    

  • yukare committed aa5ddcc on 7.x-2.x authored by Karsa
    Fix for issue #1751666 by Karsa: Decrease the number of field formatters
    
joseph.olstad’s picture

Version: 7.x-1.x-dev » 7.x-2.0
Issue summary: View changes
Status: Needs review » Fixed

fixed in 7.x-2.0
if you want this fix then please upgrade to 7.x-2.0

Status: Fixed » Closed (fixed)

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