Images are 1st class citizens of TXP

jsoo · 2010-09-29 14:58:36

Bloke wrote:

or you simply use article_image to always grab the first image and ignore the rest, which is what I think you meant with the intval comment ,yes? Question: is that true ‘support’ for multiple article images or a kludge to allow image_list and article_image to co-exist?

Yes, that’s what I meant. Fair question whether or not this doesn’t just add confusion.

I have a different idea for solving this. Basic idea is that article_image simply returns image_list, giving image_list a $thing containing a single image or thumbnail tag with appropriate attributes. If you think that has merit I’ll have a try at coding it.

Edit: I haven’t really decided about the relative merits of having article_image simply return the first image v. behaving like image_list. I’m fine with having it just return the first image. If you are too, then there’s no problem.

Last edited by jsoo (2010-09-29 15:00:36)

Bloke · 2010-09-29 15:17:14

jsoo wrote:

article_image simply returns image_list, giving image_list a $thing containing a single image or thumbnail tag with appropriate attributes

Sounds like it might be a winner. Can’t foresee any immediate downside to this. Does that mean article_image gets container ability by proxy and access to $thisimage? Or maintain the simple Single tag approach? If you fancy having a stab, please feel free.

I’m fine with having it just return the first image

Me too. That can be the one-line fallback strategy in case we run into unexpected hurdles with other approaches :-) I guess as long as the docs are clear that article_image only returns the first image but image_list can use them all (order notwithstanding), we can get away with it.

Thinking about it, this approach could actually work to our advantage. A common use case is to employ a picture to link to your individual gallery pages. So you can stick the list of IDs in the article image field and whichever ID you put first in the list can be the image you pimp on your landing page via:

<txp:permlink><txp:article_image /></txp:permlink>

to take you to the relevant gallery article. Howzat!

Last edited by Bloke (2010-09-29 15:17:44)

jsoo · 2010-09-29 15:39:25

All in all, conceptually I think it best to keep article_image a tag that returns a single image. That is what its name implies, and how the closely related image and thumbnail tags work.

If returning the first of a list of images is confusing or otherwise wrong, we could add an error check; if article_image encounters a list, it gives some kind of “out of context” message. Not perfect, because of the possibility of commas in a URL, but maybe OK.

By the way, what does image_list do with a URL in the article image field? I think it just ignores it?

Bloke · 2010-09-29 15:52:18

jsoo wrote:

I think it best to keep article_image a tag that returns a single image.

Aye aye, cap’n!

if article_image encounters a list, it gives some kind of “out of context” message

Hmm, like you say, difficult to get right. And means you can’t mix ‘n’ match txp:article_image and txp:image_list (ahem, just like now).

I’m definitely coming round to your duality idea of just using the first ID for the txp:article_image tag. It retains nice backwards compatibility for those that don’t use image_list, and the people that start to use it can either make galleries using other attributes or put ID lists in their article image fields without jeopardizing the existing txp:article_image functionality.

By the way, what does image_list do with a URL in the article image field? I think it just ignores it?

Yup. if (is_numeric($items[0])) ... else, skip it

Last edited by Bloke (2010-09-29 15:53:21)

jsoo · 2010-09-29 16:17:14

OK, I think having image_list ignore a non-numeric article-image value is a step backward, in that it changes an expected behavior. Pre 4.3.0, officially speaking the article-image field should contain either an id# or a URL, and article_image uses a simple if_numeric check to decide which it is. I think image_list needs to function as an extended version of article_image when used in that context. Hence it needs to handle the URL option, even if as stupidly as article_image currently does.

Edit: One way to handle this: image_list does do_list then checks that every item in the list is an integer. If not, it must be a URL.

Last edited by jsoo (2010-09-29 16:18:39)

hakjoon · 2010-09-29 22:21:36

jsoo wrote:
Edit: One way to handle this: image_list does do_list then checks that every item in the list is an integer. If not, it must be a URL.

That’s what hak_article_image does fwiw. Another way to allow article_image to be backwards compatible and handle lists of article images would be to have a limit parameter default to 1.

Last edited by hakjoon (2010-09-29 22:21:47)

Bloke · 2010-09-29 22:40:52

jsoo wrote:

having image_list ignore a non-numeric article-image value is a step backward

OK, makes sense. Pure laziness on my part.

Unfortunately I’ll probably not get a chance to look at this over the next few days — in fact, probably not till early next week — so if anybody else would like to have a go at this and get it soak tested, someone can commit the fix asap (maybe me, if it can wait until after the weekend). Thanks in advance.

jsoo · 2010-09-30 02:05:32

Patch submitted to txp-dev list. Functional changes are:

