I want to change the order of the images that are attached to a node using image attach.

On the content type page it says:

"Since you installed CCK module, you can change the image weight in the Manage fields page."

I find this instruction very unclear.

Am I supposed to create a new CCK field, if so what? If this is not the case what am I missing?

I've looked through the documentation and also find it confusing.

Documentation: "The image is displayed as a thumbnail, linked to the image node, in the top RH corner of the node by default. This can be controlled to a minor degree by changing the "Attached image ... weight" on the content type's admin page (see above), and to a more major degree using theming and custom CSS."

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

> "Since you installed CCK module, you can change the image weight in the Manage fields page."

What it says. Go to the Manage fields page. Drag & drop things.

The documentation is out of date, it seems. You probably have the access rights to edit that page; if so, please consider updating and expanding it :)

AnnanFay’s picture

All I can manage to do on the CCK page is change the weight of the "Attached images".

However my understanding was that it was possible to change the image weight?

That is, change the position of each image in relation to the other images.

Using the CCK field page it doesn't seem as though it changes any of the image weights?

In clarification, I want to change the weight of the images not the weight of the 'Attached images' field.

Is this possible?

joachim’s picture

> All I can manage to do on the CCK page is change the weight of the "Attached images".

Yup.

> That is, change the position of each image in relation to the other images.

Ah. That's not possible. This is a big limitation of the current UI, but when this was extended to allow multiple attached images there wasn't time to also adapt the UI to allow reordering. The data storage is there, so if you're desperate to order attached images on a particular node, you can hack the delta column in the database :/

Enno’s picture

Hello,

i ran into the issue as well trying to have a custom order for attached images on the node. One shortcut that would work for me is to name the image nodes accordingly as image_attach seems to give them their weight in table "image_attach" based on the image node titles.
So, if I have a content type "car" and I attach various images to it, which are respectively called "Image Car 1", "Image Car 2", etc. they will show up in the correct alphabetical order on the node page.

That is obviously only useful on single-user sites where one person is responsible for adding and attaching images to a custom content.

On a side note, if you use that procedure, you will find that your first image in a node is not the image chosen by the view if it has "attached images" as an output field. The view will chose rather the last image (in alphabetical order). You can easily have that changed by altering a single line in file "image_attach_views_handler_field_attached_images.inc"
In the function "pre_render" you could change

$result = db_query("SELECT nid, iid FROM {image_attach} WHERE nid IN (" . $nids_string . ") ORDER BY weight");

to

$result = db_query("SELECT nid, iid FROM {image_attach} WHERE nid IN (" . $nids_string . ") ORDER BY weight DESC");

Not sure if this is worth a separate issue and a patch. I would appreciate Joachim´s input there.

joachim’s picture

Hm I notice there's no theme function for the output of that views field. That's a bit shoddy :/ You should file a bug for that, but I can't say when I'll be able to look at it, so feel free to have a go and see if the theme function for attached images on nodes can be used there, if that's within your capabilities :)

AnnanFay’s picture

I managed to hack the code in order to get image order working. Though it's not really that nice.

I noticed that the ImageField module has a nice interface for ordering, uploading and removing images. Maybe it can be borrowed.

joachim’s picture

Title: How do I change order of images ? » UI to change order of attached images
Category: support » feature

Given image D6's expected lifespan, an ordering UI is not something I can commit time to.

However, if you or anyone else want to work on it, I'll review patches :)

BTW:

> Hm I notice there's no theme function for the output of that views field. That's a bit shoddy

This is now fixed: #412288: restructure theme_image_attach_body/teaser

uberEllis’s picture

*subscribe*

sun’s picture

Status: Active » Closed (duplicate)

In light of #7, I think that #698284: Reverse attached image thumbnails so they are in the correct order left to right is all we can and want to do for D6.

mja’s picture

Joachim - be interested in your thoughts on the attached - a first stab at patching in a drag-to-reorder list for attached images. Quite a few functions had to change, so really would like your eyes on it, but it's working on my system at the moment, and will save the order even if you use the node edit submit button, as opposed to just the 'upload' custom submit on the image attach form.

Could do with some delete functionality built into the table, but wanted to check if you thought it was heading in the right direction first...

