Spin-off from #576916: Default menu "Parent item" should be Navigation, not Primary links after discussion with Bojhan in IRC:

The current list of menus is overwhelming and provides absolutely no clue about the purpose and location of any menu:

  • Administration shortcuts
    The Admininstration shortcuts menu contains commonly used links for administrative tasks.
  • Main menu
    The Main menu is the default source for the Main links which are often used by themes to show the major sections of a site.
  • Management
    The Management menu contains links for content creation, structure, user management, and similar site activities.
  • Navigation
    The Navigation menu contains links such as Recent posts (if the Tracker module is enabled). Non-administrative links are added to this menu by default by modules.
  • Secondary menu
    The Secondary menu is the default source for the Secondary links which are often used for legal notices, contact details, and other navigation items that play a lesser role than the Main links.
  • User menu
    The User menu contains links related to the user's account, as well as the 'Log out' link.

We want to enable users to get at least the purpose, exploring location based menu's in our discussion we found its nearly impossible to achieve this. Users often get confused about the label "menu" as they are already browsing menu's and want to find the main navigation links.

  • Main menu -> Main navigation
    Use this for linking to the main site sections.
  • Management -> Administration
    Contains links to administrative tasks.
  • Navigation -> Tools
    Contains links for site visitors. Some modules add their links to this menu.
  • User menu -> Account links
    Links related to the user account.

The solution proposed here is not perfect, but it is a step in that direction. We intend to improve on Tools and Administration, spliting out the dumping ground, and finding a good location for a often one link menu :)

Files: 
CommentFileSizeAuthor
#75 drupal8.menu-rename-update.75.patch9 KBsun
PASSED: [[SimpleTest]]: [MySQL] 46,231 pass(es).
[ View ]
#73 drupal8.menu-rename-update.73.patch8.01 KBsun
PASSED: [[SimpleTest]]: [MySQL] 46,228 pass(es).
[ View ]
#72 drupal8.menu-rename-update.72.patch3.24 KBsun
FAILED: [[SimpleTest]]: [MySQL] 46,114 pass(es), 108 fail(s), and 7 exception(s).
[ View ]
#71 drupal8.menu-rename-update.71.patch1.83 KBsun
FAILED: [[SimpleTest]]: [MySQL] 45,903 pass(es), 13 fail(s), and 13 exception(s).
[ View ]
#68 menu-594660-68.patch46.87 KBdagmar
PASSED: [[SimpleTest]]: [MySQL] 46,224 pass(es).
[ View ]
#68 interdiff.txt1.1 KBdagmar
#66 menu-594660-66.patch45.77 KBno_commit_credit
FAILED: [[SimpleTest]]: [MySQL] 46,198 pass(es), 5 fail(s), and 0 exception(s).
[ View ]
#66 interdiff.txt5.41 KBno_commit_credit
#63 platform.menus_.63.patch45.46 KBsun
PASSED: [[SimpleTest]]: [MySQL] 42,255 pass(es).
[ View ]
#61 platform.menus_.61.patch45.54 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch platform.menus_.61.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#61 interdiff.txt1.55 KBsun
#60 platform.menus_.60.patch44.36 KBsun
PASSED: [[SimpleTest]]: [MySQL] 42,184 pass(es).
[ View ]
#60 interdiff.txt6.15 KBsun
#59 platform.menus_.59.patch46.11 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch platform.menus_.59.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#59 interdiff.txt24.42 KBsun
#57 platform.menus_.57.patch57.69 KBsun
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/lib/Drupal/system/Tests/Menu/TreeOutputTest.php.
[ View ]
#53 rename_menus-594660-53.patch60.36 KBdagmar
PASSED: [[SimpleTest]]: [MySQL] 41,427 pass(es).
[ View ]
#51 rename_menus-594660-51.patch59.25 KBdagmar
FAILED: [[SimpleTest]]: [MySQL] 41,429 pass(es), 8 fail(s), and 0 exception(s).
[ View ]
#50 rename_menus-594660-50.patch57.6 KBdagmar
FAILED: [[SimpleTest]]: [MySQL] 41,435 pass(es), 8 fail(s), and 0 exception(s).
[ View ]
#45 rename_menus-594660-45.patch58.47 KBjenlampton
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch rename_menus-594660-45.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#43 rename_menus-594660-42.patch58.47 KBjenlampton
PASSED: [[SimpleTest]]: [MySQL] 36,814 pass(es).
[ View ]
#31 platform.menus_.31.patch58.52 KBsun
PASSED: [[SimpleTest]]: [MySQL] 36,827 pass(es).
[ View ]
#29 platform.menus_.29.patch57.51 KBsun
FAILED: [[SimpleTest]]: [MySQL] 36,815 pass(es), 7 fail(s), and 0 exception(s).
[ View ]
#28 drupal8.menus-rename.28.patch47.45 KBsun
FAILED: [[SimpleTest]]: [MySQL] 36,758 pass(es), 75 fail(s), and 30 exception(s).
[ View ]
#26 change_menu_names-594660-26.patch62.69 KBjenlampton
FAILED: [[SimpleTest]]: [MySQL] 35,653 pass(es), 26 fail(s), and 0 exception(s).
[ View ]
#24 rename_menus-594660-24.patch47.49 KBjenlampton
FAILED: [[SimpleTest]]: [MySQL] 35,648 pass(es), 51 fail(s), and 0 exception(s).
[ View ]
#20 2toolboxesin1.png44.5 KByoroy
#16 594660-15-menu-names.patch382.66 KByoroy
FAILED: [[SimpleTest]]: [MySQL] 34,320 pass(es), 60 fail(s), and 0 exception(es).
[ View ]
#12 Navigation and Management menus.png25.26 KByoroy
#12 menus-before.png91.86 KByoroy
#12 594660-after-1.png78.2 KByoroy
#12 594660-12-rename-menus.patch40.61 KByoroy
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 594660-12-rename-menus.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Comments

