At the University of Baltimore we saw several users go to edit their content type to try and enable a vocabulary. Currently users cannot enable vocabularies from this page.

Files: 
CommentFileSizeAuthor
#14 vocabulary_node_type_form-394482_3.patch2.94 KBBojhan
Passed: 11660 passes, 0 fails, 0 exceptions
[ View ]
#11 vocabulary_node_type_form-394482_2.patch2.66 KBderhasi
Passed: 11660 passes, 0 fails, 0 exceptions
[ View ]
#11 vocabulary_on_content_type_edit_2.png21.97 KBderhasi
#6 BEFORE patch38.01 KBderhasi
#6 AFTER patch58.54 KBderhasi
#3 vocabulary_node_type_form-394482_1.patch2.73 KBderhasi
Passed: 11660 passes, 0 fails, 0 exceptions
[ View ]
#1 vocabulary_node_type_form-394482.patch2.64 KBderhasi
Passed: 11660 passes, 0 fails, 0 exceptions
[ View ]

Comments

Assigned:Unassigned» derhasi
Status:Active» Needs review
Issue tags:+d7uxsprint
StatusFileSize
new2.64 KB
Passed: 11660 passes, 0 fails, 0 exceptions
[ View ]

Added this functionallity with attached patch.
* additional taxonomy fieldset via taxonomy_form_alter
* additional #submit function saving vocabulary data

