I used the previous dev-version, and was able to create a view that took image-size as an argument. Now that I upgraded to the newest dev, that option disappeared. I mean, I can still take image size as an argument - and it gets verified and stuff. But when I want to output an image as a field, I have to specify the image size. So, I can use size as an argument - but I can't do anything with it anymore?

CommentFileSizeAuthor
#10 image_454290_views_size_argument_2.patch7.79 KBjoachim
#8 image_454290_views_size_argument.patch8.29 KBjoachim

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Postponed (maintainer needs more info)

Sorry, I'm not really sure what you mean here. Could you please elaborate a bit more?

Ibn al-Hazardous’s picture

Ibn al-Hazardous commented

I have a view that displays image nodes as fields. I take "Image: Image preset size" as an argument, and I would like to use it when displaying the image field. When I add "Image: Image" to my set of fields, I get a number of settings - almost all of which look like in the previous dev-version. But the last one is a drop-down box that says "Image size to show", and this is a new one. Since it's there, I can't use the size set by the argument - I always get the size from the field settings.

I suppose that the expected behaviour would be for the "Image size to show"-option to disappear if I have size as an argument, or at least to have an option that says "Use argument size". In the previous dev-version, this just worked - no list of sizes, and the view used the size I specified as an argument.

My current workaround is to have a separate view for each size (4 of them), which works but isn't much fun to admin.

joachim’s picture

joachim commented
Title: No relation to size » Restore views image size argument
Category: bug » feature
Status: Postponed (maintainer needs more info) » Active

I'll look into this.
This isn't a use case we had considered and I can see its uses.
I'm all in favour of making one view work in several places -- I do that a lot :)

The thing is though the old argument would have been a filter. This is going to be merely a setting. So it's a sort of dummy argument that has to pass its value to the field. Not sure how we'll do that... suggestions welcome!

You could have 4 different displays as a workaround in the meantime. You'd only need to duplicate the fields, rather than have 4 views.

Ibn al-Hazardous’s picture

Ibn al-Hazardous commented

I'll see if I come up with something, but I'm not at all familiar with the programming API for views. I do know, however that the size argument can be passed to another field (named "Image size") - but it just writes the name for the text.

I tried using different displays first, but I call the view with views_embed_view - and it didn't take display names other than the standard ones as arguments. I may look into it a bit more - but I had to deliver something that works the day before yesterday. :/

joachim’s picture

joachim commented

Off the top of my head:
The image field handler could have a size option "take from argument".
The handler then needs to look in the $view object it has, look in the arguments, and find the argument we need. It then picks the value from that argument.
The argument handler meanwhile doesn't need to do anything except sit there, though if empty it should probably supply '_original'.
There should be a validation warning too, if the size option is set to argument but the argument isn't there.

If you fancy taking a stab at it, that would be great.
I'm coming up to a deadline so don't know when I'll be able to look at it.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

How about:

- Use the preset size select list to define the default.
- If the view doesn't use the preset size argument, use the selected size.
- If an argument is passed and it's a valid preset size, use that size.
- If no argument or an argument with invalid preset size is passed, use the selected size.
- Profit?

jesss’s picture

jesss commented

Subscribing.

From a user perspective, I think joachim's workflow is a bit more consistent with how arguments work elsewhere. Since you can set a default preset as part of the argument handling dialogue, all the decision making would happen in one place.

joachim’s picture

joachim commented
Status: Active » Needs review
StatusFileSize
new image_454290_views_size_argument.patch8.29 KB

Patch on HEAD.

Please test :)
Note that #544098: Handler validation: validate defaulted handlers too means validation will not always manage to detect if the argument and the field are both present in a display.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ contributions/modules/image/views/image.views.inc Locally Modified (Based On 1.2)
@@ -154,7 +162,7 @@
-    'module' => 'views', // This just tells our themes are elsewhere.
+    //'module' => 'views', // This just tells our themes are elsewhere.

uhm, why not simply remove this line?

+++ contributions/modules/image/views/image_handler_argument_image_node_image_size.inc Locally New
@@ -0,0 +1,105 @@
+ * @file
+ *
+ * Provide an argument to set the image size of an Image node image field.
+ * This works with image_handler_field_image_node_image and expects it t

Description seems to be cut off.

Prior to the description, there should be a one-line summary - not sure about the overall current state of the Views integration files.

+++ contributions/modules/image/views/image_handler_argument_image_node_image_size.inc Locally New
@@ -0,0 +1,105 @@
+    // TODO: this is not yet perfect
+    // as it is possible to add a display which inherits this argument,
+    // but overrides its fields, and the checking here doesn't find that.
+    // See http://drupal.org/node/544098

Can we use proper PHPDoc syntax here?

// @todo Your comment, properly capitalized, and when it is exceeding 80 chars
//   like this dummy comment, then indent following lines.
// @see function() || <URL>

Beer-o-mania starts in 22 days! Don't drink and patch.

joachim’s picture

joachim commented
StatusFileSize
new image_454290_views_size_argument_2.patch7.79 KB

> //'module' => 'views', // This just tells our themes are elsewhere.
TBH I just commented out out of curiosity for what it might do. It's used in views but it's probably a re-basing thing for inc files that supply things for core modules. I've left in the uncommented line to avoid patch noise :)

joachim’s picture

joachim commented

This patch is essential to a 1.0 release of image module.
I intend to commit it while at DrupalCon Paris, regardless of its status, prior to making a 1.0 release
If you care about the quality of image module, please try to test it before then.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: +Release blocker

@joachim: You can use a nice issue tag for that so we get a neat overview of issues to tackle for 1.0 (in addition to the manually compiled list in the release coordination issue). :)

joachim’s picture

joachim commented
Status: Needs review » Fixed

Nobody testing... :(

Ran it through its paces again. All works for me.
Committed.

Hetta’s picture

Hetta commented

Sorry about the "not testing". I tried to upgrade localhost from d5 to d6 and ran into a host of problems, although I was using a copy in a subdirectory ... something funky in my apache install. Gotta start that from scratch, and _that_ takes time.

Now, d5 I can test ... however, you're not doing patches against d5 anymore, so shrug.

joachim’s picture

joachim commented

I didn't mean anyone personally :)
Just that with however many thousands of users we have and hundreds of people commenting on issues, we have very few testers! :(
If there are tested patches for 5, I can commit them :)

Status: Fixed » Closed (fixed)
Issue tags: -Release blocker

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

Issue tags: +Release blocker

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.