Bojhan’s picture

After having watched many users, I understand the confusion that the lack of context these labels bring is a critical issue. We changed the menu's around - the testing have shown that the administrative / site divide of navigation worked well, but people are still confused where a menu its link would be shown.

I believe although this solution might have its complexities, from a user perspective it solves a lot of issues regarding orientation. We have been able to do it for blocks, and I believe it should be just as possible for menu's since the same arguments stand.

The menu vs. navigation argument is fine, I don't favor either - nor do I think research will choose a side here. But looking at those labels amongst the other menu's I believe that "navigation" creates more direction and perspective - then the menu's would.

Jarek Foksa’s picture

Why not completely remove "Administration shortcuts", "Management" and "User" menus? Those duplicate the functionality already provided by admin toolbar.

jix_’s picture

#2: The admin toolbar is not necessarily enabled on every site. The menus are fallbacks.

Also, I think that the name of a region should not be used as a menu name (like "footer navigation"), because it's not always in that particular region. It differs from theme to theme.

We'd best try to stick with names that describe the type of content it links to. For example, "main menu" links to, well, the most important content. "Secondary menu" links to less important content like disclaimers.

I personally don't think the currently used names are that bad, with exception of the one called "navigation". That one seems to have become sort of a trash can for uncategorized links.

sun’s picture

Also, I think that the name of a region should not be used as a menu name (like "footer navigation"), because it's not always in that particular region. It differs from theme to theme.

The goal is to provide some reasonable default menus in the default installation profile for the 90-95-99% use-case. I've yet to see a web site that has no links in the footer... that's almost 100%, and I've heard from many users that they first try to find certain links in the footer nowadays. (such as "contact", "legal notice", "privacy policy", etc.)

We'd best try to stick with names that describe the type of content it links to. For example, "main menu" links to, well, the most important content. "Secondary menu" links to less important content like disclaimers.

The problem with that is: No one knows the concept you are trying to describe here. Not even I understand it. Each time I look at those menu names, they just don't make any sense at all.

