Bloke / smd_thumbnail

Multiple image thumbnails of arbitrary dimensions
GNU General Public License v2.0
8 stars 4 forks source link

Support for wraptag attribute and error handling #26

Closed jools-r closed 5 years ago

jools-r commented 5 years ago

The ability to use <+> as a placeholder in the wraptag attribute makes it easier to construct tags without the need for if_exist type tags if you just want to prepend/append a tag. Currently it seems smd_thumbnail doesn't support wraptag, so I wrap it with txp:evaluate to 'retrofit' it with that functionality.

For some reason, txp:evaluate (even with the addition of the attribute test="smd_thumbnail") doesn't seem to catch no output, so to make a failsafe tag, I need to do the following

 <txp:smd_if_thumbnail type="xl"><txp:evaluate wraptag="<+> 1800w,"><txp:smd_thumbnail type="xl" display="url" /></txp:evaluate></txp:smd_if_thumbnail>

for each smd_thumbnail profile size within a regular txp:images-loop.

It would be great if I could do just:

 <txp:smd_thumbnail type="xl" display="url" wraptag="<+> 1800w," />

and if no image exists for that profile, then nothing is output. In this example I've hardcoded the width, but one could plug the smd_thumbnail's profile width in there as a tag-in-tag too.

Does that sound doable?

Bloke commented 5 years ago

This might be because I handle wraptag explicitly in the plugin. If the tag is removed from the lAtts() array of the smd_thumbnail tag, it should default to the global attribute and permit you to perform what you want.

However, I'm not sure then what will happen with the id and class attributes that I test for $wraptag before handling them. Also, the final return line will probably need altering.

If removing the attribute from the plugin isn't viable due to the above usage, I'm not entirely sure the best way to "add" support for the global processing of <+> to the plugin. Perhaps @bloatware can suggest something?

bloatware commented 5 years ago

The way smd_thumbnail handles wraptag attribute (via doTag() function) should work fine with <+> placeholder. FWIW, its processing is integrated into tag() function called by doTag(), dunno what could be wrong here.

jools-r commented 5 years ago

I've investigated some more and think I've been able to narrow it down some more :-)

Using smd_thumbnail with display="url" seems to suppress the wraptag. Removing the attribute display="url" produces the whole img tag with the wraptag working correctly. So it would seem outputting just the url bypasses the wraptag. It would be great if wraptag could also be applied that output. Maybe that's easier to solve?

bloatware commented 5 years ago

It's mainly a bwc question imo.

jools-r commented 5 years ago

For the moment I've forked smd_thumbnail and made two commits, one adding wraptag support when using display="url" (0d9448c6f72297acc260d436bca25eb1e70df7b8) and a second to add a quiet="1" attribute (60cb3c71e90b1b6a6399cccab6fe3cd22aa98a39) to optionally silence the unknown image error message. The default setting is '0', i.e. error reporting as before.

I can then do something like:

…
<txp:evaluate test="smd_thumbnail">
    srcset="<txp:smd_thumbnail type="t"  display="url" quiet="1" wraptag="<+> 350w," />
            <txp:smd_thumbnail type="s"  display="url" quiet="1" wraptag="<+> 600w," />
            <txp:smd_thumbnail type="m"  display="url" quiet="1" wraptag="<+> 900w," />
            <txp:smd_thumbnail type="l"  display="url" quiet="1" wraptag="<+> 1200w," />
            <txp:smd_thumbnail type="xl" display="url" quiet="1" wraptag="<+> 1800w," />"
    sizes="<txp:variable name="img_sizes_attribute" />"
</txp:evaluate>

… which is much more succinct than in the first post in this issue.

It's not hugely vital but makes for much cleaner code. In the site I'm currently working on, some images don't yet have any thumbnails while others do. Those without get a regular <img src="…"> tag while those with get the whole srcset and sizes caboodle.

If you think the additions make sense, feel free to pull them over from my fork.

--

NB: I've yet to find a really good way of handling the sizes attribute, because it's so layout dependent. For this particular site, I've got five main image cases – full, large, small, half, grid – and have a self-made txp:php snippet that populates the img_sizes_attribute variable depending on the respective image's class name (a simple switch, case setup). It's not terribly elegant but it works.