Images are 1st class citizens of TXP

Bloke · 2010-01-27 21:28:55

maniqui wrote:

I don’t like the way that file_download_list works, returning all files when id attribute is there but it’s empty.

Yes, it’s not ideal. And you’ll get the same behaviour with image_list :-(

It’s partly because of the way the internal lAtts() TXP function works, that 99% of plugins use. By the time that function has done its stuff and extracted/checked/processed all attributes there is no way to tell (programmatically) if the user specified a value that was empty or the default empty value has been used. They are identical.

I have bypassed it (in fact in smd_gallery) by putting some code before the lAtts() function that checks to see if $atts['id'] is set and raising a flag if it is. Then, lAtts() can do its stuff and I still know that there was an attempt to do something with the id tag, and it ‘failed’.

It’s easy enough to stop the ‘all images’ behaviour. I only left it in because it mirrored file_download_list and I was going for consistency between tags wherever possible. I can remove that easily and — as long as it’s made clear that the image_list tag returns absolutely nothing if your given options match nothing — we’re in the clear. The same can’t be said of file_download_list unfortunately because it’s a non-backwards-compatible change. Can people live with the differences or do you think that will cause us problems with countless support threads on why they’re different?

As jsoo says, another “problem” is how to get a list of all images that aren’t assigned to any category. Currently you can’t do that but, to be fair, you can’t do it with file_download_list or linklist either. Again, I’ve solved it in the past with a trigger word — which is fine for a plugin because you have the plugin prefix to fall back on — but it makes me nervous to do that kind of thing in the core!

Last edited by Bloke (2010-01-27 21:30:51)

jsoo · 2010-01-27 21:47:36

Bloke wrote:

It’s partly because of the way the internal lAtts() TXP function works, that 99% of plugins use. By the time that function has done its stuff and extracted/checked/processed all attributes there is no way to tell (programmatically) if the user specified a value that was empty or the default empty value has been used. They are identical.

For some plugins I have set one or more attributes’ default value to null, so that a user-specified attribute="" means something different. But it would be confusing to introduce that kind of behavior in core, when so many other tags don’t work that way.

Bloke · 2010-01-27 22:11:40

I agree, jsoo. Nice trick with the null attribute value, btw. I’ll have to remember that one for future plugins.

I’ve just been testing the latest changes so this is now the defined behaviour for <txp:image_list>:

If id, category or name are used, they (cumulatively) determine the images in the gallery pool
If any of the above are not used (or they all happen to equate to empty) AND we’re in an article context, the article_image field is checked, like this:
- If article_image is empty it is ignored
- If article_image contains a URL or some non-numeric reference to an image, it is ignored
- If article_image contains one or more image IDs they will be added to the pool
If all the above result in a valid pool, it is used and passed to the Form/container for processing
If all the above result in no images being returned, nothing is output

Note that although the article_image field can contain a list of image IDs here, it cannot be used like this with the good old <txp:article_image /> tag. So you make your choice at the outset: to use article_image to render one image or use image_list to have the option of rendering one or more. It may be possible to retrofit article_image with the ability to understand a list of IDs, but it’s not high priority.

As a side benefit, when and if the file_image and link_image tags come into existence, image_list could easily (and automatically) take advantage of them to allow you to show a gallery of images associated with a file or link!

Does that sound logical to you all, or should I try harder? :-)

I’ve also had a sudden pang of guilt over image_size. Although it does seem to cover some of the same ground as image_info and the break attribute isn’t quite the same as other tags, image_size does (currently) do something different to <txp:image_info type="thumb_w | thumb_h" />. Namely, if the sizes are not defined in the database’s thumb_w and thumb_h fields (for whatever reason) the tag will go and check the actual thumbnail file and try to read its dimensions. Conversely, image_info looks in the database for the size information and if it’s not set it returns nothing.

This behaviour is perhaps superfluous now, since the reading of the file’s dimensions was a hangover from the days when thumb_w and thumb_h didn’t exist in the DB. So I may still axe the attributes or the tag entirely. If anyone has any preference either way, speak now.

I have, however, altered the default break to an x so something like 1024x768 is the default output of the tag.

Any comments on the above considered. Otherwise I’ll commit this change and wait for the next batch of fallout :-)

jsoo · 2010-01-27 22:44:37

Bloke wrote:

