Hi guys,

To sum up quickly, the Image Link Formatter module allows combining upon display (it's just a display formatter) the values of a Core Image field and the ones of a Link field (in a few words: it wraps the link around the image).

For multiple values, the delta of the field's values is used to match each image with a corresponding link.

After receiving several issues related with Panels, Panelizer and Fieldable panels panes:

we did a first round of testing, debugging and managed getting the formatter and its form to display correctly the related link fields that could be selected (see more at #1932184-5: Views integration), in Panels, Panelizer and Fieldable panels panes.
 

Problem identified: Delta offset, Reversed or Limit would change the delta values preventing from matching with the Link field at the Field View/Formatter level.

After doing some more testing and receiving more feedback on this issue, we found out that if specific Entity Field Panes display settings were used, such as delta_reversed (Reversed display of values), delta_offset (Offset), delta_limit (Limit), for an Image field displayed through the Image Link Formatter, it would become very difficult to match image values with their corresponding link based on delta since a new_delta would be assigned to the values of the items or entity passed to the display formatter, see in file entity_field.inc, line 155:

<?php
  $field_output = field_view_field($entity_type, $clone, $field_name, $field_settings, $language);
?>

values are passed through $clone.
 

Screenshot of the Entity Field Pane formatter settings form with advanced display options: offset, limit and reversed.:
20130821DO_ctools-entity-field-panes-advanced-display-settings.jpg

Ideally, it would be great if it would be possible to potentially pass or merge with $field_settings the properties of $conf.
This would allow passing to field_view_field the delta_offset, delta_reversed and delta_limit values in the $field_settings, which would then be passed to the display formatter in the $display variable.

I have already tested the Formatter with Views and it is possible to access the $display variable (for the formatter settings, see hook_field_formatter_view) to get Views field handler display settings, such as offset, limit or reversed options values.

I was wondering if this is a change that you could potentially consider as acceptable.
Personally, I would think that it wouldn't necessarily bring any harm to other modules and it would perhaps get closer aligned with what is done by the Views module.

Please let me know if you would have any questions, objections, comments, suggestions, recommendations or concerns on any aspects of this issue, I would be glad to provide more information or explain in more details.

Any questions, feedback, testing, changes, ideas or recommendations would be highly appreciated.
Thanks to all in advance.

Comments

dydave’s picture

Quick follow up on this issue:

Please find attached to this issue at patch against ctools-7.x-1.x-dev at e81da7a that basically merges the $conf array containing the Entity Field Pane advanced display settings and the $field_settings array which is passed to the display formatter (in field_view_field as mentioned in issue summary).
File attached as: ctools-entity-field-panes-merge-display-settings-2070217-1.patch

Basically, as long as this patch has not been applied, Image Link Formatter will not be able to fully support Advanced display settings for Entity Field Panes created through Panels, Panelizer and Fieldable panels panes:
For example, if the data to be displayed has been reversed, with an offset, it becomes very difficult to find the corresponding Link value based on items' delta.

This patch has been tested and seems to work as expected, but I would greatly appreciate to have your feedback, questions, comments, reviews, suggestions, recommendations, improvements and testing of this patch.

Feel free to let me know if you would have any further comments, issues, questions, objections, recommendations, suggestions, testing, reporting or concerns on the attached patch or any other aspects of this ticket in general, I would be glad to provide more information or explain in further details.

Thanks in advance to all for your feedback, reviews, testing and reporting.
Cheers!

merlinofchaos’s picture

Would it be safer to put the non-field settings into a single array that's safely named? My main worry would be accidentally stomping some other formatter's random setting that happens to coincide with our setting names.

dydave’s picture

Thank you so much @merlinofchaos for answering so promptly and reviewing the patch.

After quickly discussing on IRC to reduce potential back and forth, we agreed a better solution would be to use a specific name/key in the field's settings array to store all the $conf values, rather than merging the arrays which could end up creating issues with name conflicts.

Please find attached an updated patch against ctools-7.x-1.x-dev at e81da7a, that adds the pane_settings property to the field settings array, to contain the Entity Field Pane $conf values.
File attached as: ctools-entity-field-panes-merge-display-settings-2070217-3.patch.

Once again, I would greatly appreciate your feedback on this updated patch and more particularly if you would still see anything that could potentially prevent it from being committed.

Feel free to let me know if you would have any further comments, issues, questions, objections, recommendations, suggestions, testing, reporting or concerns on this updated patch or any other aspects of this ticket in general, I would be glad to provide more information or explain in further details.

Thanks in advance to all for your feedback, reviews, testing and reporting.
Cheers!

merlinofchaos’s picture

Status: Needs review » Reviewed & tested by the community
dydave’s picture

Hi @merlinofchaos,

Quick follow-up on this issue:

It has already been pending RTBC with no updates for two weeks.

I wouldn't like to sound insistant, but this issue is actually a blocker for the problem reported at #2045133: Panels and related modules: Compatibility issues, as mentioned in the summary.

I would greatly appreciate if you could please let me know if there would be anything that would potentially prevent the patch from #3 from being committed.
I would surely be glad to respond as soon as possible to any additional request from your end that could help moving this feature request forward towards getting it integrated to module's code.

A fix/patch for #2045133: Panels and related modules: Compatibility issues has already been tested, with the patch suggested at #3 and since it seems to be working fine, I would have hoped it could have been committed at some point.

Feel free to let me know if you would have any further comments, issues, questions, objections, recommendations, suggestions, testing, reporting or concerns on the patch from #3 (and marked RTBC in #4) or any other aspects of this ticket in general, I would be glad to provide more information, or re-roll the patch if necessary.

Thanks in advance to all for your feedback, reviews, suggestions, testing and reporting.
Cheers!

dave reid’s picture

I think #2336985: ctools_entity_field_content_type_render() unnecessarily alters field deltas might also be of help or a possible alternative, if the delta values just aren't modified at all.

EDIT: Wrong issue link, sorry.

dydave’s picture

Relating this issue with the ones mentioned in the summary, now the function exists on DO.

@Dave Reid
Shall I also relate this issue to #2336985: ctools_entity_field_content_type_render() unnecessarily alters field deltas?
Not completely sure how they really relate or what you meant by:

EDIT: Wrong issue link, sorry.

Friendly bump: This issue has been sitting in RTBC for more than a year.
Any additional feedback, comments, ideas, questions, suggestions or concerns on patch from #3 would be greatly appreciated.

Thanks in advance to all for your feedback, reviews, suggestions, testing and reporting.
Cheers!

japerry’s picture

Status: Reviewed & tested by the community » Fixed

Fixed based on #3 since Earl already RTBC'd it. Looks good to me. Committed.

  • japerry committed 066dcb1 on 7.x-1.x authored by DYdave
    Issue #2070217 by DYdave: Pass Entity Field Panes display settings to...

Status: Fixed » Closed (fixed)

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

dydave’s picture

Hi @japerry,

Really sorry for getting back to you so late.
I just wanted to drop you a line to say:

Thanks a lot to you a @merlinofchaos!! for committing the patch and guiding me in the first place towards this solution.
I'll get to work ASAP on related ticket #2045133: Panels and related modules: Compatibility issues for the Image Link Formatter module, since this patch should allow us to move forward with a robust solution.

Thanks again very much for all the maintenance work on the module.
Cheers!