Images are 1st class citizens of TXP

Gocom · 2010-02-05 00:37:25

Bloke wrote:

Please let me know your thoughts on how TXPish this is, vs how useful it is, vs how easy it is to understand. First person to chart this in a meaningful way on a multi-axis graph wins a gold star :-)

Truth be told, IMO, it looks like good old smd-plugin ;) Don’t know if it really fits the core of Textpattern, but it is powerful. Good job Bloke.

r3310 […] how TXPish

From programming style perspective TXP’s core seems to favor: return; instead return ''; among other things. If we look source we can see what is written by someone known as Stef. But I don’t even know if you guys have somesort of rules about the style :D … Just kidding.

It’s really good that you use tabs only to indent lines and not as space between stuff :)

/* Gaselle like random jumps */
    'random' => 'indent',
	'is' => 'fun'
    'so'	=>		'spacing'
    'is'    =>	'too' /* insert random comment here (in German) or just word "nasty" */

Too much coffee :)

Last edited by Gocom (2010-02-05 00:58:09)

wet · 2010-02-05 05:17:11

Bloke wrote:

<txp:image_list has_thumb="1" /> : all images that have thumbnails assigned to them

I’d prefer to stick with established attribute names.

new attribute context (any better names for this attribute, please holler). By default, the tag is context sensitive, such that if you omit all other selection attributes it checks the article image field for ID(s) to use as your gallery. In future, this attribute may take values other than article (its default). For example, context="file, link, article" would tell the tag to first look for any file images

We have file images?

Bloke · 2010-02-05 09:26:52

Gocom wrote:

Truth be told, IMO, it looks like good old smd-plugin ;)

Dammit.

Don’t know if it really fits the core of Textpattern, but it is powerful. Good job Bloke.

OK, point taken. So how could it be changed to fit?

From programming style perspective TXP’s core seems to favor: return; instead return '';

OK. I’m not smart enough to know if it makes a difference — programmatically — which way it’s done, but I’ll change it if it’s consistent with other functions. I’ll look around.

If we look source we can see what is written by someone known as Stef.

I’ve tried to stick to the convention that went before me (e.g. braces on a new line, indentation, etc) but it takes some getting used to. I’ll try and be more “TXP” in future :-) Thanks.

wet wrote:

I’d prefer to stick with established attribute names.

Right you are. I’ve been thinking about the thumb attribute actually and I might ditch it, meaning there’ll be less confusion between having both a thumbnail and a thumb attribute. Currently, thumb is only used to say “I’d like to see thumbnails in my gallery” when not using a form/container, i.e. for the default output. That’s pretty thin given that 99% of use cases will be employing a form/container, so I think forcing a thumbnail to be displayed — like image_index does — is the right way to go. I’ll fix that later today, thanks.

We have file images?

Not yet, but I’m not ruling out the possibility. Lists of files with an associated thumbnail makes download lists prettier and, from what I can gather, quite a few people hack it or use a plugin to achieve this. Similarly, lists of links to resources could contain an optional thumbnail. Dunno, it was just a random thought that might never come to fruition. Didn’t want to have to rewrite this tag too heavily if it did ever make sense to include such a feature.

Bloke · 2010-02-05 09:52:29

Removed thumb attribute from txp:image_list
has_thumb is now thumbnail in txp:image_list
thumb attribute is now thumbnail in txp:image_url

r3311

Gocom · 2010-02-05 13:57:20

Bloke wrote:

I’ve tried to stick to the convention that went before me (e.g. braces on a new line, indentation, etc) but it takes some getting used to. I’ll try and be more “TXP” in future :-) Thanks.

Basically I’m saying that your cleaness pops up. Smudge it, darn it ;)

OK, point taken. So how could it be changed to fit?

Nothing much if you want to keep it — powerful. Steflikestyle.

Could context become type? Category tags use type.
~~Maybe it’s just my high droid rage against tags with long list of attributes.~~
~~Less attributes equals less reading?~~

jsoo · 2010-02-05 14:03:06

I like the way you’ve pulled this together from disparate opinions and ideas, and from your last revision above I don’t see anything that sticks out as anti-Txpish (questions of code style aside). Controllable context is a new idea for Txp but very clever and forward-looking.

I would expect demand for:

paging
preserving arbitrary sort order (e.g. id="4,2,3" returns the images in that order).

Bloke · 2010-02-05 14:52:12

jsoo wrote:

I like the way you’ve pulled this together from disparate opinions and ideas

Thanks. Couldn’t have done it without ya. It’s the “draw a line in the sand and let others blur it to completeness” methodology :-)

paging

Hmmm, yeah. As it stands we have $thispage which is article-centric. Since <txp:file_download_list> and <txp:linklist> are not pageable (well, without building it yourself using limit/offset) this probably needs to be considered as something that is applicable across all content types. I’ll take advice on this one over the best approach; drop me a line if you have any thoughts.

preserving arbitrary sort order (e.g. id="4,2,3" returns the images in that order)