If I'd start building a site off D7 today, one of the first things I'd do is to rename those menus to something that makes sense for the users that have to manage the links in those menus.

As can be seen from this issue's title: this is about the default installation profile.

If your site or site's theme doesn't belong into the very large majority of sites that have a "top", "footer", and "main navigation" menu, then you just delete it, or rename it, or you didn't use the default installation profile in the first place. :P

jix_’s picture

For example, Garland has it's secondary menu in the header if I'm not mistaken. If it would be called "footer navigation" that would be confusing to users as well.

I do agree that the location of a menu as it's name is easier to understand for users, but only if they're always at that exact location. At least in all core themes.

But in that approach, "user menu" would be an inconsistency. That should then be called something like "Top right navigation", but yet again it's not always at the top right.

paddy_deburca’s picture

The problem as I see it is that the names are usage dependent. Some sites may put the top navigation in the footer and a main navigation somewhere else equally un-foreseen.

The flip side of the coin, is that these menus need names - and all sites will not use all these names in the same way all the time.

I would suggest names that are consistent with the default themes/layouts in D7 - that by default they make sense. Sites that choose to move top navigation to elsewhere on the web page should also rename the menu to reflect that change.

So, good and logical defaults and the possibility to change if and when necessary.

LeeHunter’s picture

In the existing text, I don't know what the difference is between "Main Menu" and "Navigation" and I'm left asking myself "Why is one item called a "menu" and the other is not? Is that significant? Are they both menus?"

The proposed example is a little better in some ways because at least the naming is more consistent (everything's called "navigation") but I'm confused by "Top navigation" versus "Main navigation". It's not obvious to me what the difference would be between "top" and "main". The description for Top Navigation simply says "Links in the upper area of the site" and I'm not sure what that means (I often see ALL the links in the top area or ALL the links on the side) and it's not clear why two menus are named by location (top and footer) and one by function (main).

How about a completely neutral and consistent scheme: "Menu 1", "Menu 2", and "Menu 3." Then in the description we can say, for example,
"In default themes, Menu 1 is generally used to provide links for major sections of a site and is often displayed at the top of the page."
"In default themes, Menu 2 is generally used to provide primary navigation links and is often displayed at the side of the page. Modules may add non-administrative links to this menu by default."
"In default themes, Menu 3 is generally used to display minor links such as legal notices and is often displayed at the bottom of the page."

Bojhan’s picture

Version:7.x-dev» 8.x-dev

It seems the approach described although admirable, needs quite some thinking and possibly refactoring - so moving this to Drupal 8.

catch’s picture

Priority:Critical» Major

Downgrading all D8 criticals to major per http://drupal.org/node/45111

jenlampton’s picture

I'd also like to point out that the "Navigation" menu is as of D7, in no way, navigation. So it needs a new name :)

quicksketch’s picture

Category:bug» task

It looks like if #1050616: Figure out backport workflow from Drupal 8 to Drupal 7 goes through, critical and major bugs will actually prevent new development on Drupal core. Considering this isn't actually functional bug at all, I'm moving this to a task. I also want to see where this issue heads, the menu names (and the quantity of menus) is just ridiculous.

yoroy’s picture

Status:Active» Needs review
StatusFileSize
new40.61 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 594660-12-rename-menus.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
new78.2 KB
new91.86 KB
new25.26 KB

So looking at a fresh d8, all modules enabled, I only see these menus:

* Main menu
The Main menu is used on many sites to show the major sections of the site, often in a top navigation bar.

* Management
The Management menu contains links for administrative tasks.

* Navigation
The Navigation menu contains links intended for site visitors. Links are added to the Navigation menu automatically by some modules.

* User menu
The User menu contains links related to the user's account, as well as the 'Log out' link. I'm assuming the 'Administrative shortcuts'

Particularly missing 'Secondary menu' compared to the original post.

1. I assume 'Administration shortcuts' is a special case menu used by the Shortcuts module?

'Secondary menu' simply not existing anymore is already a win :) (Possibly we should then consider removing the relatively obscure 'use foo for main, use bar for secondary functionality found in /admin/structure/menu/settings, but that's another issue)

