While in the process of designing a theme for D7, I realized it would be very handy for Drupal to install an image field along with the title and body field for the 'article' content type. By including this field by default, themes can place those images in their design. It helps differentiate the article type from the page type. It helps show people that Drupal can in fact now handle images right from the start. And it allows designers to make cool "magazine-style" themes and other such prettiness with *images*!

I'm specifying the teaser to display a "medium" sized image linked to node, while the full node displays the "large" image (via image styles that are already predefined.)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

Category: feature » task
Issue tags: +Usability

I strongly support this change. I think it would be really awesome to show off this great new feature of Drupal front-and-center. I also love that now there actually would be a visible difference between Article and Page in the default install, for the first time ever. :) Will help greatly with user confusion around this point, IMO.

Tagging so the UX folks can weigh in.

Bojhan’s picture

I think, that conceptually as webchick mentioned it is great to add weight to anything that makes the distinction between a page and article greater. Apart from the obvious design advantages, I think it has great information value that we now support media. Be aware that the field has to support several image,doc and file exstentions.

jensimmons’s picture

FileSize
5.6 KB

Hey here's a patch.

dopry’s picture

Status: Active » Needs review

@bohjan; don't bike shed this.. This issue isn't about all kinds of media handling. It specifically about demonstrating the image handling capabilities of drupal up front. The bigger question of handling multiple media types and supporting them in core as a part of the default profile is something when can do when that functionality is realized.

webchick’s picture

Status: Needs review » Needs work

Looks like there are some unrelated hunks in here (.htaccess, default.settings.php..)

Also, um. Wow. Does it really take /that/ much code to make a simple field? o_0

jensimmons’s picture

Yeah, there's crap in the original patch that doesn't belong there. Trying again....

dopry’s picture

well it's only two function calls, most of the code is configuring the field and then the field instance.

jensimmons’s picture

Status: Needs work » Needs review
FileSize
3.06 KB

How about this?

yched’s picture

+    'cardinality' => 1,

+    'locked' => FALSE,
+    'indexes' => array( 'fid' => array('fid')),

+    'settings' => array(
+      'uri_scheme' => 'public',
+      'default_image' => FALSE,
+    ),
+    'storage' => array(
+    'type' => 'field_sql_storage',
+    'settings' => array(),
+     ),

+    'required' => FALSE,

+    'widget' => array(
+      'type' => 'image_image',
+      'settings' => array(
+        'progress_indicator' => 'throbber',
+        'preview_image_style' => 'thumbnail',
+      ),
+    ),

+      'rss' => array(
+        'label' => 'hidden',
+        'type' => 'image_large',
+        'settings' => array(),
+        'weight' => -1,
+      ),
+      'search_index' => array(
+        'label' => 'hidden',
+        'type' => 'image_large',
+        'settings' => array(),
+        'weight' => -1,
+      ),
+      'search_results' => array(
+        'label' => 'hidden',
+        'type' => 'image_large',
+        'settings' => array(),
+        'weight' => -1,
+      ),

are all defaults that can be omitted.

+        'type' => 'image__large',

typo

webchick’s picture

Status: Needs review » Needs work
forestmars’s picture

Status: Needs work » Reviewed & tested by the community

Patch tested and passed. I did get this spurious truncate message:

patching file default.install
patch unexpectedly ends in middle of line
Hunk #2 succeeded at 223 with fuzz 1.

However it works as advertised.

One thing however with the image sizing— someone may want to look into image cache as when I tested this with a 450x450 image it blew it up to 640x640.

dopry’s picture

FileSize
3.47 KB

Yched:

The defaults are there because they are not clearly documented anywhere, and this provides and illustrative example for any developers working to build an install profile that uses an imagefield. The formatter type is also there, so they are not in fact just defaults. Actually image_large is the typo it should be image__large.

This patch corrects the typos.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Let's get documentation that these are default values that normally don't have to be set, then.

Also:

+    'indexes' => array( 'fid' => array('fid')),

There shouldn't be a space before 'fid'.

Haven't tested the patch yet.

dopry’s picture

Status: Needs work » Needs review
FileSize
3.81 KB

docuemented defaulting, fixed a few whitespace issues.

last one you get from me, the rest is one someone else.

