The attached patch adds drag-n-drop support to the 'manage fields' tab.
Fields and fieldgroups can be reordered, and fields nested into fieldgroups, using core's js drag-n-drop feature.

This new feature kind of forces another one upon us : the ability to act on the weight of non-cck elements (title, body, taxonomy, other _nodeapi elements added by contrib modules). We've resisted this request so far as being outside cck's strict scope, and potentially carrying along lots of possible conflicts and support / bug requests. With d-n-d coming into D6, though, now's a good time to take a stab at it.

After discussing this with Karen, here's the road taken by the patch :
We leave the previous 'dummy node form' approach to collect the components of the node form (annoying conflicts have been reported where some cck fields are not configurable anymore), and take a declarative approach instead.
We provide an API for contrib modules to have their 'fields' supported by the 'manage fields' tab at a minimal cost. If they don't do the job or do it wrong, this does not go into our issue queue :-)

Contribs modules :
- 'expose' their additions for the 'manage fields' tab using hook_extra_node_fields,
- instead of using hardcoded weights in theitr hook_form_alter / hook_nodeapi(''view') additions, call content_extra_node_field_weight() to get the desired weight stored by the 'manage fields' tab.

/**
 * Implemantation of hook_extra_node_fields.
 */
function mymodule_extra_node_fields($type_name) {
  if ($type_name == 'type_foo') {
    return array('mymodule_bar' => array('label' => 'Bar', 'weight' => 3));
  }
}

function mymodule_form_alter(...) {
  // if node form for type 'type_foo' :
  $form['mymodule_bar'] = array(
    ...
    '#weight' => module_exists('content') ? content_extra_node_field_weight('type_foo', 'mymodule_bar') : 3, 
  );
  ...
}

function mymodule_nodeapi(...) {
  // if 'view' op and node type is 'type_foo'
  $node->content['mymodule_bar'] = array(
    ...
    '#weight' => module_exists('content') ? content_extra_node_field_weight('type_foo', 'mymodule_bar') : 3, 
  );
  ...
}

Content.module's own implementation of the hook takes care of core-defined 'fields' (title, body, taxonomy...)
Content.module also takes care of altering weights in node form and $node->content for fields that are already in there by the time content's hook_form_alter and hook_nodeapi_view are called (due to hook execution order, that means mostly core stuff).

CommentFileSizeAuthor
#9 cck.patch36.9 KByched
#5 cck.patch35.26 KByched
cck.patch36.03 KByched

Comments

yched’s picture

yched commented

Side note : testing this patch requires a menu_rebuild();

karens’s picture

karens commented

Generally, as we previously discussed, I support this direction. I only have a small question -- shouldn't we rename the hook to prefix it with 'content'? Calling it 'extra_node_fields' doesn't indicate what module is implementing the hook if you run across it in the code of other modules. Why not 'content_node_fields' or 'content_extra_fields' instead of 'extra_node_fields'?

yched’s picture

yched commented

Yeah, I agree. I went with 'hook_content_node_fields' at first, but it sort of clashes with our {content_node_fields} table (with a completely different scope). 'content_extra_fields' misses the fact that we're talking about nodes, and 'content_extra_node_fields' is probably too much :-)

I'm not too happy with the current names. I guess I could go with 'hook_content_extra_fields' if we don't come up with a better idea :-)

karens’s picture

karens commented

I'm not sure we need to say we are talking about nodes, since nodes are the only thing the content module works with, so what else would they be for? I didn't think about the similarity of the table name for content_node_fields, so that one is certainly a bad idea.

I'd vote for hook_content_extra_fields (or hook_content_custom_fields).

yched’s picture

yched commented
StatusFileSize
new cck.patch35.26 KB

Agreed. New patch with hook_content_extra_fields and content_extra_field_weight().

Sample code above becomes :

/**
* Implemantation of hook_content_extra_fields.
*/
function mymodule_content_extra_fields($type_name) {
  if ($type_name == 'type_foo') {
    return array('mymodule_bar' => array('label' => 'Bar', 'weight' => 3));
  }
}

function mymodule_form_alter(...) {
  // if node form for type 'type_foo' :
  $form['mymodule_bar'] = array(
    ...
    '#weight' => module_exists('content') ? content_extra_field_weight('type_foo', 'mymodule_bar') : 3,
  );
  ...
}

function mymodule_nodeapi(...) {
  // if 'view' op and node type is 'type_foo'
  $node->content['mymodule_bar'] = array(
    ...
    '#weight' => module_exists('content') ? content_extra_field_weight('type_foo', 'mymodule_bar') : 3,
  );
  ...
}
yched’s picture

yched commented

Also note that a trade-off of the 'no dummy node form' approach is some logic duplication between hook_form_alter and hook_content_extra_fields, that both contain the logic to decide if an addition is made - see content_content_extra_fields() in the patch :

if (module_exists('locale') && variable_get("language_$type_name", 0)) {
...
}
if (module_exists('taxonomy') && taxonomy_get_vocabularies($type_name)) {
...
}

No big deal IMO.

karens’s picture

karens commented

OK, tried the patch out. Love it!! Love it!! Love it!!

This is a huge improvement on the old screen.

Can't say I've done a real thorough test yet, though, like testing how it works when javascript is turned off or what will happen if other modules are trying to alter weights. Since HEAD is still beta code, it might be safe to commit this to make it easier for the community to help debug it further.

yched’s picture

yched commented
Status: Needs review » Fixed

OK, I committed a slightly different version, which stores the 'extra' information in the cached content_type_info() array
(previous patch called the hook once per node type being displayed on a page, for every page)

Also added a drupal_alter on the results of the hook, for instance allowing 'automatic nodetitle' module to remove the 'title' row...

I'll add a note in the CCK doc pages about the 'manage fields' API, describing what contrib modules need to do to have their additions supported.

I still need to update the 'display fields' tab accordingly (no reordering, no 'extra fields', but we can display fields correctly nested inside groups :-). I think the screen real-estate this patch buys us is a good opportunity to think of ways to enhance/reorganize those pages...

yched’s picture

yched commented
StatusFileSize
new cck.patch36.9 KB

For the record, here's the patch I committed.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)

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

moshe weitzman’s picture

moshe weitzman commented

FYI, OG now fully implements this feature for both group posts and group nodes. Great stuff.

spelzmann’s picture

spelzmann commented

OG?

PC Pro Schools’s picture

PC Pro Schools commented

"Also added a drupal_alter on the results of the hook, for instance allowing 'automatic nodetitle' module to remove the 'title' row..."

I don't see it...