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...
Comment | File | Size | Author |
---|---|---|---|
#1 | lightbox2.admin_.inc_.diff | 2.96 KB | miro_dietiker |
lightbox2.formatter.inc_.rewrite.zip | 4.17 KB | miro_dietiker |
Comments
Comment #1
miro_dietikerhi again
i totally forgot the admin page..
see the attached patch for it.
Comment #2
miro_dietikerbut i didn't want to change the issue title... :-X
Comment #3
miro_dietiker... and patch provided state, code needs review ...
Comment #4
stella CreditAttribution: stella commentedI'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
Comment #5
stella CreditAttribution: stella commentedMarked #312787: Provide a "download hi-res version" link if imagecache is used as a duplicate.
Comment #6
stella CreditAttribution: stella commentedReleased in 6.x-1.9.
Cheers,
Stella