jensimmons’s picture

Oh, wait. The order of the fields is not what I intended. I wanted them to be
• title
• image
• body
• tags

for both the display of the node (teaser and full) and for the add content form.

Shoot. I don't like having tags before the body.

heather’s picture

I have tested and created screenshots.

jensimmons has a point. Although the Tags form field has been under the title field for as long as there have been tags, they display *under* the node when viewed. I think it is best to place editing fields in the order they are displayed. This makes sense in an editing context. When viewed, seeing tags underneath the image and text, the user could have an expectation that the tags field is after the text.

If the UI changes are to make the most common settings into defaults: this might be a good change.

heather’s picture

Issue tags: +Needs usability review
yched’s picture

re dopry #12 : "The defaults are there because they are not clearly documented anywhere"
They are documented in the PHPdocs of field_create_field() and field_create_instance().
[edit: which clarifies that, contrary to what I said in #9, the display settings for rss etc are not defaults and should rightly be specified]

pwolanin’s picture

Priority: Critical » Normal

A nice example of how to add a field to a type on install. We should also resolve: http://drupal.org/node/560780#comment-2190222 since that may alter what needs to be done here.

jensimmons’s picture

Assigned: jensimmons » Unassigned
Status: Needs review » Needs work

I thought by setting the image field to weight: -1, it would end up in the correct order.

Title is: -5
Body is: 0
Taxonomy is: 4

Again, the preferred order is:
Title
Image
Body
Taxonomy

I'm not sure what needs changing to make this happen. Someone else?

yched’s picture

Oh, that's because

+    'weight' => -1, 

needs to be in the 'widget' section instead of at the top level of the $instance array. The Field API (but not the current UI) supports different weights on forms and on each build mode.

yched’s picture

Side note: do we really want to index the image (whatever that means) and have it displayed in search results ? (search_index and search_result build modes)

heather’s picture

@yched, i have seen people asking how to make images appear in search results. can we have this an option in the display options for Search, RSS, etc etc?

webchick’s picture

Issue tags: +D7 UX freeze

Would still very much like to see this patch committed. It sounds like there's still some minor stuff to clean up, per yched.

yoroy’s picture

Subscribe. Important to have this in the default install profile, it's one of the coolest new things in Drupal we should show off!

yched’s picture

Status: Needs work » Needs review

Updated patch for my remark in #21.

re heather #23: Yes, the 'Manage Display' screen in field_ui lets you specify which fields should be hidden or visible in each 'build mode', like 'full node', 'teaser', 'search index', 'search result'... The settings specified by this patch are only 'how the field will be displayed until some admin changes the display settings'.

So my question was: do we really want the default behavior to be that this image field is indexed (I'm not even sure what this means for an image) and displayed in search results (I'm not even sure how this would come up in our current search results display) ?

yched’s picture

Forgot the patch.

catch’s picture

Indexing would possibly mean that the file name / alt / title tags get indexed? Not sure though. Either way defaulting to off seems sensible.

jensimmons’s picture

The fields still come up in the wrong order.

Only local images are allowed.

Tags should go *after* Full text.

(and how in the world can I get my screenshot to embed in the page? Img tags aren't allowed. Uploading an attachment doesn't auto embed. What's the deal?)

jensimmons’s picture

Ok I think we got it. Try out this puppy.

jensimmons’s picture

Rerolling to include a one-character change to install/install.module so that the large images in image styles are not upscaled by default

matt2000’s picture

Status: Needs review » Reviewed & tested by the community

I'm not a FieldAPI expert yet, but testbot likes it, and I like it, and I don't see any obvious flaws, so...

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Great work, everyone! Oh-so-gladly committed to HEAD! :D

quicksketch’s picture

I didn't realize this was going on, how exciting! Regarding Bojhan's concern in #2:

Be aware that the field has to support several image,doc and file exstentions.

If you meant we should have a "generic upload field", then that's something that I'm working on over in #563000: Provide upgrade path to file, which would replace the functionality of Upload module with File. Though Upload module isn't enabled by default currently (so File won't make any default fields if we match it one-to-one).

Status: Fixed » Closed (fixed)
Issue tags: -Needs usability review, -Usability, -D7 UX freeze

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