Kind of a followup to #409750: Overhaul and extend node build modes:
Just like we transformed $teaser into $build_mode in node_view() and friends, we should add a $build_mode param to the user and term buidling workflow.

Reason: even if user and term entities currently only expose one build mode ('full'), there's no guarantee it will stay that way before we ship, and even if it does, build modes are extensible from contrib anyway - only currently, adding a build mode for either terms or users is useless, because there's no way to specify a build mode for a displayed user or term.

For users, it means adding a $build_mode param to user_build_content() and related hooks: hook_user_view(), hook_profile_alter()
(note: functions and hooks names consistency is terrible - not to be solved in this thread, though)

For terms, it's a little more tricky, there's no taxonomy_term_build() API function, so it needs to be introduced...

I'd be thrilled if someone took the lead on this ;-)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

subscribing

Required for #498694: D7 core patch? (User Display API in core).

sun’s picture

Now that I thought about it some more (in the other issue), a $build_mode param for users would only make sense to force a special, known-to-exist build mode for rendering a user. I am able to foresee a few hard-coded build modes:

- 'full': The full user account/profile page content.

- 'teaser' or 'name': A username.

- 'picture': A user picture.

Eliminating theme('username') and theme('user_picture') by replacing it with theme('user', $account, $build_mode = 'name') ...

However, since modules output users in all kind of locations (not necessarily in views, blocks, or nodes), I don't think this really buys us something. To achieve the flexibility of User Display API (see issue above), we would pass a "class" only and allow the site administrator to configure which class should use what build mode.

yched’s picture

"a $build_mode param for users would only make sense to force a special, known-to-exist build mode"
Which could be defined by core (currently only 'full') or by contrib. We cannot reason only on what core currently uses.
Since Views can list users, having alternate build modes is precious.

To provide an example I already used in other places, a contrib module that lets admins add custom build modes would be both super easy and super useful (this is also true in D6 land, BTW...)

I'm not sure I get your point, but what i mean is that having user display support build modes has use cases aside from User Display in core (which would be awesome).
I'd even say that, since available build modes are extensible from contrib, 'supporting build modes' is a part of the 'being fieldable' contract, even if the entity itself doesn't actually uses them in its base code.

moshe weitzman’s picture

theme('user', $account, $build_mode = 'name'). this is mixed up. The build mode is for *building*, not theming. It would be user_build($user, $build_mode). See node_build() for a working example. I think we can just punt on which user build modes to introduce. Leave that for contrib. As yched says, we just need to assure that build_mode is supported for users.

Not at all sure what build modes are needed for terms. Maybe for vocabulary sharing or some odd case like that.

yched’s picture

re moshe #4: "Not at all sure what build modes are needed for terms"
example: list of terms in a side block as a navigational element - I want the term name + description + the picture.
More generally, anything that's 'listable' (Views) has use cases for build modes - of course, you can use 'Field'-style Views for this, but they're harder to theme and reuse, and with Field cache and multiple load, they are not even really faster in simple cases.

But, true, I'm not sure what could be *core* use cases for term build modes. Ni big deal, time will tell :-)

yched’s picture

Title: add $build_mode param to user and term display » add $build_mode param to term display
Component: field system » taxonomy.module
Priority: Normal » Critical

Bumping, + moving this one to taxonomy.module and a twin to user.module

yched’s picture

Additionally, it would be really good to move term display to a template.

sun’s picture

Issue tags: +Fields in Core, +API clean-up

Introducing a new tag for feature freeze: API clean-up.

sun.core’s picture

Priority: Critical » Normal

ugh... this got lost in nirvana... taxonomy terms are now fieldable, but yet, there's no view function like for other entities? That's totally awkward.

Any takers for Jan 15th?

yched’s picture

Priority: Normal » Critical
Status: Active » Needs review
FileSize
4.28 KB

Let's do this.

This patch is needed anyway to have non-field components ('name' and 'description') display nicely with Field API reorderable fields in 'term edit' forms and on displayed terms.
Bumping back to critical, because reordering fields on terms is broken without this.

A term.tpl.php would be cool, but I don't think it is required to have this patch land.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

Looks perfectly small and reasonable.

Dries’s picture

Screenshot?

yched’s picture

Turns out we do need a template.
'Term name' behaves exactly like 'node title': when we're on the taxonomy/term/% page for the term, we want the term name out because it's the page title.
OTOH, when people will want to use taxonomy_term_view() in other view modes, they will need the title in.

So attached patch provides a taxonomy-term.tpl.php and template_preprocess_taxonomy_term().
Largely derived and simplified from node.

Screenshot of taxo/term/%tid listing, for a term with a 'multiple' field:
But there's actually no visual difference with HEAD. The difference is in the markup.

Before:

<div class="content"> <!-- The 'Content' region -->
  <div class="field-name-field-multiple">
    ...
  </div>
  <div class="taxonomy-term-description">
    ...
  </div>
  <div id="node-1">
    ...
  </div>
  <div id="node-2">
    ...
  </div>
