CCK: Allow customized teaser/body display

Very interesting! I think this approach looks like it has potential. Do you want to try to make a patch??

the screenshot IS actually a screenshot
Wow. That's even better, then - kudos, that seems really neat.

2 remarks :
- Do you think something could be done about fieldgroups ?
- How this could play with feild weights. I suppose people will want to be able to have different weights depending on the context. I was wondering what this could mean in terms of UI - of course we can always say that it's not supported and they have to manually theme their nodes.

Well, I don't think fieldgroups would be able to handle the idea of having fields that live in different groups in different contexts (i.e. in the 'Admin' group on a full view but in the 'Basic' group in a teaser), at least not without some changes to both its tables and its logic.

Would you ever use groups in a teaser or rss view? The simple answer might be to assume groups only apply to the full view, but maybe I'm missing some use cases.

Thanks a lot Eaton. I did not test yet, I just looked at the code.

It seems you chose to store the additional display settings in a new db column instead of adding them in widget_settings like you mentioned in your original post. Could you elaborate on that ?

great concept and allow for better custom theming! subscribe.

Isn't it what contemplate is already doing?
http://drupal.org/project/contemplate
What is the difference?

subscribing.

Jeff, we just issued a new release of CCK and as a part of it there were a lot of miscellaneous fixes applied. I think the changes are done for now, so can you re-roll your patch?

I don't think putting it in widget settings will create problems for custom widgets if it is controlled by the content module. But another concern I have as I think about this, not having really tested anything yet, is that formatters are set in the field rather than the widget, plus only field settings are available to Views, so it may be that it has to be in field settings rather than widget settings. On the other hand you are setting this for the content type and normally per-content type settings would be set in the widget. So this whole widget setting vs field setting vs separate setting question needs to take into account all these issues about Views and formatters and how you dovetail them all together.

And now that I probably have confused everyone by rambling along as I think out loud, I think I would say leave it the way you have it for now, as a separate field, and let's test and see if it should or could be a widget or field setting instead.

Eaton : I'm currently testing your patch and working on some modifications (I left display_settings where they currently are :-), and I agree with Karen that we can leave them be for now)

I'll submit these shortly as a new patch so unless you have reworked things, maybe you don't have to bother with a re-roll.

Here's a new version of the patch against current 5.0--1.x-dev :

I modified a few things in Jeff's original patch :
- I moved the "Hide Field" pseudo-format out of the dropdown into a separate checkbox for each field / context pair.
It allows IMO for a better visualization of what's enabled or disabled, and allows you to hide a field and later show it again without losing the formatter that was selected.
- I also removed the automatic 'default' option for formatters since I _think_ field modules have to define a 'default' formatter anyway. This might need double-checking.
- I reworked the default theme_field function to take the label display setting into account (when populated | always | never)
- I managed the displayed / non displayed state through the '#access' property in the $node->content array (drupal_render stuff).
I think this was agreed as the preferred way in previous discussions, but it also means the formatter and the theme_field function are called all the same even if nothing is displayed in the end... Opinions ?

More importantly (and arguably), I tried to avoid the need for an API change for field modules (additional param for hook_field in Jeff's patch), since it might delay the functionality to a future CCK 2.0 release OR complicate the life of field module maintainers.

In short, this patch does no longer delegate display to field modules's hook_field('view') (following one item in dopry's laudry list in a recent "internal" cck mail exchange).
Meaning I moved the view operation out of _content_field_invoke('view') and into a new _content_field_view, that does not call field module hooks (but calls formatters). Most field modules implement the 'view' op the same anyway (and they should).

Now maybe there should be a way for a field module to specify he does want handle the display himself. Imagefield for instance does some specific things in his hook_field('view'). I did not check yet if this is a real problem or not.

Wow, this is too long.

I totally agree on the handling of the label. I've never run into any situation where the h3 made any sense, so I always end up themeing it out. I'm surprised no one ever posted that as an issue, but no one did and I've had other fish to fry, so I never followed up on it :-)

I did left a part of my code out of the patch - not intended, of course...
I'm away from my code right now, I'll post the complete version tonight.

Here is my take on this patch...
I liked the one without the checkboxes better. It made for cleaner code.

Just an explanation on the reason for the form_alter (which is a copy of what I used in creating the 'Manage Fields' display). The reason for doing it that way is to pick up form items added by all other modules, like taxonomy. If you only pick up the node_form and the content_form you will miss those. It is messy that way and there may be a better way to do it, but I wanted to pick up all form elements with weights between -10 and +10 so you can see exactly where the fields will drop into the form.

Also, if you guys can post screenshots of your changes we can get feedback on the UI from people who can't or don't have time to try all these patches out.