Note that although the article_image field can contain a list of image IDs here, it cannot be used like this with the good old <txp:article_image /> tag. So you make your choice at the outset: to use article_image to render one image or use image_list to have the option of rendering one or more. It may be possible to retrofit article_image with the ability to understand a list of IDs, but it’s not high priority.

~~@intval()@ will return the first ID in the list, so article_image will return the first article image even as is.~~ Edit: Oops, see below.

Last edited by jsoo (2010-01-29 13:41:56)

Bloke · 2010-01-29 13:27:40

jsoo wrote:

intval() will return the first ID in the list, so article_image will return the first article image even as is.

It makes sense that it does, but it doesn’t on my system. If I put 2, 3, 4 in my article_image field I get nothing from the <txp:article_image /> tag :-s That doesn’t mean it isn’t true on other systems of course. Perhaps article_image should be modified so it only picks up the 1st image in all cases? To avoid any unexpected behaviour that abusing the field (or variations in PHP version) might incur.

Anyway, I’ve relented a little in other matters. r3306 works like this:

<txp:image_list> now returns nothing if id/category/name and article_image field match nothing. The article image is of course ignored if it is empty or the tag is used in a list context
The article image field is only checked if id/name/category are all empty. This is probably logical given that specifically requesting certain images at the tag level would override any at the article level. For more complex cases where you want to cumulatively work with both an attribute-specified set and the article_image data, you’re best off with a plugin anyway
You may use id="*" to signify you want all images (thanks jsoo)
You can, in fact, get at the uncategorized images already like this: category="," (show all uncategorized images) or category="cat1, cat2," (show cat1, cat2, and all uncategorized images)
The <txp:image_size> tag has been removed completely, because everything it did can be obtained far more simply with a couple of <txp:image_info /> tags. Thanks to ruud / Gocom / jsoo / maniqui / et al for making me see the light

Last edited by Bloke (2010-01-29 13:32:15)

wet · 2010-01-29 13:34:10

Bloke wrote:

It’s partly because of the way the internal lAtts() TXP function works, that 99% of plugins use. By the time that function has done its stuff and extracted/checked/processed all attributes there is no way to tell (programmatically) if the user specified a value that was empty or the default empty value has been used. They are identical.

I beg to differ, as there’s both isset($atts['value']) and isset($value) at your disposal.

Bloke · 2010-01-29 13:38:12

wet wrote:

I beg to differ

Of course, you’re right. I forgot that it left $atts intact and just extracted the contents.

^{Note to self: shut up}

jsoo · 2010-01-29 13:41:31

Bloke wrote:

If I put 2, 3, 4 in my article_image field I get nothing from the <txp:article_image /> tag :-s

Ha — I’m full of it. article_image first does an is_numeric check, so you’re right.

the_ghost · 2010-01-29 18:12:39

You can, in fact, get at the uncategorized images already like this: category=”,” (show all uncategorized images) or category=“cat1, cat2,” (show cat1, cat2, and all uncategorized images)

Using category="," to output images without category looks very strange and unexpected… And what about category="*" to output images from all categories?

mrdale · 2010-01-29 19:14:54

…missed this one, stef. Been after this in core for literally years. You’re the dogs bollocks.

Last edited by mrdale (2010-01-29 19:18:36)

Bloke · 2010-01-29 21:52:43

the_ghost wrote:

Using category="," to output images without category looks very strange and unexpected

I agree. It’s just a way to do it — it’s not an officially supported mechanism and I didn’t expressly make it that way, I just noticed it by mistake as I was testing the feature. In fact, this trick will work in <txp:file_download_list> and <txp:linklist> too.

Next question then is what would make sense? Before I stumbled upon the cheat, I had started building support for a special option TXP_UNCATEGORIZED. That itself was a trick — all I made the code do was replace TXP_UNCATEGORIZED with an empty string, thus category="TXP_UNCATEGORIZED, cat1, cat2" internally became category=", cat1, cat2" (i.e. the same as if you’d omitted it yourself).

what about category="*" to output images from all categories?

Again, I considered this but it kinda goes against the meaning of ‘*’. An asterisk in the id attribute works because all images have an ID. But in a category (and name) the value itself may be empty. Thus some people would expect category="*" to output all cats (including unassigned ones) and other people might expect it to select all except uncategorized ones. The former case is redundant because id="*" covers all images. I’m not comfortable with the latter case unless someone can convince me its meaning won’t be confusing.