---

screengrab of Navigation and Management menus with all core modules enabled

So the Management menu is the fallback menu for when not using toolbar/shortcut. The Navigation menu is the default container for new links that may be used by site visitors.

2. I suspect merging these two has been discussed before but runs into access checking issues?
3. It seems that the 'Management' menu never gets more than that one 'Administration' link added to it, can we verify that?

---

As for a naming scheme, we should provide sensible defaults and thus name things as specifically as possible. Everything can be overridden, so if in some specific use case the default label doesn't make sense, that only helps trigger people to find a way to change it. (favorite example for this is the default Wordpress site slogan 'Just another Wordpress blog'. You bet people will try and find the place where they can change that!)

All that said, I had a go at a rewrite of the existing labels and descriptions.

Before:
before screenshotAfter:
after screenshot

The new labels and descriptions in this patch are:

Account links – Links related to the user account.
Administration – Contains links to administrative tasks.
Main navigation – Use this for linking to the main site sections.
My menu – Contains links for site visitors. Some modules add their links here.

'My menu' is totally not very nice, but I wanted to get some git practise going on. Feedback especially on question 2 and 3 would be helpful.

Status:Needs review» Needs work

The last submitted patch, 594660-12-rename-menus.patch, failed testing.

yoroy’s picture

:( Seems my notes for how to create a patch are not correct.

Niklas Fiekas’s picture

It says:

...
Output: [error: cannot apply binary patch to 'core/modules/simpletest/tests/upgrade/drupal-7.bare.database.php.gz' without full index line
error: core/modules/simpletest/tests/upgrade/drupal-7.bare.database.php.gz: patch does not apply
error: cannot apply binary patch to 'core/modules/simpletest/tests/upgrade/drupal-7.filled.database.php.gz' without full index line
error: core/modules/simpletest/tests/upgrade/drupal-7.filled.database.php.gz: patch does not apply].
...

Try git diff --binary --full-index, if you have changed those files.

yoroy’s picture

Status:Needs work» Needs review
StatusFileSize
new382.66 KB
FAILED: [[SimpleTest]]: [MySQL] 34,320 pass(es), 60 fail(s), and 0 exception(es).
[ View ]

Thanks, that certainly makes for an interesting looking patch. Following your suggestion, lets see what happens.

Status:Needs review» Needs work

The last submitted patch, 594660-15-menu-names.patch, failed testing.

sun’s picture

The patch changes too many instances of "navigation" into "my-menu".

'Secondary menu' simply not existing anymore is already a win

I wouldn't necessarily call that a win -- as outlined in the issue summary, the 90%+ purpose of this menu was to use it as a footer menu, which on its own is most probably required for more than 80% of all sites built with Drupal. I'd like to introduce a "Footer navigation" or "Footer links".

(Possibly we should then consider removing the relatively obscure 'use foo for main, use bar for secondary functionality found in /admin/structure/menu/settings, but that's another issue

Those menu settings will only be obsolete after #474004: Add options to system menu block so primary and secondary menus can be blocks rather than variables

2. I suspect merging these two has been discussed before but runs into access checking issues?

Management and Navigation was one menu before; we deliberately separated them on purpose for D7, since "Functional navigation for site users/visitors" and "Administrative tools" are completely different tasks having entirely different audiences.

In light of that, "My menu" doesn't really make sense to me. The menu may not necessarily contain "stuff related to you" (as in "my"), that would be the purpose of the User menu.

In reality, the Navigation menu is just simply the default dumping ground for any kind of links to (unique) pages that modules may provide. This ranges from Filter module's filter tips (manual page), onto Aggregator module's aggregated feed items page, onto Node module's "Create content" page(s), to Flag module's default view of "My bookmarks". In that sense, a generic name like "Tools" would make more sense.

3. It seems that the 'Management' menu never gets more than that one 'Administration' link added to it, can we verify that?

There is no technical limitation or enforcement for that, but by default, the Management menu only contains the "Administration" link on the top-level. There's another issue in the queue that wants to remove the additional "Administration" level/layer on the top-level, but can't find it right now.

Renaming the "Management" menu to "Administration" should be left to that issue.

The only parts that I like in this patch are the renames to "Account links" and "Main navigation", although both could still be a little more concise - e.g., "User account links" might make it more intuitive that this menu only appears when you're logged in [although we should actually move a "Login" link in there IMO].

yoroy’s picture

Thanks! And yes, 'Account links' and 'Main navigation' are the best bits in here, the rest is mostly thinking out loud :)

Re: no secondary links – I agree that it is useful for many sites. But maybe we can leave that as an exercise for the user? Fewer initial menus gives clarity of choice, we can use the footermenu use case as a trigger for discovery of the 'create your own' functionality.

For account links: adding a login would be sweet. Don't think 'User' substantially adds meaning.

2. 'My menu' was totally a cop out indeed. I like the 'tools' word. I think I remember some of that discussion around seperating them now :) I don't remember what I argued for then, but I suspect 'Administration' is just another word for 'Tools' in many a mental model. I do understand the challenge lies in having to provide this default link dump container for modules. A visual treatment that groups these two menus into a single toolbox is worth exploring here I think.