Haven't tried the patch out, but the screenshot looks awesome. Am I correct in my current interpretation that this could negate the need for the break command if used with two fields - one of which was set to show only on 'full page' view (e.g. the body) and the other set to show on 'full page' and 'teaser' views (e.g. the teaser)?

This is too cool for words. Really.

It's no big deal that the teaser column doesn't get populated in the node revision table, since that's not permanent stuff there anyway. Teaser column of that table is kind of like a scratch pad that is subject to be rewritten/overwritten at any time. Or in this case not written to at all. (hey, maybe this is an optimization too! Saves on the I/O) ;)

How does this jibe with field formatters in a table View? Seems like some duplicate functinality there ... I have only looked at the screenshot so far.

Wow, you raised a lot of enthusiasm here, Eaton :-)

A few remarks on the last patch in #30 :

- I don't know what dopry precisely meant when he wrote in #25 :
I liked the one without the checkboxes better. It made for cleaner code..
Obviously (i proposed that) I do like the checkboxes for the 'context' (full, teaser, page) columns,
but I do prefer the 'no checkbox' version of label columns (simple dropdown).
With checkboxes all over the place, you don't distinguish label columns from 'context' columns.
Plus we lose the "when populated" behaviour, that is now CCK's current (and most wanted IMO) behaviour.

What about two dropdowns for the labels :
When populated (default) / Never / Always
Label (default) / Inline

- I like the Inline label styling but it does not seem right with multiple fields (label gets repeated). :
label : value_1
label : value_2
label : value_3

Something like
label : value_1
..........value_2
..........value_3
(not actual dots obviously, but my indents get truncated no matter what)
would be better IMO, but this would probably require providing some CSS.

- I'm not fond of the formatter_matrix name, especially since it also contains label display settings. I'm not sure I get the issue with the initial 'display_settings' name - OK, that's a detail.

I'll try to submit a new patch shortly - opinions welcome meanwhile ?

the current behavior when labels are turned on *is* the same as the current 'When Populated' mode
I missed that - In this case, I agree that having two select boxes is not needed.
A checkbox for 'label / no label' is the right UI choice obviously, but I do find it makes the structure of the table much less readable. If we could find a way to visually highlight that the label column is another animal than the last three ? (still searching)

About "page" : mixed feelings here as well - I left it in for now. One thing is sure : we are bound to get support requests about this if we don't provide a proper explanation.

About "inline" : it also not suited for "block" fields ('filtered' text fields, image fields). Plus I think for those fields where you have to fall back to "header", the visual difference between both label styles makes some ugly output.
I think we can give this and "rss" issue a closer look after this gets committed ?

Attached is an updated patch - same UI as in #28, with a little code cleanup.
The main thing that prevents this from being ready IMO is the case of imagefield (and possibly others ?), that will be affected by our bypassing of hook_field($op = 'view') - see the end of my comment #19

@KarenS, re: form_alter
So you want all fields added by all modules not just CCK to be managed by FieldGroups? I guess its necessary for FieldGroups.

@yched, re: display_settings
It's too close to widget_settings where the developed creates a form that is settings specifically for the widget. I guess display_settings is ok. I just thought it was a little confusing. Since it didn't actually tie into any of the CCK callbacks.

re: checkboxes
if we didn't have them we make the form handling logic a little more straight forward, and easier to for people to override. I do like the UI though...

+1 for strong vs h3

+1 to adding rss context, but that would require changing the finger of theme_field I think. I'd really like to see this simplified enough to get it in as a feature addition in the cck 1.x series. I think changing those functions fingerprints will force us into 2.x territory, since modules will have to be updated instead of having an unused hook_field op=view.

So there is the question of which branch of CCK people feel this should be included in.

Looking at the UI screenshot, I think it would be simpler to remove a checkbox and have "Hidden" in the dropdown list. For D5 a similar task was done to the blocks admin page and I like it a lot better (we removed enabled/disabled checkbox and added a to the region list). But looks kick ass so far!

@dopry, hook_form_alter wasn't so that fieldgroups would manage the fields, it was so all the important elements would show up (dimmed out) on the page so people can see whether the weights they give their fields are going to place the fields where they expect to find them. This is related to all the support questions we started getting when we fixed weights (like 'how come my title is now in the middle of the page'). The idea was to make the layout emulate the way the page will lay out.

I haven't had a chance to try this patch out, but hope to get to it soon. For the label css, I think we should wrap both labels and values in divs, then put some appropriate style in content.css as a starting point. It's hard to overcome elements like h3 and strong if you don't want that behavior. For multiple elements, we can give the first label a different class than the subsequent ones, then you can style them differently (maybe even just 'display:none' on the subsequent ones). Divs are the most flexible elements we have to work with, so I would stick with them.

