Issue status update for http://drupal.org/node/22215 Project: Drupal Version: cvs Component: theme system Category: tasks Priority: critical Assigned to: Anonymous Reported by: adrian Updated by: Bèr Kessels Status: patch 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/ 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> ------------------------------------------------------------------------ May 7, 2005 - 10:46 : 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! ------------------------------------------------------------------------ May 7, 2005 - 14:49 : 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 =) ------------------------------------------------------------------------ May 7, 2005 - 15:28 : Bèr Kessels hey, don't blame me for copy-pasting bad code ;) ------------------------------------------------------------------------ May 8, 2005 - 17:36 : 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. ------------------------------------------------------------------------ May 9, 2005 - 14:10 : 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?