3. Great, removing that layer would help a lot already, happy to discuss further when we find the link :)

Seems to me especially number 2 needs more discussion in here. Happy to continue account link changes in a seperate issue.

yoroy’s picture

StatusFileSize
new44.5 KB

mockup with 2 menus grouped into 1 container

The boxes on the left are about naming the menus from the admin ui perspective
The sketch on the right is a possible front-end perspective. If these would still be two blocks combined, than neither of them shows a block title, but each emphasises the first actionable link. For the site tools that would be 'Create content' (ideally as a funky dropdown that lets you shortcut to the form for a specific content type). For admin, we could choose to make /admin, /content or a dashboard the one most prominent link. Here I went for a prominent /admin link with three additional regular links to content, structure and configuration.

sun’s picture

mmm, #20 might be interesting, but also very debatable in terms of Drupal use-cases, and essentially boils down to allowing a (menu) block's title to be a link on its own (which comes with many questions/problems/consequences, such as, what if I want to rename the block title?). That's definitely a discussion for a separate issue.

btw, #273137: Split Navigation to User and Administration menu was the issue in which we split Navigation into Management. And closely related: #408160: Normal users should not see the create content link appear by default in a menu called "Management"

Still unable to find the issue about turning Management into Administration; i.e., removing the top-level link.

yoroy’s picture

Oh, I was imagining the block title was and there was a li.first a {} bit of styling applied to make the first actual menu item pop out.

Thanks for the back links, useful.

Bojhan’s picture

Still like all the improvements in this thread.

jenlampton’s picture

Status:Needs work» Needs review
StatusFileSize
new47.49 KB
FAILED: [[SimpleTest]]: [MySQL] 35,648 pass(es), 51 fail(s), and 0 exception(s).
[ View ]

rerolled the patch, also added some docs updates.

Status:Needs review» Needs work

The last submitted patch, rename_menus-594660-24.patch, failed testing.

jenlampton’s picture

Status:Needs work» Needs review
StatusFileSize
new62.69 KB
FAILED: [[SimpleTest]]: [MySQL] 35,653 pass(es), 26 fail(s), and 0 exception(s).
[ View ]

trying again

Status:Needs review» Needs work

The last submitted patch, change_menu_names-594660-26.patch, failed testing.

sun’s picture

Component:base system» menu system
Status:Needs work» Needs review
Issue tags:+Platform Initiative
StatusFileSize
new47.45 KB
FAILED: [[SimpleTest]]: [MySQL] 36,758 pass(es), 75 fail(s), and 30 exception(s).
[ View ]

Re-rolled against HEAD, incorporating the latest issue summary (which is a result of a IRC discussion, updated by @Bojhan).

Let's see what fails. Didn't really review the code/changes.

sun’s picture