Updated patch :
- implements Karen's suggestion about replacing 'h3's and 'strong's with 'div's for labels display.
- fixed a typo in my previous patch (widget_settings was no longer retrieved)
- renamed formatter_matrix back to display_settings. I really find formatter_mattrix too misleading. Suggestions welcome for a better name.

Attached screenshot shows labels styling implemented in the patch above.

First field 'texte' is multiple with 'inline' label.
Second field 'nombre' is single with 'header' label

Well, I guess the fields could be ordered by weight, but that would probably require handling fields nested in fieldgroups and I'm afraid this is a little beyond me right now.

Other than that, this looks ready to me.
I'd rather wait for Karen to review this, and dopry to say his word about imagefield, though.

Oh, and maybe wait for someone with a more fluent english than I have to propose some word of explanation ?

Just looking at the screenshot, haven't tested the patch, but...

The UI would be improved imo by using RobRoy's suggestion in #43 and including "" as one of the options.

Consider the fact that you have to click three times in order to turn on a box that is using a non-default option: once to check the box, once to select the drop-down and another time to select the "whatever" in the drop-down. Now repeat that for your typical CCK type that might have at last 6-8 fields. Now repeat that for each CCK type. etc. Further, drop-downs are bigger and easier to "hit" with the mouse than checkboxes, making data entry easier.

Bah. That should be "including '<hidden>'..." -- I should really learn to preview someday. ;)

Well I was working with a couple of the earlier patches and you kept changing things here faster than I could get my installation updated :-)

+1 to getting rid of the checkboxes. They clutter the screen more, as webchick noted they add more clicks, and they take up room we may want in the future for other options.

+1 to removing the 'Page' option. I still don't understand when that would have come into play anyway, can someone enlighten me??

+1 to adding RSS in now that there is more room.

I think we could futher simplify this now to eliminate the other fields on the screen (the whole $dummy_form bit) since we already have a screen that shows weight and I'm not sure we need it again here, although we'll have to re-visit that whole idea if we come back and try to allow for different weights in different contexts (which would definitely be a 2.0 feature).

I'm not sure about the label option. It's not really the same thing as the others, which makes it confusing. Also I would change the wording from 'header' and 'inline' which are kind of technical terms to 'above' and 'beside'. There's no better place to put it though, I guess.

As to the issue of how this affects other modules, I would like to retain some way for a field module to provide its own handling of the display anyway, so one option is to first go ahead and see if anything is returned with the $op = 'view' when creating the display and if it does, let the module handle it, otherwise handle it here. That will keep other contrib modules from breaking until they get updated and make a way for them to do custom handling if they need it. Plus of course we need to extend the patch to alter the core contrib modules to remove their handling of the view.

The other thing I was going to do was take a good look at Dopry's patch to see if there are other things we should incorporate from that, but I haven't gotten that far yet...

I was thinking more about this and I think we should go with something like the current patch, with whatever changes we agree on, for 1.0, then for 2.0 I would change the 'display' tab to have a series of subtabs for 'Teaser', 'Full', 'RSS', 'Print', etc. Each of those subtabs would have a display like the 'Manage Fields' screen that provides a way to change weights and groups just that option, and on those screens I would go back to pulling in all the other fields in the form for context.

Also, we never got back to the question of whether this should be a separate setting or part of field or widget settings. Since it looks like we may end up with many other options in the future, maybe it makes sense to keep it a separate field?

Some comments on the last posts :

- I think dopry's version of the patch is already "in there". The patch evolved in an iterative way, each new one building on / modifying the previous one without forking.

- about label setting names : would 'above' and 'inline' be OK ? 'inline' is also common language.

- about how to get this in 1.0 and handling of field module's current hook_field('view') :
I don't think we can call it anyway and see if it returns something, for it currently always will. And we can't require field modules to remove their 'view' op right now, because then they would break with <= 1.2 cck release. And I think we want to avoid things like "date.module 1.5 requires cck 1.3".
What we could do I guess is, as Karen also suggested, to provide field modules a way to explicitly say "I want to handle the display myself" (in hook_field_info ?), and only then defer to it.

- it seems there's a consensus about removing checkboxes, but I'd like to advocate one last time. If consensus stays, I'll forever hold my peace :-)
- They provide a better visualization of what's enabled and what is not.
- 'enabled / disabled' and 'output format' are basically two different settings. With only a dropdown, when you disable a field and later change your mind an re-enable it, you have lost the output format you previously set and have to figure it again.
- They are checked by default so I don't agree with webchick's 'count of clicks' in #52 :
Most of the time IMO you'll only want to change 'enable' status OR 'display format', and not both.
This being said, I'm aware that 'admin/blocks' moved away from checkboxes, but I'm not sure this was a good choice, for the same reasons.