What is vocabularies_old for? I don't see it being used in the code so why is there?
I could see it useful if we check that the vocabulary will actually change (it's added now to the node type, or it was removed from the node type) before saving the vocabulary. Otherwise we do a save that doesn't change nothing.

StatusFileSize
new2.73 KB
Passed: 11660 passes, 0 fails, 0 exceptions
[ View ]

tic2000, you are right. That's redundant data. I used it for comparison in my first approach, but don't need it anymore. I changed it, and also moved the form to FORMI-form_alter (taxonomy_form_node_type_form_alter), as the old form was only a pseudo-form-function.

Patch attached.

Before and after screenshot for our non-technical friends?

One minor problem. The documentation standard (at least for now) says that it should be "Implement" not "Implementation of"

StatusFileSize
new58.54 KB
new38.01 KB

Some screens...

Yes - I think we can make the label better, so that we don't need the description.

So instead of simply "Vocabularies", something like...

* Vocabularies for node terms.
* Vocabularies for node tagging.
* Active vocabularies for node term selection.
?

What about.

*Vocabularies enabled on content form.
*Vocabularies enabled on content creation.

I am thinking the last one is the best, but this will need some feedback

Status:Needs review» Needs work

*Vocabularies enabled on content creation.

Let's go with this one for now.

StatusFileSize
new21.97 KB
new2.66 KB
Passed: 11660 passes, 0 fails, 0 exceptions
[ View ]

* Changed to "Vocabularies enabled on content creation"
* removed #description
* Corrected documentation to "Implement ..."

Patch and new screen attached

Status:Needs work» Needs review

The only nitpicking I would have is to change $voc to $vocabulary.

StatusFileSize
new2.94 KB
Passed: 11660 passes, 0 fails, 0 exceptions
[ View ]

Ok, changed it.

Now $vocabularies became $vocabularyabularies.

Jeez, I just replaced wrong. I am sorry, but I fully understand nitpicking - but it doesn't really aid me if you say I made a mistake you can fix by posting a new patch in like 1 minute.

Status:Needs work» Needs review

Hey Bojhan:

This patch mimics the current vocabulary admin form's handling of node types but from the opposite direction in terms of UI context. The latest patch on #412518: Convert taxonomy_node_* related code to use field API + upgrade path shows how I've tried to use the current vocab admin form to trigger creation of field instances, though I admit it is a bit out of date. As soon as #491190: Provide a taxonomy term field type lands, this patch will be easy to rewrite. The submit handler will only need to create or delete field instances instead of saving vocabulary data. That's not important now, but (I hope) it will be important very soon.

We chatted in IRC, so I wanted to reiterate that I appreciate the usability goals of this issue. I'm not raising these other issues to say "this is a bad idea" but rather to say "there are other issues that are immediately relevant." I would be dismayed if term fields landed and Drupal's UX experts and contributors were caught by surprise by the changes it brings.

In the meantime, taxonomy_get_vocabularies() takes a node type argument and returns an identically formatted array. I think this patch could be shorter and avoid some of these local variables if it did something like this:

function taxonomy_form_node_type_form_alter(&$form, $form_state) {
  if (isset($form_state['args'][0]->type)) {
    $node_type = $form_state['args'][0]->type;
    $vocabularies = taxonomy_get_vocabularies();
    // Only show vocabulary fieldset, when vocabularies exist.
    if (count($vocabularies)) {
      $options = array();
      foreach ($vocabularies as $vocabulary) {
        $options[$vocabulary->vid] = $vocabulary->name;
      }
      $form['taxonomy'] = array(
        '#title' => t('Taxonomy'),
        '#type' => 'fieldset',
        '#collapsible' => TRUE,
        '#collapsed' => TRUE,
        '#tree' => TRUE,
      );
      $form['taxonomy']['vocabularies'] = array(
        '#title' => t('Vocabularies enabled on content creation'),
        '#type' => 'checkboxes',
        '#multiple' => TRUE,
        '#default_value' => array_keys(taxonomy_get_vocabularies($node_type)),
        '#options' => $options,
      );
      $form['#submit'][] = 'taxonomy_node_type_form_submit';
    }
  }
}

Status:Needs review» Needs work

Status:Needs review» Needs work

@Bojhan it's 1 minute if you have a clean HEAD, I didn't have one so I just posted my review.

@tic2000 It's oke, just personally got annoyed by the 1 word nitpicking reviews without a patch. No problem. But I think @bangpound is suggesting a completely different direction. Do you have a screenshot bangpound?

Bojhan:

It's not a completely different direction at all. There are just contingencies.

I had two points:

  1. When taxonomy vocabularies are fields (called term fields) on nodes instead of a vague 'taxonomy' property that nodes pick up through nodeapi, the taxonomy_vocabulary_node_type table goes on the chopping block. Field API will become responsible for knowing which vocabularies (expressed as term fields) connect to which node types. We can still implement this usability goal using field API functions which are actually easier to write than these vocabulary object loops and taxonomy_vocabulary_save calls. I linked to some code I've written that already does this with the current Vocab admin UI, and it's even easier to "invert" it for the content type admin form.
  2. Your code can be streamlined. See my use of '#default_values' => array_keys(...).

(one point is unfortunately bigger and wordier than the other, which is why i'm probably not a usability guy.)

So how would this look?

@bangpound:
as the whole vocabularyies are allready loaded via taxonomy_get_vocabularie() and there is allready a foreach, I'd avoid a second loop.

general:
when using taxonomy term via field there could be mutlipe fields using in one content type, that use the same vocabulary ... -is that right?- so we can assign one vocabulary multiple times to a content type, and cannot determine which existing field shall be used or a new field has to be created. So I guess there would not be a opportunity to determine how to link "vocabulary to content type".
Is there allready an approach for that?

derhasi:

That's an important question. We will have the opportunity to make multiple term fields of the same vocabulary on the same node type. However, each new vocabulary will trigger the creation of a new term field, and this "automatic" term field will be the one whose values are used to generated taxonomy/term/[tid] pages. For now, we're looking at using these "automatic" term fields as the well-lit path for "Drupal 6 taxonomy in Drupal 7." These fields provide the familiar "classification" feature of Drupal taxonomy.module.

Like all fields, we can't have multiple term fields instances of the same term field on a node type. Users will have to create new fields for the same vocabulary if they need to use the same vocabulary twice on the same node. Other term fields like that would not/could not be managed by this array of checkboxes.

Status:Needs work» Postponed

Let's postpone this for a few days until Term module is committed. Then we can talk about concrete problems.

I think this problem is an important one to think about because it complicates the way fields are/could be used in Drupal as content, as functions and as architecture.

bangpound: please post a link to the issue you're postponing this on so we may track it's status here as well.

Status:Postponed» Active

Obviously this is active, anyone tried adding a vocabulary? Its pretty complex.

Assigned:derhasi» Unassigned

I'm sorry I could not contribute to this post anymore. At the moment I'm running out of time. I will return in a few weeks, but meanwhile, maybe someone else will assign this issue.

Version:7.x-dev» 8.x-dev