acrylian / zp_picturefill Goto Github PK

TL;DR
It would be great if the README had that for image.php,

printDefaultSizedImage(getImageTitle())
needs to be changed to
printHDDefaultSizedImage(null, true)

to work and work without errors.

The long version:
When updating our template (a slightly modified Basic theme) with zp_picturefill, to get the thumbnails to work, all we really had to do was add “HD” to the functions you had listed. So,

printAlbumThumbImage(getAnnotatedAlbumTitle())
in album.php became:
printHDAlbumThumbImage(getAnnotatedAlbumTitle())

All the thumbnails worked on all the pages and that was great, but when it came time to update image.php, that was when we ran into problems.

Changing
printDefaultSizedImage(getImageTitle());
to
printHDDefaultSizedImage(getImageTitle());

got us a page with no image. No rest of the page either, it just dropped off after where the image should be (and nothing was in the debug log).

We do use a custom image size that is slightly larger than the default. From the README, I thought that it might mean that we would need to use ResponsiveCustomSizedImage() instead, but that caused all kinds of different problems. No content loading on the pages, pages with password protected images instead, and so on.

After a bit of trial and error, and your post here- http://www.zenphoto.org/support/topic.php?id=1007440#post-1834935, I removed the “getImageTitle()” and that got the page working.

While the page worked, we were getting this in the debug log for every hit on the image page-

Zenphoto v1.4.10[removed]

WARNING: Missing argument 1 for printHDDefaultSizedImage(), called in /Applications/MAMP/htdocs/site/themes/basic_site/image.php on line 80 and defined in /Applications/MAMP/htdocs/site/plugins/zp_picturefill.php on line 640

printHDDefaultSizedImage called from include (image.php [80])

from include (index.php [129])

from index.php [28]

{21213:Wed, 28 Oct 2015 11:29:51 GMT}

NOTICE: Undefined variable: imgobj in /Applications/MAMP/htdocs/site/plugins/zp_picturefill.php on line 642

printHDDefaultSizedImage called from include (image.php [80])

from include (index.php [129])

from index.php [28]

So changing the entry (as you describe here- http://www.zenphoto.org/support/topic.php?id=1007440#post-1834937) to printHDDefaultSizedImage(null, true) stopped the error messages from showing up.

It would be great if this info was added to the README, so other people don’t have a similar issue.

I installed zp_picturefill under the plugins folder but it is not showing up under the “Plugins” tab to activate. I have another optional plugin installed (the really handy preloadimage.php), so I know it works.

I thought maybe it just needed to be in the plugins folder, so I modified my theme to use the new image calls, but all I got was pages with no images.

It looks like line 302:

$img_sd = $imgobj->getCustomImage(NULL, $w, $h, NULL, NULL, NULL, NULL, $thumb, $effects);

should read:

$img_sd = $imgobj->getCustomImage(NULL, $width, $height, NULL, NULL, NULL, NULL, $thumb, $effects);

The first thumb (SD and HD) have good size.
The 2nd thumb have a 2x size
The third thumb have a 4x size
...

In function getHDImageThumb(), after

setOption('thumb_size', $thumbsize * 2, false);
$img_hd = $imgobj->getThumb();

I think we need to come back to old setting:

    setOption('thumb_crop_width', $cropw, false);
    setOption('thumb_crop_height', $croph, false);
    setOption('thumb_size', $thumbsize, false);

And for line 144

$html .= '<img srcset="' . $standard_source . '" alt="' . $alt . '">';

I think we definitively need the width and height attributes...

Picturefill not only supports <picture> but also <img srcset="">. They seem to do the same but apparently that is not exactly true. <picture> sets fixes images while <img srcset> is more relative and leaves it to the browser what to display when. So in many cases to use this is more clever than <picture> actually.

Recommended reading with lots of details:

• http://www.smashingmagazine.com/2014/05/14/responsive-images-done-right-guide-picture-srcset/
• http://ericportis.com/posts/2014/srcset-sizes/ (I will implement the srcset+sizes way)
• basic standard/high density img srcset support
• basic breakpoint based img srcset & sizes support

I notice that the .php file is named zp_picturefull, when the project is named is zp_picturefill. Is this intentional, or for that matter, is this even a potential issue?

1_ There is a < /picture > tag which is not open:

Line 111

$html = '<!--[if IE 9]><video style="display: none;"><![endif]-->';

should be

$html .= '<!--[if IE 9]><video style="display: none;"><![endif]-->';

2_ There is 2x $alt variable in printHDImageThumb function:

Line 456

function printHDImageThumb($alt, $hd = false, $alt = NULL, $class = NULL, $id = NULL, $imgobj = null) {

should be

function printHDImageThumb($hd = false, $alt = NULL, $class = NULL, $id = NULL, $imgobj = null) {

3_ Variable $option in function getOptionsSupported() is not defined
Line 456

    array(gettext('HD thumb quality') => array('key' => 'zp_picturefill_thumbquality', 'type' => OPTION_TYPE_TEXTBOX,```
should be

$option = array(gettext('HD thumb quality') => array('key' => 'zp_picturefill_thumbquality', 'type' => OPTION_TYPE_TEXTBOX,```

The fallback <img> should have widht and height attributes. Possibly the <source> as well (not sure if they can or should have right now, have to look at the docs).

Line 144-145
$html .= ''."\n".'
';
should be
$html .= ''."\n";

Line 162
return $quality
should be
return $quality;

Function getOptionsSupported() should be

    function getOptionsSupported() {
        $options = array(gettext('HD thumb quality') => array(
            'key' => 'zp_picturefill_thumbquality', 
            'type'   => OPTION_TYPE_TEXTBOX,
            'order'  => 2,
            'desc'   => gettext('It is recommended to set the HiDPI image to a lower compression as because its resolution it will not be as noticable and will have file size advantages. Default is 35 percent.')),
        gettext('HD image quality') => array(
            'key' => 'zp_picturefill_imagequality', 
            'type'   => OPTION_TYPE_TEXTBOX,
            'order'  => 2,
            'desc'   => gettext('It is recommended to set the HiDPI image to a lower compression as because its resolution it will not be as noticable and will have file size advantages. Default is 35 percent.'))
            );
        return $options;                                            
    }

$options = and ) (close array) (2x) are missing

Media players will not work for video files if zp_picturefill is enabled on image.php (Which is where I think you would want it to actually run half the time, that and on thumbnails.)

If the default-
printDefaultSizedImage(getImageTitle());

is changed to zp_picturefill-
printHDDefaultSizedImage(null, true);

Videos will not load. This might seem obvious, but it took me a few minutes to figure out, and there is no warning in the readme. I tested it with the default jplayer on the current build.