New version of the patch :
- simplifies 'dummy_form' code stuff, since we only retrieve actual cck fields
- renamed 'header' into 'above' for label display options
- cleared _content_field_invoke and _content_field_invoke_default of the specific '$op == 'view' bits - 'view' does not call them anymore.
- moved around code in content_admin a bit gather with the "width" overview. I also renamed fonctions to include the notion of 'overview' - so if you tested a previous version, you'll need to empty your cache.
- added a way for contrib field modules to specify they want their 'view' op to handle the display instead of the default behaviour.
hook_field_settings accepts a new op, 'callbacks', which should return an array whose keys are op names, and whose values are booleans (TRUE if you want your callback called instead of the default - only op 'view' uses it for now).

function imagefield_field_settings($op, $field) {
  switch ($op) {
    case 'callbacks':
      return array(
        'view' => TRUE,
      );
  }
}

(i took imagefield because it's the only module I'm aware of that would require it - dunno why I focused on imagefield in my previous posts, its 'view' op is standard)
I do not find this ideal, but it's the best I could come up with in order to have the 'display settings' functionality work without requiring contrib modules to be updated (and then breaking with <= 1.2 cck) and without messing with CCK 1.x API.
We should be able to make it cleaner in 2.0.

BTW : If anyone wants to tackle 'RSS view', new patch is welcome, i'm not too familiar with that area.
I do think it can be added later on after this gets in, though.

yched, thanks!! I like this a lot. Just a couple points:

1) We need some sort of message when you save the values to indicate they got saved. It looked at first like it wasn't working because there was no message.

2) We lost the 'hidden' option for each field to leave it out completely.

I haven't yet tested the handling if a field wants to do its own display, so I'll check that next.

Here's a patch with my (minor) changes to yched's patch.

1) Added confirmation message when you change the display values.
2) Added in a 'hidden' option for each field
3) Changed the 'Display' tab to say 'Display fields' for consistency with 'Manage fields' and moved the tabs so it is just to the right of the 'Manage fields' tab. I tried just using 'Manage' and 'Display' instead, but when you're on the 'Edit' tab those labels are confusing, so I think they need to include the word 'fields'.

Also, I like the method for allowing modules to indicate they want to handle their own display. That is nicely extensible and I will use it when I make the patch for setting default values to create a method for modules to take over the handling of default values, too.

I think this is RTBC, but would probably be good for someone to test my altered patch just to be sure.

- RSS : let's do that later :-)
- missing message on form submission : quite right
- lost 'hidden' option : Er, yes, I forgot that while merging Jeff's last patch and my work.

Attached patch corrects the last two points.

Arg, crossposted :-)
Karen, I'll look at your patch, please forget my patch in #63

I modified two minor things in Karen's last patch :
- changed the weight of the "add group" tab so that it comes last
we now have "edit | manage fields | display fields | add field | add group"
- moved the help text for the display overview form into hook_help where it belongs best.

... and I committed the thing :-) Yay.

Well, back to regular bug queue fix, I guess :-)

I've got it working in 4.7 now, I'll commit a 4.7 fix.

Great ! I was thinking that this ought to be backported, but felt a little lazy about it...

OK, 4.7 is now updated, too.

Thanks yched and Eaton, this is a really useful feature!

FYI: This is really cool... Yched, don't worry about being to considerate of imagefield. I'm trying to standardize it as much as posssible. I wan't it to work as cleanly with content.module as possible in the long run.

I could be jumping the gun here, but the cvs packing script has been kind of borked the past couple weeks - there are no new tarballs for 5.x or 4.x as of right now even though I can see the commit messages. Maybe that is because they were committed as dev snapshots - I think those still only get made a couple times a day. If however they were committed as an 'official' release then something is definitely wrong and the packing script skipped over this (those get package every 5 minutes now). Just thought someone would like to know. If I'm just jumping the gun, I apologize in advance.

13+ hours and no tarballs...

I always thought tarballs were generated once per day ?

Actually, what has happened is that the 5 developer release tarball does not reflect the new date that it was created on - but the new files are in there.

If you go to the releases page http://drupal.org/node/48429/release you can see that the cck 5.x-1.1 tarball has a date of 1/8/07 instead of 1/17/07 like it should.

This is in keeping with this bug, which I've seen before but thought might have been fixed by now. I've mentioned it to dww. I'm not sure if it would help if others did or not.

Head and dev releases DO reflect the accurate updated dates.

Pardon me - a horrible typo on my part - it is the cck-5.x-1.x-dev.tar.gz and cck head which does not have the true dates on them. They indicate that they were last updated 1/13/07, when really it was 1/17/07.