Solving that in smd_gallery was a real bind. Of course, I’ve learnt a thing or two since I wrote that; namely that MySQL can sort by field, so about 100 lines of my original code can probably be written in one!

I shall certainly look into this one. Not really sure how to reconcile it against the fact you can ask for IDs and names and categories and authors and…

I guess we just need to check which options have been used and — in the absence of a sort attribute — either:

only allow the id attribute to imply an order
choose the best attribute based on some hierarchy (e.g. id > name > category > author/realname > ext)
see if multiple orders : ORDER BY FIELD(id, '42', '5', '9'), FIELD(name, 'lion.jpg', 'zebra.jpg'), FIELD... works!

Anyone with any more info or thoughts on this, feel free to educate me, thanks :-)

Last edited by Bloke (2010-02-05 14:58:08)

jsoo · 2010-02-05 15:55:39

Bloke wrote:

As it stands we have $thispage which is article-centric. Since <txp:file_download_list> and <txp:linklist> are not pageable (well, without building it yourself using limit/offset) this probably needs to be considered as something that is applicable across all content types. I’ll take advice on this one over the best approach; drop me a line if you have any thoughts.

You’re right, any solution needs to take into account the other content types. Offhand I think it makes more sense to stick with thispage and the pg query param as the only paging system, hence effectively limiting useful paging to one content type at a time. I don’t see that as an undesirable limitation. So my idea is that this would be intended for section front pages, just as it is with context-based article lists, and the assumption is you would not have both article and image_list on that page. In other words, a gallery page.

Which then leads to the question of displaying those gallery images individually. I’ve never really gotten the hang of image_index and image_display, but I think what I’m getting to is incorporating their features into image_list. That is, allow paged galleries with links to individual image pages as with image_index and image_display, but with the form/container-based output of image_list. So use the pg for paging and p for individual image display (or come up with something more intuitive than p, e.g. i for images, f for files, l for links).

Hence, if p is set, image_list returns only that image. Individual image context.

jsoo · 2010-02-05 16:07:50

Bloke wrote:

Solving that in smd_gallery was a real bind. Of course, I’ve learnt a thing or two since I wrote that; namely that MySQL can sort by field, so about 100 lines of my original code can probably be written in one!

FIELD() is MySQL-specific. With the talk of moving to PDO/Doctrine, we probably shouldn’t be adding MySQL-specific queries to core.

As is, you can use sort to get this, in a limited way:

id="4,2,3" sort="field(id,4,2,3)"

but not with the article-image field, where I see this as most useful. I guess it’s best to leave this as it is for now. If someone really wanted to sort by article-image field, a simple plugin (or smd_query) to get the raw article-image field is easy.

Bloke · 2010-03-19 17:01:18

Just a quick check. Has anybody found anything wrong with this suite of tags yet? Anyone tried them? Do they work as advertised? Are they logical? Do you get what you expect with various attribute configurations?

I’m still mulling over Rick’s patch that adds an attribute that allows you to override the ‘pooling’ ability of the id/name/category attributes. To recap: as it stands, if you specify a category AND an id AND a name attribute (or any combination thereof), the images in your image_list will become a pool of all of those images combined. If you specified any of the other filtering attributes as well (author, ext, realname, thumbnail) then images that did not match those additional filters would be removed from the pool to give you the final image list.

Rick’s patch would mean that this ‘pooling’ feature could essentially be turned off. Therefore if you specified a category AND an id AND a name (or any combination), only the images that matched the given IDs/names that were in the given category(ies) would be considered. The effect of the additional filtering parameters remains unchanged: they still remove images that don’t match.

An example set of images:

id	name	category	ext	author
3	tiger.jpg	wild-cats	.jpg	Bloke
4	lion.jpg	wild-cats	.jpg	jsoo
5	panther.jpg	wild-cats	.jpg	rsilletti
6	shark.jpg	sea-biters	.jpg	jsoo
7	piranha.jpg	sea-biters	.jpg	Bloke
8	chimp.jpg	mammal	.gif	Bloke
9	orang-utan.jpg	mammal	.jpg	rsilletti

Here are some result sets of a <txp:image_list> tag with and without the patch attribute set:

Attributes	Images without patch	Images with patch attribute set	Patch notes
`category="wild-cats" id="8" name="lion.jpg" />`	3, 4, 5, 8	None	image #8 is not in wild-cats and is not called lion.jpg
`category="wild-cats" id="4" name="lion.jpg" />`	3, 4, 5	4	image #4 is in wild-cats and is called lion.jpg
`category="wild-cats" id="8, 6" name="lion.jpg" author="jsoo" />`	4, 6	None	images #8 and #6 are not in wild-cats, nor are they named lion.jpg
`category="wild-cats" id="4, 6" name="lion.jpg" author="jsoo" />`	4, 6	4	image #4 is uploaded by jsoo, is in wild-cats and it’s called lion.jpg, but image #6 isn’t
`category="wild-cats, sea-biters" id="4, 6" name="lion.jpg" author="jsoo" />`	4, 6	4	image #4 is uploaded by jsoo, is in wild-cats, image #6 is in sea-biters but only image #4 is called lion.jpg