If the article image field contains a list entirely of numbers, article_image will treat the first item as an image ID and attempt to retrieve and display it;

If the article image field contains any non-numeric items, image_list will assume it to be a URL and, unless $thing or $form are set, will spit out a raw img tag for it (applying wraptag if appropriate).

hidalgo · 2010-09-30 08:59:34

Limiting <txp:article_image /> to the first ID is a great idea. I kept running into this last night when trying to ditch all image plugins in favour of the new image tags.

Now that <txp:image_list /> can use multiple <txp:article_image /> IDs, I’m wondering about the merits of adding a type attribute to <txp:if_article_image /> to check if multiple IDs are used? e.g. type="id, list or url"

However, If I understand what I’ve read in the last few posts correctly, the sort order of multiple article image IDs currently isn’t maintained when displayed with <txp:image_list />? I guess this makes the above suggestion less useful.

jsoo · 2010-09-30 10:04:15

hidalgo wrote:

Now that <txp:image_list /> can use multiple <txp:article_image /> IDs, I’m wondering about the merits of adding a type attribute to <txp:if_article_image /> to check if multiple IDs are used? e.g. type="id, list or url"

However, If I understand what I’ve read in the last few posts correctly, the sort order of multiple article image IDs currently isn’t maintained when displayed with <txp:image_list />? I guess this makes the above suggestion less useful.

I think the idea is useful, though I think it’s an issue better solved in image_list. In my patch I chose to have image_list output nothing if the article image appears to be a URL, and the image_list tag has a form attribute or is a container tag. I thought it didn’t make sense to override the form or tag contents. But the more I think about it, the less I like this decision. I don’t think people will tend to use image_list’s default output; they will use a form or make it a container tag. If article-image as URL is to be a supported feature, I guess image_list needs to display it somehow, in all cases.

At the same time, I am uncomfortable with giving image_list a hard-coded default format (and it had this even before my patch). It’s one thing for, say, section_list to do this. But I think image_list should behave like article in this regard. There should be an ‘image’ form type, and there should be an editable default image form just as there is an editable default article form. (I would also like to see something similar for section_list, etc., but I digress.)

Anyway, back to your point, it would be nice for image_list to have a listform equivalent. I think that is at least part of what you’re after?

hidalgo · 2010-09-30 11:28:54

The idea was basically to add functionality equivalent to <txp:upm_if_article_image_list />, which determines whether multiple article images have been defined. Instead of creating a new tag (e.g. <txp:if_multiple_article_images />) for this functionality I thought it could be incorporated into the existing <txp:if_article_image /> tag with a type attribute or such.

As a quick example, it could then be used to create more advanced gallery setups like:

<txp:if_article_image type="multiple_IDs">
  <txp:image_list limit="1" offset="1">
    <a href="<txp:image_url />"><txp:article_image /></a>
  </txp:image_list>
  ...and more
<txp:else />
  <txp:article_image />
</txp:if_article_image>

The example wraps a link to the second image around the first/default article image, if multiple IDs are defined. These type of things can be done using custom fields, but having all the IDs in the article image field has always made more sense to me and so I was glad to see <txp:image_list /> finally utilising these.

In regards to the url type, I only included this isn the example when I realised that it was a valid type. I’ve never actually used a url in the article image field and didn’t know it was possible until reading the last few posts.

Last edited by hidalgo (2010-09-30 11:34:42)

jsoo · 2010-09-30 11:57:06

Right, I’m saying you could get the same thing more easily by adding listform to image_list. You’d have the option for different forms depending on whether there is one image or more than one. And this wouldn’t be restricted to article images.

Textpattern CMS

Textpattern CMS support forum

#109 2010-09-29 14:58:36

Re: Images are 1st class citizens of TXP

#110 2010-09-29 15:17:14

Re: Images are 1st class citizens of TXP

#111 2010-09-29 15:39:25

Re: Images are 1st class citizens of TXP

#112 2010-09-29 15:52:18

Re: Images are 1st class citizens of TXP

#113 2010-09-29 16:17:14

Re: Images are 1st class citizens of TXP

#114 2010-09-29 22:21:36

Re: Images are 1st class citizens of TXP

#115 2010-09-29 22:40:52

Re: Images are 1st class citizens of TXP

#116 2010-09-30 02:05:32

Re: Images are 1st class citizens of TXP

#117 2010-09-30 08:59:34

Re: Images are 1st class citizens of TXP

#118 2010-09-30 10:04:15

Re: Images are 1st class citizens of TXP

#119 2010-09-30 11:28:54

Re: Images are 1st class citizens of TXP

#120 2010-09-30 11:57:06

Re: Images are 1st class citizens of TXP

Board footer