StatusFileSize
new57.51 KB
FAILED: [[SimpleTest]]: [MySQL] 36,815 pass(es), 7 fail(s), and 0 exception(s).
[ View ]

Fixed stale instance of old menu names and labels.

Also, officially turning this into a platform patch + sandbox branch.

Status:Needs review» Needs work

The last submitted patch, platform.menus_.29.patch, failed testing.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new58.52 KB
PASSED: [[SimpleTest]]: [MySQL] 36,827 pass(es).
[ View ]

Fixed the remaining test failures.

Bojhan’s picture

Status:Needs review» Reviewed & tested by the community

It's a big patch, but because its only changing labels I am comfortable putting this at RTBC.

sun’s picture

Title:Rename menus in default profile» Rename default menu names
Assigned:Unassigned» sun
Status:Reviewed & tested by the community» Needs review

Apparently, this has nothing to do with "default profile". ;)

sun’s picture

Status:Needs review» Reviewed & tested by the community

Unfortunate x-post ;)

neclimdul’s picture

I don't see any reference here but has anyone voiced any concern about account have other meanings? I try to avoid using it when talking to people in businesses because in their world accounts refer to business arrangements or business entities. This is confusing on sites with Commerce solutions or CRM integration or both.

Leaving RTBC because we're talking custom menu's that can be removed/changed in this patch but I wanted to make sure the perspective was on the issue.

meba’s picture

Agree with neclimdul, Account links might be confusing if one has Commerce installed.

neclimdul’s picture

Just so I'm clear, I think its pretty common that when a site visitor see's "Account" that refers to their profile, settings, etc. This is pretty consistent with the internet at large(well not facebook but...). We just need to be aware and careful when using it in the admin interface especially in things like help text that are hard to change because it can have other meanings.

catch’s picture

Status:Reviewed & tested by the community» Needs review

Account doesn't bother me much, but 'Account links' does a bit. A menu is a list of links, why do we have to add 'links' to it?

I just checked twitter to see if it uses account, and it doesn't, uses 'profile' for most things, then other stuff doesn't have a general heading (just an icon with no title attribute). I don't think we can use 'profile' here though since that's too specific.

bleen18’s picture

Oh please, oh please can we revisit the idea of having a "footer menu" (footer links?) by default. I've done a number of intro classes with people and nearly every time I get to the "working with menus" section I get two things:

  • confusion about the difference between navigation & main menu (this issue already addresses that)
  • some form of the question "what about a menu for the footer"

I understand yoroy's point here:

Fewer initial menus gives clarity of choice, we can use the footermenu use case as a trigger for discovery of the 'create your own' functionality.

But it's used by sooooo many sites and we should absolutely reconsider including it by default.

sun’s picture

@bleen18: Yes, we discussed that, and I'd like to see that, too. However, a Footer menu would in fact be a new default menu and not an existing one, and we possibly want to look into various implementation options, too. Therefore, we concluded that a Footer menu should be added in a separate issue. In short: add vs. rename ;)

sun’s picture

Status:Needs review» Needs work

@Bojhan / @yoroy: Any ideas for a better "Account links" label?

As soon as we've figured that out, renaming the internal name and the label should be a simple in-patch-file-rename. The longer we wait, the greater the chance that the patch won't apply anymore though.

jenlampton’s picture

Status:Needs work» Needs review
StatusFileSize
new58.47 KB
PASSED: [[SimpleTest]]: [MySQL] 36,814 pass(es).
[ View ]

We use 'menu' in other places (My menu) can we call it the 'Account menu' to be consistent?

bleen18’s picture

What about calling it User Account Menu? It still uses "account" but includes context...

jenlampton’s picture

Issue tags:+consistency
StatusFileSize
new58.47 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch rename_menus-594660-45.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
sun’s picture

Status:Needs review» Needs work
Issue tags:+Usability, +consistency, +Platform Initiative

The last submitted patch, rename_menus-594660-45.patch, failed testing.

tim.plunkett’s picture

Is this still a major (blocking) task?

sun’s picture