In short, if you employ the attribute that Rick is proposing your three attributes category, id and name must all match in order to display that image. If anybody can think of a practical example for this, please post it here; I’m struggling, but I’m only one man and therefore cannot pre-empt all possible implementations of this tag. I suppose for flexibility it should be there, but if it turns out to serve little practical purpose then it just adds confusion!

jsoo wrote:

I’ve never really gotten the hang of image_index and image_display, but I think what I’m getting to is incorporating their features into image_list.

Hehe, me neither! From what I remember (I’ll have to check) without a container/form, the image_list tag will output a series of linked images that, when clicked, can be displayed with <txp:image_display />. So it’s kind of halfway towards what you envision. Not sure how image_list would behave if it encountered the p URL var though. Does it change anything?

Further, I wonder if with a bit of cunning, the image_list tag could actually be coerced into fashioning a clickable image list inside an article, which would fix the forced-list feature of image_index / image_display. Hmmm, not sure offhand. Anybody have any ideas?

FIELD() is MySQL-specific

I didn’t know that, thanks; duly noted. For reference, <txp:section_list> and <txp:category_list> both use FIELD() so if an ORM creeps in, we’ll have to revisit those.

Last edited by Bloke (2010-03-19 17:07:03)

thebombsite · 2010-03-20 11:23:33

I have to say first Stef that I haven’t had time to get to grips with the new image tags yet. My current work load requires that I work with 4.2.0 as it is the currently available public version. I shall soon be re-doing thebombsite though so watch out. ;)

In the mean time, just looking at it from a purely logical perspective, I can’t see the point of Rick’s patch (sorry Rick). If an image has to match all 3 attribute values then surely you can get precisely the result you want simply by specifying one attribute such as the “id” or the “name”. The “category” attribute doesn’t really matter if you are narrowing the search with either an “id” or a “name”.

What I would say is that it will probably take me a few tries to get used to the way you have them working at the moment. Up until now specifying more than one search attribute with articles, such as a combination of “section” and “category” has narrowed the expected search results whereas with images, combining search attributes such as “category”, “id” and “name” will result in a widening of expected search results. Might just take a bit of getting used to.

Also Mary’s upm_image plugin allows article_image sorting according to the order they appear in the field so maybe have a look at it’s code??

Naturally I may have totally missed the point. ;)

Last edited by thebombsite (2010-03-20 11:29:31)

Bloke · 2010-03-20 13:39:14

thebombsite wrote:

If an image has to match all 3 attribute values then surely you can get precisely the result you want simply by specifying one attribute such as the “id” or the “name”.

That was my initial thought, but perhaps Rick or somebody else has a use case whereby specifying a category and a list of ids — perhaps both read from a custom field — might generate a useful list based on only images matching both. *shrug* perhaps I’m not thinking hard enough here!

it will probably take me a few tries to get used to the way you have them working at the moment.

Yes, that’s the non-TXPish behaviour creeping in and a strong argument for Rick’s patch: setting the — as yet unnamed — attribute to and would make the image_list tag attributes strictly match only those elements that meet all the criteria. One could argue that it should be this by default to maintain consistency with other tags, though I don’t know if it’s as useful and might result in people having to regularly specify or.

Then again, what are the use cases for being able to build up a pool of images and whittle them down with other attributes? Perhaps selecting a core set of images by id in the tag and then allowing some additional ones to be specified by category or name via a custom field? Perhaps then only allowing the images that belong to the author of the current article to show. Is that a stretch? Maybe the whole ‘image pool’ thing is a waste of effort vs the number of times people will use it, so it should just be strictly TXPish in nature all the time?

Anybody with any concrete use cases for either method, please speak up so we can figure out which way is best to go, thanks.

Also Mary’s upm_image plugin allows article_image sorting according to the order they appear in the field so maybe have a look at it’s code??

Good call, thanks. I’ll check it out and see how she’s approached it. Knowing Mary it’ll be damn clever :-)

Textpattern CMS

Textpattern CMS support forum

#49 2010-02-05 00:37:25

Re: Images are 1st class citizens of TXP

#50 2010-02-05 05:17:11

Re: Images are 1st class citizens of TXP

#51 2010-02-05 09:26:52

Re: Images are 1st class citizens of TXP

#52 2010-02-05 09:52:29

Re: Images are 1st class citizens of TXP

#53 2010-02-05 13:57:20

Re: Images are 1st class citizens of TXP

#54 2010-02-05 14:03:06

Re: Images are 1st class citizens of TXP

#55 2010-02-05 14:52:12

Re: Images are 1st class citizens of TXP

#56 2010-02-05 15:55:39

Re: Images are 1st class citizens of TXP

#57 2010-02-05 16:07:50

Re: Images are 1st class citizens of TXP

#58 2010-03-19 17:01:18

Re: Images are 1st class citizens of TXP

#59 2010-03-20 11:23:33

Re: Images are 1st class citizens of TXP

#60 2010-03-20 13:39:14

Re: Images are 1st class citizens of TXP

Board footer