Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The menu system UI works today! Congrats, compared to my last foray :P
Now, it seems like functionality has been removed compared to Drupal 5:
* ability to set primary and secondary links to arbitrary menus
* ability to set primary and secondary links to the same thing so that secondary links are just the second level of what is under the selected primary
* ability to set "default menu for content" to show all menus
I don't think any of these features should go away.
Comment | File | Size | Author |
---|---|---|---|
#17 | menu-primary-link-selection_6.patch | 7.36 KB | pwolanin |
#13 | menu-primary-link-selection_5a.patch | 7.4 KB | pwolanin |
#12 | menu-primary-link-selection_5.patch | 7.65 KB | pwolanin |
#10 | menu-primary-link-selection_4.patch | 7.09 KB | kkaefer |
#8 | menu-primary-link-selection_3.patch | 7.52 KB | pwolanin |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedPatch please.
Comment #2
kkaefer CreditAttribution: kkaefer commentedI did not add the “Show all menus” item because at the moment, it does not really make sense to select anything here.
Users can’t create menu items otf when they don’t have “administer menu” and if they have, they can see all menus anyway.
Comment #3
kkaefer CreditAttribution: kkaefer commentedChanges as per pwolanin on IRC: Don’t modify
menu_tree_page_data()
and implement the logic inmenu_primary_links()
andmenu_secondary_links()
.Comment #4
kkaefer CreditAttribution: kkaefer commentedCleaned up patch with pwolanin's help. Added some documentation.
Comment #5
kkaefer CreditAttribution: kkaefer commentedLoop is more legible now.
Comment #6
Dries CreditAttribution: Dries commentedNot being able to cache the navigation block is not a show-stopper for this patch and is something we could work on in a follow-up patch. The only way to solve this is by providing a setting in the navigation block's configuration page, or to create a contributed module that alters the cache-field in the blocks table.That or you could edit one line of code. Let's focus on what we have now, and then consider another iteration of improvements. I don't think these improvements will change the API -- they'd merely add configuration settings.
(mis-posted)
Comment #8
pwolanin CreditAttribution: pwolanin commentedper conversation with Dries - make this a theme setting, not a menu module setting.
Comment #9
pwolanin CreditAttribution: pwolanin commentednote, the patch also includes the single-line patch that is here: http://drupal.org/node/166491 so that the global theme settings are useable.
Comment #10
kkaefer CreditAttribution: kkaefer commentedChanged some strings. Functionality-wise it works correct.
Comment #11
merlinofchaos CreditAttribution: merlinofchaos commentedIMO, these are not decisions to be made by the theme layer.
Everything we're talking about is choosing what to present: Choosing which menu should be primary and secondary, etc. That is not the presentation layer, that is logic.
The theme layer should only choose HOW to present the data it's given. It should not be selecting data sources. The biggest choice about the data it should be making is whether or not to present data it is given, and how to present it.
This should remain a menu setting.
Comment #12
pwolanin CreditAttribution: pwolanin commentedPer Merlin's suggestion, as compared to the previous patch, the only setting as to *which* content to use is done in menu module. The theme system has options only to toggle the primary and secondary links on or off.
Comment #13
pwolanin CreditAttribution: pwolanin commentedfix typo and re-roll for this commit: http://drupal.org/node/166491
Comment #14
pwolanin CreditAttribution: pwolanin commentedmake the title reflect the patch.
Comment #15
kkaefer CreditAttribution: kkaefer commentedStill applies, tested it successfully. This is RTBC, we need this in D6.
Comment #16
Gábor HojtsyIs it a current/suggested practice to use "(Optional)" no optional parameters, in parenthesis and not as a leading "Optional ..." word in the sentence?
Comment #17
pwolanin CreditAttribution: pwolanin commentedre-rolled without parentheses around Optional.
Comment #18
Gábor HojtsyOk, looks fine. Committed, thanks!
Comment #19
Jaza CreditAttribution: Jaza commentedErrr... what happened to "ability to set primary and secondary links to arbitrary menus"? That was in the first few versions of this patch, but it seems to have disappeared around #12. Could we put that back, please?
We should also remove the new "Source for the secondary links" option. How did that sneak in? I don't see any discussion about it in this issue. Anyway, it's not as flexible as actually setting the menu for the secondary links, so I see it as a step backward. Let's get rid of it, and put the old options back, which weren't so limiting.
Comment #20
Jaza CreditAttribution: Jaza commentedI also created http://drupal.org/node/170322 (having 'proper' menus for books) which is related to this issue.
Comment #21
pwolanin CreditAttribution: pwolanin commented@Jaza - well, the KISS principle suggests that assigning arbitrary menus to Primary/Secondary is not really needed.
What's the burning, common use case?
Comment #22
Jaza CreditAttribution: Jaza commented@pwolanin: see http://drupal.org/node/170322 for a use case (not 'burning', I admit, but it's there).
Comment #23
pwolanin CreditAttribution: pwolanin commented@Jaza - that issue seems somewhat orthogonal, unless you are saying that you want a book to be your primary links?
Comment #24
pwolanin CreditAttribution: pwolanin commentedin the absence of a compelling use case from Jaza, I'm satisfied with what we have already.
Comment #25
wwmv CreditAttribution: wwmv commentedHi.
if you set "source for secondary links = primary links" and activate the "secondary links box" you get an empty box because links from the menu called "secondary-links" are shown.
I changed the following in function menu_block($op, $delta) to get it working:
hope this is the right place. i'm not sure if it is the right way to do....
Comment #26
Boris Mann CreditAttribution: Boris Mann commentedUse case for arbitrary menus: I create a whole new menu and then want to set it to be primary, instead of manually fiddling with primary in real time. But I'll leave that horse for now and just note that this is re-opened with the bug fix code listed.
Comment #27
pwolanin CreditAttribution: pwolanin commented@markus-v-drupal: this should not be part of this issue. A new issue is here: http://drupal.org/node/180786 please post follow-ups there.
@Boris: I still don't think assigning arbitrary menus is a good idea. I'd be much more supportive of adding utility function/admin page to allow two menus to be swapped. Or that could be a contrib module.
Comment #28
catchComment #29
David_Rothstein CreditAttribution: David_Rothstein commentedThis was fixed in 6.x as a result of http://drupal.org/node/216813
Comment #30
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.