Images are 1st class citizens of TXP

Bloke · 2010-01-27 16:36:45

Gocom wrote:

It does? You are saing that your code doesn’t use globals? Shame.

Well, if the image has been seen before it should be cached so it might only be 1 DB call. But I think at the moment images are only cached if the txp:image tag is called. I’ll see how easy it is to cache the results in image_list too, thanks for waking me up!

Gocom · 2010-01-27 16:40:47

Bloke wrote:

thanks for waking me up!

There are only few special Stefs that can do TXP dev’s work while sleeping ;) It’s advantage.

Last edited by Gocom (2010-01-27 16:41:05)

MattD · 2010-01-27 17:11:53

I was thinking article_image supported comma seperated list of article IDs but maybe I’ve been using smd_gallery to long.

jsoo · 2010-01-27 17:33:24

MattD wrote:

I was thinking article_image supported comma seperated list of article IDs but maybe I’ve been using smd_gallery to long.

From article_image():

$rs = safe_row('*', 'txp_image', 'id = '.intval($image));

$image being the contents of the article-image field.

Yep, too much smd_koolaid for you.

I think all the image plugins with gallery functionality support comma-separated article-image field. Rather suggests this is an in-demand feature.

Bloke · 2010-01-27 17:35:46

MattD wrote:

I was thinking article_image supported comma seperated list of article IDs but maybe I’ve been using smd_gallery to long.

:-) I’d like that as well. Maybe one day it can be added. Don’t think it’ll kill any existing sites because they’d just continue to output one image.

But I’m currently pondering over maniqui’s thoughts on being able to output the article_image information in a raw format somehow. Can’t think of a clean way to do it at the moment. Any and all good ideas from anyone considered!

Bloke · 2010-01-27 17:40:19

jsoo wrote:

$rs = safe_row(‘*’, ‘txp_image’, ‘id = ‘.intval($image));

Yes, so changing that to a MySQL IN list would work nicely. We’d need a break attribute to be able to put something between each image, but I think that’d do it. Trouble is, doing it that way would mean there’ll be a lot of code duplication if file_image and link_image come to fruition. Mind you, there’ll be code duplication whichever way we go, so I suppose it’s not a massive problem. Hmmmmm.

jsoo · 2010-01-27 18:48:17

Bloke wrote:

But I’m currently pondering over maniqui’s thoughts on being able to output the article_image information in a raw format somehow.

But that isn’t really what he’s asking for, is it? It’s only a means to getting at other fields for that image.

In my own image plugin, various tags default to the article image field if no id, category, or whatever is specified. The equivalent here would be to have image_list behave this way: in the absence of id, name, and category, grab the article image(s).

maniqui · 2010-01-27 19:39:34

jsoo wrote:

But that isn’t really what he’s asking for, is it? It’s only a means to getting at other fields for that image.

As Bloke stated, I’m asking for a way to use the value(s) inside article_image field as attribute’s value on image_list. Something like <txp:image_list id='<txp:article_image raw="1" />' /> (raw is an invented attribute, similar to show_me_the_damn_id="1" that Bloke suggested on a previous post).

In my own image plugin, various tags default to the article image field if no id, category, or whatever is specified. The equivalent here would be to have image_list behave this way: in the absence of id, name, and category, grab the article image(s).

That would be useful, particularly if, as in other TXP tags, it works inside an article_custom loop.

maniqui · 2010-01-27 19:41:11

BTW, an alias for txp:image_* so it could be written as txp:img_* would be appreciated (at least, by me) :)

Bloke · 2010-01-27 20:31:07

I (stupidly) wrote:

I’ll see how easy it is to cache the results in image_list

Duh. It’s using the new global $thisimage so I was utterly wrong with the number of DB calls.

In which case, there’s no penalty for having two tag calls to do the job of <txp:image_size />, it’s just more typing. Would anyone miss the break, type and as_html attributes from <txp:image_size /> so all it did was to output width="X" height="Y" — of either the main image or the thumb="1"? If you want to output the width and height you would have to use two <txp:image_info /> tags instead. Or should I just drop the tag altogether since it’s only of marginal benefit?

jsoo wrote:

in the absence of id, name, and category, grab the article image(s).

That makes a lot of sense. At the moment, without any id/category/name it uses every image (1=1). What I could do is add a step such that if nothing was chosen, it would first check if the tag was being used in an article context and if so try the article image field. Perhaps if that failed, it would then use all images. This would also lend itself nicely to being able to check if the image_list is being used inside a file or link context and therefore be able to automatically look in the (prospective) file_image or link_image fields if nothing has been chosen manually.

Question though: is this confusing behaviour? If you had a <txp:image_list> tag in an article form and you viewed article A (which had some values in its article image) you’d see a nice gallery. In Article B however, if there are no IDs in the article_image field (and still no category/id/name used) you’d see a gallery of all images in the database. Undesirable? I’d say yes.

A workaround would be to not default to ‘all images’. But then, at the other end of the scale, if you use a bare <txp:image_list> tag you’d surely expect something to be displayed if you’ve uploaded images, rather than nothing? In the same way that file_download_list will list all files if you don’t narrow it down. So I wonder if there should be either:

a) an ‘opt-in’ facility where you can say “yep, use the context if it’s there” and then it’s on your head if you have an empty article_image field

b) an ‘opt-out’ facility to say “if the tag matches no images, leave it with nothing and don’t default to all images”

