Conversation
…ta(), image_url(), and image_date(). Added error checking and caching. NB: this changes the behavior of imageFetchInfo().
… changes behavior if user has specified both id and name, but there's no reason to use the tag that way.)
…), respectively, for output.
|
commit c9cbf1c does give slightly different output: no hard-coded style attribute for image_display(), no thumbnail height and width attributes for image_index(). |
|
That probably wraps up this batch of commits. It would be possible to eliminate some duplicated code by adding the functionality of thumbnail() to image() and then having the former call the latter, but that would add complexity to image() and hardly seems worth it. |
| } | ||
|
|
||
| if (!$from_form) { | ||
| $thisimage = ''; |
There was a problem hiding this comment.
This is great stuff, Jeff, thanks.
Do you happen to know what the point of this conditional is? I always assumed it was in case you used the tag in a loop scenario, that it resets $thisimage so it's not left populateed after the tag completes, therefore leading conditionals such as <txp:if_thumbnail> to return incorrect information if they were used outside image context at a later point on the page. Or, maybe, it's to make sure that the next image in a <txp:images> loop is "clean". I'm not entirely sure. Either way, it always struk me as odd that $thisimage is an array but it's reinitialised to an empty string instead of an empty array.
Do you think it's important enough to keep it in these tags? Or does your refactorisation negate the need for it?
There was a problem hiding this comment.
What it was, I reckon, was carelessness: mucking with a global instead of creating a local copy. Obviously images() needs to do this to pass data to the other image tags it contains. Equally obviously a tag such as image_info(), which cannot be used as a container, has no business changing the value of $thisimage.
And I'm really glad you brought this up, because image_url() can be used as a container. Maybe there's a valid use case for having it temporarily change $thisimage. For example, within an images() tag, image_url() looks for an image with name matching $thisimage['author']. And then the image_url() tag, also a container, wants the caption of that image of the author. A bit convoluted, perhaps, but not unreasonable. I'll add a commit to restore this behavior while we think it over.
Edit: well, not completely restore, because the 4.6.2 version of image_url() doesn't bother checking whether $thisimage was already populated before it nukes it.
… to include logic duplicated across image_data(), image_url(), and image_date(). Added error checking and caching. (NB: this changes the behavior of imageFetchInfo(). The 3 tags above are the only Txp functions using imageFetchInfo(), which has only been around since 4.6.0.)