Assigned:sun» Unassigned
Priority:Major» Normal

Nope, this shouldn't block other patches from landing.

That said, we're really close to getting this done... the patch "just" needs to be re-rolled against latest HEAD once more. Any takers?

dagmar’s picture

Status:Needs work» Needs review
StatusFileSize
new57.6 KB
FAILED: [[SimpleTest]]: [MySQL] 41,435 pass(es), 8 fail(s), and 0 exception(s).
[ View ]

Re-rolled

dagmar’s picture

StatusFileSize
new59.25 KB
FAILED: [[SimpleTest]]: [MySQL] 41,429 pass(es), 8 fail(s), and 0 exception(s).
[ View ]

I forgot to save a file.

Status:Needs review» Needs work

The last submitted patch, rename_menus-594660-51.patch, failed testing.

dagmar’s picture

Status:Needs work» Needs review
StatusFileSize
new60.36 KB
PASSED: [[SimpleTest]]: [MySQL] 41,427 pass(es).
[ View ]

Some strings were not in the correct case 'tools' is not the same than 'Tools'.

Status:Needs review» Needs work
Issue tags:-Usability, -Needs issue summary update, -consistency, -Platform Initiative

The last submitted patch, rename_menus-594660-53.patch, failed testing.

dagmar’s picture

Status:Needs work» Needs review
Issue tags:+Usability, +Needs issue summary update, +consistency, +Platform Initiative

#53: rename_menus-594660-53.patch queued for re-testing.

sun’s picture

Awesome! I still love this patch.

I reviewed the latest patch in-depth and additionally compared it to an earlier one - here's what I found:

+++ b/core/includes/menu.inc
@@ -1749,21 +1749,21 @@ function menu_set_custom_theme() {
+    'main-navigation' => 'Main navigation',

Meanwhile, I wonder whether it would be too much to ask to shorten the internal identifier for the Main navigation to just 'main'?

All of these system menus are menus/navigation/whatever, so the "-navigation" suffix in the 'main-navigation' identifier is unnecessary.

In turn, that would give us a really nice set of built-in default menus:

tools, administration, account, main

--

And, perhaps... (please don't shoot me ;)) ...could we also shorten the internal ID 'administration' to just 'admin'?

Because, this would look even more awesome to me:

tools, admin, account, main

+++ b/core/includes/menu.inc
@@ -1749,21 +1749,21 @@ function menu_set_custom_theme() {
/**
- * Returns an array of links to be rendered as the Main menu.
+ * Return an array of links to be rendered as the Main navigation menu.
  */
function menu_main_menu() {

Unintentional reversion of another phpDoc improvement ("Returns").

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/TreeOutputTest.php
@@ -54,8 +54,8 @@ class TreeOutputTest extends WebTestBase {
     // Validate that the - in main-menu is changed into an underscore

We've lost a comment fix here in the re-rolls.

sun’s picture

StatusFileSize
new57.69 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/lib/Drupal/system/Tests/Menu/TreeOutputTest.php.
[ View ]

Merged HEAD.
Fixed stale comment in TreeOutputTest.

Will incorporate #56 now, but attached patch is a fallback in case that proposal doesn't fly.

Status:Needs review» Needs work

The last submitted patch, platform.menus_.57.patch, failed testing.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new24.42 KB
new46.11 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch platform.menus_.59.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Reverted main-menu vs. main-navigation change for ex. "primary links" in page.tpl.php.
Renamed 'main-navigation' into 'main'.
Renamed 'administration' into 'admin'.

sun’s picture

StatusFileSize
new6.15 KB
new44.36 KB
PASSED: [[SimpleTest]]: [MySQL] 42,184 pass(es).
[ View ]

Reverted phpDoc change for menu_main_menu().
Removed unnecessary t() replacement value in contact_help().
Fixed stale 'administration' in MenuNodeTest.
Fixed double-quotes vs. single-quotes in menu_install().
Reverted unnecessary change to TreeOutputTest.
Reverted "Main menu" phpDoc change to page.tpl.php.
Fixed stale references to 'account-links' vs. 'account'.

sun’s picture

Assigned:Unassigned» sun
StatusFileSize
new1.55 KB
new45.54 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch platform.menus_.61.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Added upgrade path for renamed system menus and Menu module's custom menu records.

Status:Needs review» Needs work

The last submitted patch, platform.menus_.61.patch, failed testing.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new45.46 KB
PASSED: [[SimpleTest]]: [MySQL] 42,255 pass(es).
[ View ]

Merge branch '8.x' into platform-menus-594660-sun

sun’s picture

I think this is ready to fly. Would be great to make progress here, so we can follow up with the new 'footer' menu that has been asked for ten gazillions of times ;)

Bojhan’s picture

Status:Needs review» Reviewed & tested by the community

Whoo, yes!

no_commit_credit’s picture

StatusFileSize
new5.41 KB
new45.77 KB
FAILED: [[SimpleTest]]: [MySQL] 46,198 pass(es), 5 fail(s), and 0 exception(s).
[ View ]

Formatting nitpicks.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, menu-594660-66.patch, failed testing.

dagmar’s picture

Status:Needs work» Needs review
StatusFileSize
new1.1 KB
new46.87 KB
PASSED: [[SimpleTest]]: [MySQL] 46,224 pass(es).
[ View ]

Fixed tests for views integration.

sun’s picture

Status:Needs review» Reviewed & tested by the community
Issue tags:+Avoid commit conflicts

Thanks @xjm + @dagmar!

Can we pretty pretty please get this in now? This was RTBC a couple of times before and is a huge PITA to re-roll.

Dries’s picture

Status:Reviewed & tested by the community» Fixed

Committed to 8.x. Thanks!

sun’s picture

Status:Fixed» Needs review
Issue tags:-Needs issue summary update, -Avoid commit conflicts
StatusFileSize
new1.83 KB
FAILED: [[SimpleTest]]: [MySQL] 45,903 pass(es), 13 fail(s), and 13 exception(s).
[ View ]

Awesome, thanks.

Sorry, missed some updates.

sun’s picture

StatusFileSize
new3.24 KB
FAILED: [[SimpleTest]]: [MySQL] 46,114 pass(es), 108 fail(s), and 7 exception(s).
[ View ]

Oh. Interesting. The System menu block deltas require some additional massaging.

sun’s picture

StatusFileSize
new8.01 KB
PASSED: [[SimpleTest]]: [MySQL] 46,228 pass(es).
[ View ]

Of course, that block delta fix requires some more adjustments.

Attached patch should cut everything. Hopefully it comes back green.

webchick’s picture

Issue tags:+Needs tests

This implies we need upgrade path test coverage here.

sun’s picture

StatusFileSize
new9 KB
PASSED: [[SimpleTest]]: [MySQL] 46,231 pass(es).
[ View ]

Added some assertions to the filled Standard upgrade path test.

sun’s picture

Status:Needs review» Reviewed & tested by the community

Given the audience and assumed followers of this issue, I'm marking this follow-up patch RTBC (even though it is my own), so as to ensure that the update/upgrade path failures are taken care of ASAP, and also to avoid too much havoc in other areas (the block delta changes here have an impact on #1535868: Convert all blocks into plugins, and I hope that one wasn't adjusted for the system menu block names in current HEAD yet).

sun’s picture

Ping?

webchick’s picture

Status:Reviewed & tested by the community» Fixed

Sorry, but we don't commit self-RTBCed patches, especially when they are non-trivial. But I got verbal confirmation from chx that this looks good to him in #drupal-contribute, and looks good to me as well, so..

Committed and pushed to 8.x.

sun’s picture

Thanks.

The follow-up feature request: #891670: Add a Footer menu to the built-in system menus

Status:Fixed» Closed (fixed)

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

fubhy’s picture

This caused a regression for system menu blocks. #1863180: System menu blocks lack role="navigation" and .block-menu class

fubhy’s picture

Issue summary:View changes

updated for a new patch