[drupal-devel] [task] Integrate primary links with menu navigation

Sat May 7 09:47:04 UTC 2005

Issue status update for http://drupal.org/node/22215

 Project:      Drupal
 Version:      4.6.0
 Component:    theme system
 Category:     tasks
 Priority:     critical
 Assigned to:  Anonymous
 Reported by:  adrian
 Updated by:   Bèr Kessels
 Status:       patch

"This code needs a lot of work.

"
Above all it needs some decisions to be made. Then the best preactices
for these decisions can be chosen, and only then work on the code
should be done!

Please lets not rush this code in, but let us make it future proof and
above all usable. So lets first look for what we want and **only then**
code it. I know about the silver-gold rule. But imo Talk is silver, Code
is gold, but having both the gold AND silver, instead of gold OR silver
makes you the richest.

We must decide on:
* The DOM: do we follow the local tasks (imo not the ideal), or do wee
design a new DOM.
* Nested vs non nested: all listamatic examples (and nearly all oters)
use non neested ULs.
* Empty ULS: I know that a lot of designs will look very ugly or even
break if there is no empty DIV, in place where normally the secondary
rows will show up.
* Optional parameters: an often heard request, for example, is to
render *all* levvels, for DHTML purposes.
* Dependency on menu.module. Adrian kinda convinced me that thats not a
good thing. But still id like to hear other opinions. We could simply
say "primary links use the navigation menu by default. If you want to
change that, you must enable menu.module". [1]
* (How) Do we provide,  the defaults. Drupal should offer sensible,
useable and 60%-of-the-cases-already-correct primary links. IMO >60% of
the users should not have to bother about changing them, but can use the
defaults.

We then must find best preactices:
* Do we use one themable function with more parameters, or rather more
funtions with less parameters. Its  like: menu_items_nested($menu) and
menu_items_unnested($menu) VS. menu_items($menu, $nested = TRUE);
* Where do we put the GUI. It now lives in theme settins, and should
remain there, untill that whole theme settings stuff gets sorted out.
But IMO navigation settings should live with the menu settings.
* Optimise help, names and descriptions. primary links was chose by me,
simply because we have always named them like this. But its not a good
name, at all.
* Optimise code and speed, minimise memory usage, minimise iteration
loops. **we should end with this, not start this. Technology should
follow functionality, never the other way around**

[1] This is the case anyway. The system now uses a menu as "container"
for primary links. By default you have only one menu/block/container:
navigation. If, for example you want Blogs, Forum and My CV in your
primary links, you must create a new menu, called e.g. "Primary Menu".
Menu module will make this a new block too. You then add the three menu
*items* to that menu, possibly with children.
Now, in the current approach you can use the theme settings to set that
"Primary Menu" as your container, for primary links.
Conclusion: for this to make sense you need menu.module anyway!

Bèr Kessels

Previous comments:
------------------------------------------------------------------------

May 7, 2005 - 00:37 : adrian

Attachment: http://drupal.org/files/issues/menu_patch.diff (7.58 KB)

Here is the first version of the patch that uses the menu system to
generate the primary and secondary navigational links.

The navigation menu should by default be used as the source for the
primary / secondary navigation (currently only in bluemarine, templates
need to be modified to use this functionality). 

Furthermore, the css used sucks .. and we will need someone to write
proper default css for us to make it at least presentable in the
absence of theme css.

The navlinks aren't really useful if there are more than 5 links in a
menu, so admin/ looks utterly horrible.

This code is copied and adjusted from Bér's primary links module, and
Adrian Simmons cleaned up the CSS a bit. Bér didn't have any css with
the module, so I had to rip it from his webschuur.com site.

------------------------------------------------------------------------

May 7, 2005 - 08:03 : Bèr Kessels

The way I /intended/ to write this, was that the local task CSS would be
used, unless you theme the function. Or unless you provide your own CSS
to override the local task CSS. For this purpose you had -by default-
to wrap the primary links in another span or div, called something like
"main menu".

But we should nto add more primary links CSS in drupal.css. that file
is already way too cluttered an big. We should just have *one* small
chunk of CSS that handles *all* horizontal LIs.

------------------------------------------------------------------------

May 7, 2005 - 08:17 : stefan nagtegaal

  <div class="list">
    <ul id="primary-links">
      <li class="item $i++"><span>.....</span></li>
    </ul>
  </div>

This makes all themable possibilities possible imo and very easy...
I vote for a markup like this...

------------------------------------------------------------------------

May 7, 2005 - 09:48 : Dries

I tried this patch without looking at the code, but it does not seems to
work.  After clicking around for 5 minutes, I found this little note on
the 'global theme settings page':

Navigation settings
You have only one container. The primary items are automatically set to
use that container.
This message is officially nominated for the "most obscure status
message I've seen in months"-award.  No seriously, I've no idea what to
do with this.  

1. It took me a while to locate the menu setting (message).  Not the
most intuitive place IMO.
2. The message seems to suggest it uses a default container of some
sort.  (What is is 'container'?  What are 'primary items'?  What has
this to do with 'navigation'?)
3. What does this 'container' contain?  It still see 'edit primary
links' and 'edit secondary links'.
4. No further instructions?  (I don't even have the menu module enabled
so I can spend hours looking for 'containers' and 'primary items'.)

The UI and integration need work IMO.

------------------------------------------------------------------------

May 7, 2005 - 09:54 : Dries

Looking at the code, I'd rather not have the system module iterate over
the $menu array.  Ditto for theme.inc -- no way someone is ever going
to theme that function.  The $menu array is supposed to be private to
menu.inc/menu.module.  Don't emit tabs (\t\t).  There is also a TODO in
the code?  

This code needs a lot of work.

------------------------------------------------------------------------

May 7, 2005 - 10:14 : Junyor

Stefan: why not use something like the following?  There's no reason for
the extra DIV and SPAN elements.

  <ul class="list" id="primary-links">
    <li class="item $i++">.....</li>
  </ul>