joachim’s picture

Status: Closed (duplicate) » Active

Dreditor is broken and it's late so this is only a quick patch review...

Looks promising!
Though needs some cleaning up...

+        // A database query is needed in order to display the attached images according to their true weights (otherwise they are simply displayed in the order they are 'node-loaded' from the database  
+	     $result = db_query("SELECT iid, weight FROM {image_attach} WHERE nid = %d ORDER BY weight", $node->nid);
+	      while ($data = db_fetch_object($result)) {
+		     // Store all the attached images' weights keyed by attaching iid.
+		     $attached_images_weights[$data->iid] = $data->weight;

... then the ordering should go into the node-loading part, rather than run the same query again.

Wathc indentation too btw.

I don't recall the structure of $node->iids, but see if you can fit the weights into there, if they are needed. I suspect they're not though -- you just need the order. When you save, you start at 0 again and save sequential weights.

mja’s picture

Hey Joachim,

Thanks so much for taking a look. Will def sort out indents and tidy up hopefully later on today.

Just wanted to be clear about the query you highlighted - basically, the only point of it is to pull the iids out of the database ordered by weight, as opposed to the order in which they appear attached to the node (usually ordered by iid - ie. not useful). After that, the new associative array of iids => weights is used to power the foreach that builds the form, rather than just foreach-ing the unordered node->iids object.

So just wanted to be clear, when you say 'avoid running the same query again', do you mean:
(a) that instead of building a separate array first, I should just build the form by foreach-ing the output of db_fetch_object($result)?
or (b) that I should left join a few tables and get the image thumbnail filename straight from db, rather than use node_load?

Sorry - just wanted to be clear before I dive back into this again.

joachim’s picture

What I mean is that the form_alter function shouldn't be running any queries at all, ideally.
It's the job of hook_nodeapi() 'load' op to load what we're going to need:

    case 'load':
      $res = db_query("SELECT iid FROM {image_attach} WHERE nid = %d ORDER BY weight", $node->nid);
      $iids = array();
      while ($iid = db_fetch_array($res)) {
        $iids[] = $iid['iid'];
      }
      return array('iids' => $iids);

Hence the $node->iids are already ordered by weight.

You don't actually need the exact weight value when you are making the draggable table -- all you need is the correct order. Put in values for the weight form element starting at 0 and going up.

Similarly, when you save you don't save the actual weight value -- that may have gaps in it. Weight should always be numbers 0...N-1 for N items, so save the lightest node with weight 0 and increment up to N-1.

mja’s picture

Brilliant - get it now - on it later.

Thanks Joachim...

sun’s picture

Is it wise to change the UI this late in an already stable branch? Sites may have applied custom styles to the current UI.

joachim’s picture

It's editing UI -- will sites have styled that? I tend to think that any changes made to editing UI should really be contributed back as patches if they improve it.

mja’s picture

Hi Joachim & Sun,

Okay, here's a new version of the patch. Took onboard what you said Joachim, so this one has no extra db calls, cleaner formatting, better commenting and also includes a delete check-box in the sortable table.

Hope it looks better. I understand what Sun says about messing with a stable version - completely up to you guys what you want to do with this, but just wanted it to be available to other users who might have clients who want to be able to sort their images.

Just to be clear to everyone, this is a patch against the production version of the Image module: 6.x-1.1

sun’s picture

Status: Active » Needs work

ok, after looking at the code, it looks like we can do this.

What follows is a mammoth patch review:

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -224,13 +224,34 @@ function image_attach_form_alter(&$form,
+          $form['image_attach']['image_thumbnail']['#theme'] = 'image_attach_image_attach';

Should be (theme_)image_attach_form_thumbnails

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -224,13 +224,34 @@ function image_attach_form_alter(&$form,
+          $imageweight = 0;

Should be $weight

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -224,13 +224,34 @@ function image_attach_form_alter(&$form,
+          foreach($node->iids as $iid) {
+	    $image = node_load($iid);

1) Missing space after foreach -- same issue also elsewhere in this patch.

2) No TABs. Two spaces instead.

See http://drupal.org/node/1354 for details.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -224,13 +224,34 @@ function image_attach_form_alter(&$form,
+	    $form['image_attach']['image_thumbnail'][$iid] = array(
+	        'image' => array(

Each of the sub-elements should be declared separately; i.e.,

$form['image_attach']['image_thumbnail'][$iid]['image'] = array(
  ...
);
+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -224,13 +224,34 @@ function image_attach_form_alter(&$form,
+	      'delete' => array (

Stray space after array.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -224,13 +224,34 @@ function image_attach_form_alter(&$form,
+	        '#value' => $image->title,

Missing check_plain()

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -224,13 +224,34 @@ function image_attach_form_alter(&$form,
+	        '#title' => t('position'),

Should be 'Weight' to follow core, and also use proper capitalization.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -224,13 +224,34 @@ function image_attach_form_alter(&$form,
+	        '#default_value' => isset($form_state['values']['image_thumbnail'][$iid]) ? $form_state['values']['image_thumbnail'][$iid]['weight'] : $imageweight++,

We do not need the conditional assignment here, since we are only specifying the #default_value. Form API's form_builder() will check whether there is a POST value for this element and assign it to the effective #value, if there is one.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -224,13 +224,34 @@ function image_attach_form_alter(&$form,
+	      'id' => array(

Should come first.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -292,6 +319,32 @@ function image_attach_form_alter(&$form,
+ * This private function is called by submit handlers to discover how the user has ordered their images.
+ * It returns an array with identical keys and values (just like the ordinary iid array)

Various phpDoc lines exceed 80 chars. Make sure to read http://drupal.org/node/1354

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -292,6 +319,32 @@ function image_attach_form_alter(&$form,
+function _image_attach_set_image_weight($rawweightdata, $selectboxarray) {

Instead of this helper, we want and need to use a proper helper function for uasort(). This means we need to fork drupal_sort_weight() from D7, as image_attach_sort_weight().

Removal of deleted images needs to happen before or after sorting in a simple for each loop (or if you want to get into extremes, a helper function for array_filter() would work, too).

Lastly, the code in image_attach_form_alter() should already set a proper (maximum) weight for new attached images.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -292,6 +319,32 @@ function image_attach_form_alter(&$form,
+  return array_combine(array_keys($weightarray),array_keys($weightarray));

FYI: Always check compatibility — array_combine() is PHP 5 only. Use drupal_map_assoc() instead.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -417,7 +475,8 @@ function image_attach_nodeapi(&$node, $o
-        // Populate weight column with placeholder values.
+        // Get weights from drag-to-reorder table
+        $node->iids = _image_attach_set_image_weight($node->image_thumbnail, $node->iids);

We cannot do this change. hook_nodeapi() is an API function; the 'update' operation is invoked whenever node_save($node) is called with an existing node. There is no form involved, so $node->image_thumbnail does not exist.

It only exists here, because you happened to submit the form, and Node API contains some ugly code that simply converts the submitted form values into node object properties.

Effectively, this overall feature likely requires a database schema change in order to add a 'weight' column and value per 'iid', so the weight in submitted form values can be properly stored and read from the DB.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -573,10 +632,40 @@ function image_attach_theme() {
+    'image_attach_image_attach' => array(
+      'arguments' => array(
+      'form' => NULL
+      ),
+    ),

The inner array of 'arguments' should be not use multi-line syntax.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -573,10 +632,40 @@ function image_attach_theme() {
+ * Theme image thumbnails in editing ui to a drag-to-reorder table

Should be "Returns HTML for tableDrag of attached images in node form."

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -573,10 +632,40 @@ function image_attach_theme() {
+ * @param $form
+ *  The $form object
+ *
+ */

1) Wrong indentation of @param description.

2) Stray blank trailing phpDoc line.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -573,10 +632,40 @@ function image_attach_theme() {
+  drupal_add_tabledrag('draggable-table', 'order', 'sibling', 'weight-group');
...
+  $output = theme('table', $header, $rows, array('id' => 'draggable-table'));

The HTML ID needs to be within the namespace of Image Attach module; i.e., 'image-attach-form-thumbnails-table'.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -573,10 +632,40 @@ function image_attach_theme() {
+  $header = array('Attached Images', 'Delete', 'Title', 'Weight');

Missing t() for table header column names.

+++ image_diff/contrib/image_attach/image_attach.module	2011-01-28 14:50:07.171875000 +0000
@@ -573,10 +632,40 @@ function image_attach_theme() {
+    $element['weight']['#attributes']['class'] = 'weight-group';

Should be 'weight' only.

Powered by Dreditor.

mja’s picture

Status: Needs work » Active

Blimey! Thanks sun, but feeling rather shame-faced at this end.

Let me have a look at this end and see what I can do. Re the database schema change - there is already a weight column in the image_attach table - I cut out a call to it on joachim's advice. Do you want me to add it back in?

Thanks for such an in-depth review.

sun’s picture

Status: Active » Needs work

That simplifies things, of course. :) So we only need to store the tableDrag weights and make sure that they're properly accounted for in an ORDER BY when attached images are loaded from the DB.

mja’s picture

Status: Needs work » Active

Hey sun - one quick question.

If we strike the node->image_thumbnail stuff out of the hook_nodeapi call (and I understand why) we are faced with a problem - what happens if people reorder images, and then hit the main node 'save' button, rather than the 'upload' submit handler for the image_attach form? As it's currently set up, the images will simply be saved with placeholder weights. The query could be changed not to set any weights, but that still doesn't help people who just reorder and hit enter, say...

Would it be acceptable to put if a conditional to check if node->image_thumbnail had been passed from a form? Otherwise I think we'd have to roll another submit handler...

Apologies if I've misunderstood things.

joachim’s picture

Status: Active » Needs work

> Blimey! Thanks sun, but feeling rather shame-faced at this end.

You really shouldn't. Sun's patch reviews are very detailed -- brutal even ;) And lots of issue patches go through lots of work and brutal reviews by lots of contributors before they get in.

> there is already a weight column in the image_attach table -

Hehe. I put it there when we switched to multiple images so the schema wouldn't have to be updated later on ;)

>Each of the sub-elements should be declared separately; i.e.,

Really? I think I prefer a Big Fat Array -- provided it's all indented nicely.

Watsdis’s picture

This looks really good - in fact what I've been looking for awhile. Any chance it will be available anytime soon?

joachim’s picture

It's marked as 'needs work'. That's open to anyone to jump in and finish it off. Neither myself nor the other maintainer have the time to do more than review patches & commit them when ready, so it's up to the community to work on this.

klaasklever’s picture

+1 - need this feature too.

what solutions do you guys have in use as a workaround?

thx and regards, kk

jakobbebop’s picture

FileSize
6.17 KB

If anybody's still interested in this, I have tried to some of the remarks from #18 (the trivial ones really), and re-applied the patch to by hand to go with the new version of image_attach.module from image-6.x-1.1

It's the first time I do something like this, so if I messed up in formatting the patch or sumthing similarly silly, please let me know

joachim’s picture

Version: 6.x-1.0-beta5 » 6.x-1.x-dev
Status: Needs work » Needs review
FileSize
6.68 KB

Nice work!

I've tested this and it seems to work nicely.
Here's a reroll with a few things cleaned up: whitespace, code and comment style, and variable names. I've changed the 'Delete' label on the draggable table to 'Remove' because it doesn't actually delete the image node, just unattaches it.

One thing that strikes me about the UI we now have is that there are *two* ways to unattach an image: the remove ticky box and unselecting it in the select box. It might be an idea to convert the select box to just being about attaching existing images. I've had a play around with that but there are lots of checks on form_state[iids] in the various submit handlers and so it's a fair amount of work. So unless anyone feels like working on that, it may be best to leave that to a follow-up issue for tidying up the UI. After all, it's not broken.

I'll leave this at needs review for a few weeks to give people time to try it and report back. If there are no problems after then, I'll commit and make a new release! :)

jakobbebop’s picture

One last thing (also as mentioned in #18) is to avoid array_combine for the sake of PHP version compatibility; I edited the patch to use drupal_map_assoc instead

joachim’s picture

Status: Needs review » Fixed

Thanks for spotting that!

It's been a month and no further comments, so I'm going to commit and make a release. Thanks to everybody who's worked on this!

- #792444 by mja, jakobbebop, joachim: Added UI to change order of attached images.

joachim’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.