Which is better? I think I favour method (a) but I’m willing to be swayed. Since you’re all on a roll and my brain is flagging courtesy of the 4am bed time last night after getting this patch out the door, any cool ideas on how this should be implemented? Perhaps some suggestions for a suitably-named attribute, or some very clever default values for the existing attributes?

maniqui wrote:

an alias for txp:image_* so it could be written as txp:img_* would be appreciated

It’s a lot of code for saving two measly characters. I’d say probably not going to happen, sorry.

Last edited by Bloke (2010-01-27 20:36:15)

jsoo · 2010-01-27 20:51:29

Bloke wrote:

Question though: is this confusing behaviour? If you had a <txp:image_list> tag in an article form and you viewed article A (which had some values in its article image) you’d see a nice gallery. In Article B however, if there are no IDs in the article_image field (and still no category/id/name used) you’d see a gallery of all images in the database. Undesirable? I’d say yes.

Easily avoided using if_article_image, so no need to add another attribute.

Bloke · 2010-01-27 20:53:43

jsoo wrote:

Easily avoided using if_article_image, so no need to add another attribute.

*sigh* told you my brain was mashed ;-) Thanks for the nudge.

maniqui · 2010-01-27 21:03:40

Bloke wrote:

Question though: is this confusing behaviour? If you had a <txp:image_list> tag in an article form and you viewed article A (which had some values in its article image) you’d see a nice gallery. In Article B however, if there are no IDs in the article_image field (and still no category/id/name used) you’d see a gallery of all images in the database. Undesirable? I’d say yes.

I would say yes too.
And I’m not sure about image_list returing all images if no attribute is specified. I wonder if there is a real case use for that. I would much prefer some kind of trigger value on category attribute for those situations, something like category="ALL" (the problem is to find this trigger word to avoid issues with any existing category already named “all”).

More: now that Bloke mentioned file_download_list, I would like to say that I don’t like the way that file_download_list works, returning all files when id attribute is there but it’s empty.
In code: <txp:file_download_list id="" /> returns all files.
So, if you have: <txp:file_download_list id='<txp:custom_field name="files" />' /> and your custom field is empty, it outputs a list of all files. Undesirable, as Bloke commented regarding image_list.

The workaround is:

<txp:if_custom_field name="files">
<txp:file_download_list id='<txp:custom_field name="files" />' />
</txp:if_custom_field>

Similarly,

<txp:if_article_image>
<txp:image_list id='<txp:article_image give_me_the_raw_thing="1" />' />
</txp:if_article_image>

could be an option.

But, imho, <txp:image_list id="" /> (and <txp:file_download_list id="" /> should return absolutely nothing.

To summarize: one thing is having the behaviour of <txp:image_list /> returning all images (may be desirable, although I don’t believe there is a real case use, same with files) but the othe thing is having <txp:image_list id="" /> returning all images, which I find totally undesirable.

A workaround would be to not default to ‘all images’. But then, at the other end of the scale, if you use a bare <txp:image_list> tag you’d surely expect something to be displayed if you’ve uploaded images, rather than nothing?

I think I prefer it to return nothing at all. Explicit is better than implicit in this case. And a trigger word on category attribute or new attribute to display all images may be the option to have all website images listed.
—
Hehehe, don’t worry about the txp:img_* request. My RSI won’t miraculously heal by avoiding typing just two characters.

jsoo · 2010-01-27 21:09:46

maniqui wrote:

I would much prefer some kind of trigger value on category attribute for those situations, something like category="ALL" (the problem is to find this trigger word to avoid issues with any existing category already named “all”).

<txp:image_list category='<txp:category_list type="image" break=","><txp:category /></txp:category_list>' />

jsoo · 2010-01-27 21:14:16

Although that doesn’t get images without an assigned category.

But I agree with Julián that defaulting to every image in the DB is strange behavior. If this needs to be an option I’d prefer something like <txp:image_list id="*" />.

Last edited by jsoo (2010-01-27 21:16:35)

Textpattern CMS

Textpattern CMS support forum

#16 2010-01-27 16:36:45

Re: Images are 1st class citizens of TXP

#17 2010-01-27 16:40:47

Re: Images are 1st class citizens of TXP

#18 2010-01-27 17:11:53

Re: Images are 1st class citizens of TXP

#19 2010-01-27 17:33:24

Re: Images are 1st class citizens of TXP

#20 2010-01-27 17:35:46

Re: Images are 1st class citizens of TXP

#21 2010-01-27 17:40:19

Re: Images are 1st class citizens of TXP

#22 2010-01-27 18:48:17

Re: Images are 1st class citizens of TXP

#23 2010-01-27 19:39:34

Re: Images are 1st class citizens of TXP

#24 2010-01-27 19:41:11

Re: Images are 1st class citizens of TXP

#25 2010-01-27 20:31:07

Re: Images are 1st class citizens of TXP

#26 2010-01-27 20:51:29

Re: Images are 1st class citizens of TXP

#27 2010-01-27 20:53:43

Re: Images are 1st class citizens of TXP

#28 2010-01-27 21:03:40

Re: Images are 1st class citizens of TXP

#29 2010-01-27 21:09:46

Re: Images are 1st class citizens of TXP

#30 2010-01-27 21:14:16

Re: Images are 1st class citizens of TXP

Board footer