CommentFileSizeAuthor
#8 lazyloader-use_theme_image_for_image_tag-2531426-8.patch1.81 KBperennial.sky
#1 lazyloader-use_theme_image_for_image_tag-2531426-1.patch1.52 KBperennial.sky

Comments

perennial.sky’s picture

perennial.sky commented
Status: Active » Needs review
StatusFileSize
new lazyloader-use_theme_image_for_image_tag-2531426-1.patch1.52 KB

Here is the patch

loparr’s picture

loparr commented

@akashjain can you explain please?

perennial.sky’s picture

perennial.sky commented

Hi loparr,

It is not a bug, i was using this module and saw the code that it uses img tag, drupal is already has api to render img tag, then i think we should use this api instead of img tag, just to improve the code.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Status: Needs review » Closed (won't fix)

@akashjain132,

7.x-1.x is considered stable and will only receive bugfixes. Any new development needs to happen on the 2.x branch, which (I think) already uses the theme_image everywhere. I will therefor mark this closed won't fix.

perennial.sky’s picture

perennial.sky commented
Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Closed (won't fix) » Needs review

Hello legolasbo

opening this issue, just saw code in 7.x-2.x branch, it uses img tag instead of image_theme.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

The patch in #1 doesn't apply to 2.x

perennial.sky’s picture

perennial.sky commented
Assigned: Unassigned » perennial.sky
perennial.sky’s picture

perennial.sky commented
Assigned: perennial.sky » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new lazyloader-use_theme_image_for_image_tag-2531426-8.patch1.81 KB

Here is the patch against 7.x-2.x branch

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Status: Needs review » Needs work 
+++ b/lazyloader.admin.inc
@@ -38,10 +38,13 @@ function lazyloader_admin() {
-  $loader_dir = base_path() . drupal_get_path('module', 'lazyloader') . '/loader/';
   $loader = array(0 => t('None'));
+  $loader_image_path = drupal_get_path('module', 'lazyloader') . '/loader/loader-';
+  $variable['alt'] = 'Lazyloader Icon';
+  $variable['attributes'] = array();
   for ($i = 1; $i <= 8; $i++) {
-    $loader[$i] = '<img alt="Lazyloader Icon" src="' . $loader_dir . 'loader-' . $i . '.gif" />';
+    $variable['path'] = $loader_image_path . $i . '.gif';
+    $loader[$i] = theme_image($variable);

This seems out of scope for this issue when I look at the title. Since there is no issue summary I don't know what the actual scope of this issue is.

  • legolasbo committed ce53358 on 
    Issue #2531426 by akashjain132, legolasbo: Never use img tag, always use...
legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Status: Needs work » Fixed

Committed a slightly altered version of your changes to lazyloader.module, but left out the out of scope changes. If you think those changes should also be committed. Please open a new issue clearly explaining why.

Status: Fixed » Closed (fixed)

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