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

Uwe Hermann drupal-devel at drupal.org
Mon Sep 19 00:38:41 UTC 2005


Issue status update for 
http://drupal.org/node/22215
Post a follow up: 
http://drupal.org/project/comments/add/22215

 Project:      Drupal
 Version:      cvs
 Component:    theme system
 Category:     tasks
 Priority:     critical
 Assigned to:  Anonymous
 Reported by:  adrian
 Updated by:   Uwe Hermann
-Status:       patch (code needs review)
+Status:       patch (code needs work)

Patch doesn't apply anymore.




Uwe Hermann



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

Fri, 06 May 2005 23:37:31 +0000 : 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.




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

Sat, 07 May 2005 07:03:38 +0000 : 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.




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

Sat, 07 May 2005 07:17:39 +0000 : 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...




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

Sat, 07 May 2005 08:48:31 +0000 : 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.




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

Sat, 07 May 2005 08:54:50 +0000 : 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.




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

Sat, 07 May 2005 09:14:01 +0000 : 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>




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

Sat, 07 May 2005 09:46:57 +0000 : Bèr Kessels

"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!




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

Sat, 07 May 2005 13:49:13 +0000 : adrian

Dries: I did not say that this code was in any way final. It's a start.


To customize the menu, you need to create another menu container with
menu.module, and then you will be able to select the second menu you
made. Also, the message isn't mine , it's copied verbatim from Bér's
module =)




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

Sat, 07 May 2005 14:28:11 +0000 : Bèr Kessels

hey, don't blame me for copy-pasting bad code ;)




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

Sun, 08 May 2005 16:36:35 +0000 : adrinux

"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.

"
Despite having helped clean up the css in that patch a little I think
it's better just not added. All themes will style these major links,
when does anybody view the menu without a theme enabled? It's yet more
code in drupal.css for theme developers to override.


These links are presented as lists precisely because the degrade well
in the absence of CSS, sure they look ugly, but they still work. We'd
be better off either just using the local task css as suggested, or
adding to the core themes CSS to style these in a theme specific
manner.




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

Mon, 09 May 2005 13:10:03 +0000 : Bèr Kessels

I dont get it: What is all the fuzz abuot this CSS about? m HTML will be
rendered with the *already available* CSS for local tasks. why do we
need additional CSS? Are these local tasks not good enough?




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

Tue, 07 Jun 2005 10:40:40 +0000 : Bèr Kessels

Attachment: http://drupal.org/files/issues/primary_navigation.diff (18.03 KB)

I have to abandon this, I spent way too much time on this already, but
have trouble with the bordering levels. I simply do not manage to get
the proper iterations of the -very complex- menu array. 


Is there anyone who has enough advanced array knowledge, so that he/she
can finish this? If not,we will not have more advanced pirimary menus in
core for 4.7.


You can find the -working- module:
http://cvs.drupal.org/viewcvs/drupal/contributions/modules/primary_links/







More information about the drupal-devel mailing list