Hello stella

As i told you this is the code i was working on.
While i started to add a new feature (download original link), i recognized some problems with argument codes and then i've been stumbled upon a lot of things. Finally i decided to rearrange a lot of code for consistency.

This is the full list done in the file:
- original image download link in lightbox, configurable, variable for text, added generic link separator
- restructured processes in theme rendering: title, caption, details
- using field_name instead of field... where field is NO array
- image alt now contains title if it was empty
- $item usage only for item data, not for formatting and output rendering options. This has been corrected after looking into many others' contributions. Misusing $item as a injection into formatters is strictly wrong. added $args where needed
- at least 4 situations less where php would alert about undefined variables.
- strict variable naming if one changes format (such as $attributes_html = drupal_attributes($attributes); ) i changed a few variables to improve understanding of code

I'd suggest also you to update the readme.txt and add i18n variable definitions if people want to translate them:
lightbox2_node_link_text
lightbox2_original_link_text
lightbox2_image_count_str
lightbox2_page_count_str
see details at http://drupal.org/node/83514

I've tested things as much as possible, but since this is a rewrite you possibly shouldn't release it as a one-step pass as stable. We need some people to check if everything still works fine.

See the file attached:
lightbox2.formatter.inc_.rewrite.zip
(i've had issues with uploading this file, hope it works!)

Thanks a lot for all feedbacks...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

miro_dietiker’s picture

Title: replacement code for formatter » forgot admin page diff
FileSize
2.96 KB

hi again

i totally forgot the admin page..
see the attached patch for it.

miro_dietiker’s picture

Title: forgot admin page diff » replacement code for formatter & admin patch

but i didn't want to change the issue title... :-X

miro_dietiker’s picture

Status: Active » Needs review

... and patch provided state, code needs review ...

stella’s picture

Status: Needs review » Fixed

I've committed this, though with a few changes, including the adding of a 'download original image' permission and download support for images created by the image node. I also had to modify hook_theme() to account for changes to the theme functions, and add additional calls to variable_del() in the hook_uninstall().

Cheers,
Stella

stella’s picture

Category: bug » feature
stella’s picture

Released in 6.x-1.9.

Cheers,
Stella

Status: Fixed » Closed (fixed)

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