</div>

After:

<div class="content"> <!-- The 'Content' region -->
  <div class="term-listing-heading">
    <!-- term.tpl.php -->
    <div id="taxonomy-term-1" class="taxonomy_term">
      <div class="content">
        <div class="field-name-field-multiple">
          ...
        </div>
        <div class="taxonomy-term-description">
          ...
        </div>
      </div>
    </div>
    <!-- end of term.tpl.php -->
  </div>
  <div id="node-1">
    ...
  </div>
  <div id="node-2">
    ...
  </div>
</div>
yched’s picture

Status: Reviewed & tested by the community » Needs review

And of course, other difference is that the 'description', which escaped the 'as field' treatment in D7, plays nicely with Field UI d-n-d field reorder.

yched’s picture

Patch #13 removed CSS styling for .taxonomy-term-description, but we actually need it when there's no other field in the term (er, which remains the most common case...)

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

back to rtbc

Status: Reviewed & tested by the community » Needs review
Issue tags: -Fields in Core, -API clean-up

Re-test of taxo_term_view-499192-15.patch from comment #15 was requested by webchick.

Status: Needs review » Needs work
Issue tags: +Fields in Core, +API clean-up

The last submitted patch, taxo_term_view-499192-15.patch, failed testing.

yched’s picture

Status: Needs work » Needs review
FileSize
8.48 KB

Reroll after #636992: Entity loading needs protection from infinite recursion :
- Moves the recently added entity_prepare_view() call from taxonomy_term_page() menu callback to taxonomy_term_view() API func (just like field_attach_prepare_view())
- Uses the new '__' convention for template suggestions.

I'm away from my usual setup, so no "diff -uP" patch this time. Regular Eclipse-generated patch, no context info. See the patch in #15 if needed, it's the same patch apart from the updates above.

Also, reminder: the patch adds a file (taxonomy-term.tpl.php)

moshe weitzman’s picture

Status: Needs review » Needs work
+++ modules/taxonomy/taxonomy.pages.inc	14 Jan 2010 23:55:56 -0000
@@ -28,18 +28,13 @@
+    '#prefix' => '<div class="term-lising-heading">',

typo - lising

+++ modules/taxonomy/taxonomy.module	14 Jan 2010 23:55:55 -0000
@@ -561,6 +589,83 @@
+  // Make the field variables available with the appropriate language.

what do we mean by language here? as in locale module? please clarify.

+++ modules/taxonomy/taxonomy.module	14 Jan 2010 23:55:55 -0000
@@ -561,6 +589,83 @@
+
+  // Clean up name so there are no underscores.
+  $variables['template_files'][] = 'taxonomy-term__' . $vocabulary_name_css;
+  $variables['template_files'][] = 'taxonomy-term__' . $term->tid;

$variables['theme_hook_suggestions'] instead of template_files. See #678714: Unify use of theme hook / template suggestions, fix clobbering problems, and improve suggestion discovery performance

Powered by Dreditor.

yched’s picture

Status: Needs work » Needs review
FileSize
8.68 KB

Updated for the remarks in #20.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

Thanks yched. RTBC.

WTGBC (wait for green before commit)

catch’s picture

Completely missed this one, looks great though!

yched’s picture

#678590: Path alias link is below submit button on taxonomy term add has just put back the 'Relations' fieldset and 'URL alias' input in the middle of the form. We'll need to decide what we do with those (reorderable through hook_field_extra_fields(), or just sink at the bottom).

See my comment in #678590-7: Path alias link is below submit button on taxonomy term add.

yched’s picture

Title: add $build_mode param to term display » Fix display and forms for "Fieldable terms"

+ retitle. The scope of the patch is more than $view_mode now.

yched’s picture

Bump ?
Patch still applies and works as expected.

hefox’s picture

   $build['#attached']['css'] = array(drupal_get_path('module', 'taxonomy') . '/taxonomy.css');

as far as I can tell, this is overriding field.css attached via field_attach_view... with the odd side effect of making inline field labels not appear as inline (and not bold-ed either)!

Manually edited the patch to change it to [], /cross fingers

yched’s picture

Good catch. This didn't show up on a 'taxo/term/%tid' listing page with actual nodes, because the fields in the nodes took care of loading field.css.

webchick’s picture

Priority: Critical » Normal
Status: Reviewed & tested by the community » Needs work
Issue tags: -API clean-up +Needs documentation

Overall this looks sane to me. Since there are no visual differences to speak of, and this is fixing a critical problem in the API, committed to HEAD, along with PHPDoc on template_preprocess_taxonomy_term().

We need to document at least the existence of that new *.tpl.php file.

yched’s picture

yched’s picture

And sorry for the missing PHPDoc on template_preprocess_taxonomy_term(). Thks Angie.

Status: Fixed » Closed (fixed)

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

studio77’s picture

I think that in the screenshot, it looks great!