mrdale

Nice of you to stumble in from the cold. Glad I could finally do something that’s been on your wish list. Figured it was about time upm_image was essentially distilled and made a core feature because a) it’s a stock component of most people’s arsenal, b) the inconsistency between treatment of files, links and images annoyed the tits off me :-)

jsoo · 2010-01-30 19:56:50

Rick has an interesting point (and patch) on the txp-dev list about selecting images with multiple attributes in image_list, namely, that an AND (conjunctive) search should be an option as well as an OR (disjunctive) search.

I agree that one is unlikely to want to select images by, say, category="foo" has_thumb="1" with a disjunctive search. AND is much more useful for this combination.

Generally I would expect an OR search within an attribute, e.g. category="foo,bar" means category='foo' OR category='bar'. But across attributes, I generally expect an AND search. For example, category="foo" author="joe" means category='foo' AND author='joe'. It’s how doArticles() works. Getting even more complex, I would expect category="foo,bar" author="joe,jane" to give (category='foo' OR category='bar') AND (author='joe' OR author='jane').

id and name are an exception. It is pretty much pointless to be able to search for, say, id='2' AND name='foo'. But then I doubt I’d ever need to search by both id and name even disjunctively.

Tangentially, above I used author in an example. This would be a useful attribute for this tag. Also ext, search by file extension.

maniqui · 2010-01-30 20:12:59

jsoo wrote:

(…) I generally expect an AND search. For example, category=“foo” author=“joe” means category=‘foo’ AND author=‘joe’. It’s how doArticles() works. (…)

That’s the behaviour that I would expect by default too.

ruud · 2010-01-30 21:01:18

jsoo wrote:

For some plugins I have set one or more attributes’ default value to null, so that a user-specified attribute="" means something different. But it would be confusing to introduce that kind of behavior in core, when so many other tags don’t work that way.

It’s certainly less confusing than introducing attribute="*", because in most cases not specifying attribute in a list tag already means that attribute is not used to limit the resulting list. Using a default NULL value to distinguish from an empty value is better, I think, and it doesn’t introduce the asterisk as a special character.

rsilletti · 2010-01-31 01:48:22

Allowing for an AND search would make a choice like this workable:

Where assigned categories go this way, links in the sidebar, image_display tag in the content div:

image ids 1, 3 & 4 test_cat1
image ids 2, 8 & 9 test_cat2
image ids 5,6,7 & 8 test_cat3

<txp:category_list type="image">
<h3><txp:category /></h3>
<ul>
<txp:image_list category='<txp:category />' id="1,2,3,4,5,6,7,8,9" sort="id ASC" add_atts="1">
<li><a href="<txp:site_url />category/<txp:image_info type="category" />/?p=<txp:image_info type="id" />">
<txp:image_info type="caption" /></a></li>
</txp:image_list>
</ul>
</txp:category_list>


<txp:image_display />

Textpattern CMS

Textpattern CMS support forum

#31 2010-01-27 21:28:55

Re: Images are 1st class citizens of TXP

#32 2010-01-27 21:47:36

Re: Images are 1st class citizens of TXP

#33 2010-01-27 22:11:40

Re: Images are 1st class citizens of TXP

#34 2010-01-27 22:44:37

Re: Images are 1st class citizens of TXP

#35 2010-01-29 13:27:40

Re: Images are 1st class citizens of TXP

#36 2010-01-29 13:34:10

Re: Images are 1st class citizens of TXP

#37 2010-01-29 13:38:12

Re: Images are 1st class citizens of TXP

#38 2010-01-29 13:41:31

Re: Images are 1st class citizens of TXP

#39 2010-01-29 18:12:39

Re: Images are 1st class citizens of TXP

#40 2010-01-29 19:14:54

Re: Images are 1st class citizens of TXP

#41 2010-01-29 21:52:43

Re: Images are 1st class citizens of TXP

#42 2010-01-30 19:56:50

Re: Images are 1st class citizens of TXP

#43 2010-01-30 20:12:59

Re: Images are 1st class citizens of TXP

#44 2010-01-30 21:01:18

Re: Images are 1st class citizens of TXP

#45 2010-01-31 01:48:22

Re: Images are 1st class citizens of TXP

Board footer