Second level menu items can't be reached if they have children

Not sure if this is the right approach, but this splits the menu and and expand so you can use both.

Screen recording:

Before:

After:

1) Drupal\Tests\navigation\Kernel\NavigationMenuBlockTest::testHtmlMarkup
//li[contains(@class,'toolbar-menu__item--level-1')]/span[contains(@class, 'toolbar-button') and text()='title 3']
Failed asserting that 0 matches expected 1.
/builds/issue/drupal-3480321/core/modules/navigation/tests/src/Kernel/NavigationMenuBlockTest.php:352

Verified test coverage.

Never seem grid-column used with a negative before, nice

Only moving to NW for the issue summary, as it's missing proposed solution

Since navigation is still experimental assuming don't need a CR for the twig change.

Thanks for the review! Updated the issue summary. On the -1 I can only pass credit on to stack overflow :) Front-end is definitely not my forte.

Test status - Pass
Testing steps -

- Install drupal 11.x
- Login as admin
- Enable navigation
- Try to click on Structure > Display modes, which does not bring you to its menu item link /admin/structure/display-modes

Patch applied successfully.
Issue looks fixed.
But, can see that the design is breaking for children menu titles.

Please refer the gifs in attachments.

Looks like you need to clear your cache - both the twig template and css change. Thanks!

Hello scott_euser,

Test status - Pass
Patch applied successfully.
Issue looks fixed.
Testing steps -
- Install Drupal 11.x
- Login as admin
- Enable navigation
- Try to click on Configure > system,

But, can see that the design is breaking for children menu titles after clearing cache.
Check attachments

Thanks for checking; yes I can see the bug - I fixed it now + added more test coverage to validate.

Maybe this is tying loosely into #3425084: Support Deeper Navigation Levels

#16 did you mean to push some changes up?

Are #15 and #13 testing the same thing btw? Re-uploading additional screenshots or clips are not needed that are same or similar are usually not needed jsut an FYI.

Ah sorry, pushed to the old closed branch! Fixed now

You can also view it at https://mr10134-y6ngslwhd9aeawkbkod3pwnb7vnmyrr3.tugboatqa.com/user/login + https://mr10134-y6ngslwhd9aeawkbkod3pwnb7vnmyrr3.tugboatqa.com/admin after logging in

Seems like random unrelated test failure (seeing that sometimes lately), rerunning

Removing summary update tag as it appears fine now

1) Drupal\Tests\navigation\Kernel\NavigationMenuBlockTest::testHtmlMarkup
//li[contains(@class,'toolbar-menu__item--level-1')]/span[contains(@class, 'toolbar-button') and text()='title 3']
Failed asserting that 0 matches expected 1.
/builds/issue/drupal-3480321/core/modules/navigation/tests/src/Kernel/NavigationMenuBlockTest.php:355
FAILURES!
Tests: 3, Assertions: 37, Failures: 1.
Exiting with EXIT_CODE=1

Shows the test coverage

Manual testing appears to have been done in #13

Code wise see nothing wrong

LGTM

Thank you for work here.

1. We had something like this in initial versions of design. Then it was replaced with current variant. So i added required label.

2. Visual regression in focus ring https://gyazo.com/5b8989e769ebf73709d00bf94f2e0004

3. Keyboard navigation seems not worked as before. EG: Right and left arrow behaviour.

4. Also small note in MR

1. Sorry I didn't quite follow, where should I add a required label? Or is that a process FYI thing?

2. Fixed:

3. This seems to work: if you tab to the down arrow and press right/left it opens closes. I am not sure it makes sense to have both the button and link both respond to the right/left arrow? If so, do you have any tips? I tried duplicating these bits from the down arrow:

'aria-expanded': 'false',
'data-toolbar-menu-trigger': menu_level,

I might be a bit in over my head there if that is what you are expecting.

4. Changed to SDC, thanks!

In any case thank you very much for the reviewing and tips!

For context, it was decided earlier that access to the second level menus would be limited in the menu. This is due to the reason that Drupal Core uses those primarily to display the same information as what would be in the navigation but as a page. As a result, users would likely develop a pattern of not accessing those pages. The UX team did research to validate that the pages aren't considered to be useful.

The tricky bit is that there are some edge cases where the second level menu is actually useful. This is not great from UX perspective, because if the user has developed a pattern of not accessing the second level pages, they would have challenges to find these pages. The original plan was to provide an alternative affordance for this use case, to make it easy to differentiate between the two use cases so that users know when to expect something useful.

What is in the MR doesn't fulfill this. I think we need some direction from @ckrina on how we should handle this scenario.

I suppose it could be in contrib extending the template and overriding the css?

If in Core it sounds like maybe we need a mechanism either to opt in or opt out of having a direct access link. If opt in would need to update a lot of a contrib projects but at least that makes it a conscious decision. Perhaps a new attribute in menu link?

Core needs to provide direction for how to fix the Simple Blocks and Paragraphs examples from the issue summary. This might lead into change in either Core or these modules.

Ideally the fix for the Simple Blocks and Paragraphs would keep the Display mode use case intact because the Display mode example is as it is by design. This is where a contrib module could change the preference for the users that prefer a different behavior.

If in Core it sounds like maybe we need a mechanism either to opt in or opt out of having a direct access link. If opt in would need to update a lot of a contrib projects but at least that makes it a conscious decision. Perhaps a new attribute in menu link?

We could potentially automate that; the pages we don't want to display in the navigation are always rendered by \Drupal\system\Controller\SystemController. It's a special controller designed to render menus. We would only want to link from the menu when the controller is something else, e.g. \Drupal\block\Controller\BlockListController.

That sounds sensible. I would be happy to work on that but sounds like we should get ckrina's opinion first? If its a go for that route, would that be okay to add to this issue's scope?

@ckrina is out until next Wednesday but we could reach out to her then. I have discussed this a while back with @ckrina on a high level.

Here's the latest design I've seen for this:

Essentially in this design there would be an Overview link that would allow user to navigate to the parent link. I don't think this a finalized design so we probably want to wait for input from here before we get too far on the implementation.

Okay thanks for talking me through it! Happy to help on implementation when we get to that stage (that does look simpler to achieve)

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

This feels like a duplicate of #3478869: Add "All" or overview links to parent links, which feels like a duplicate of #3402907: Parent menu items with URLs are not linked to, do we make these available?.

Let’s leave this one open for the time being until we can get @ckrina's input per @lauriii's comments. As you pointed out @michaellander, this has popped up a few times, so it's a pain point for some. We should at least document the decisions made. #3402907: Parent menu items with URLs are not linked to, do we make these available? comes close to that, but it was from a time when the design was much less settled than it is today.

I know there have been more recent design discussions on this topic. I'm attempting to reach out to some folks that worked on the designs to see if they can lend some insight into more recent discussions. Otherwise, I agree we should get @ckrina's input here before closing it out.

To note, pages that list other things can still be useful, like /admin/structure/views is quite essential as many sites will have too many views to list in the menu + there is plenty of other useful information in that table that can help site builders choose which view edit that they lose out on if they stop being able to access it (fully aware that hiding it does not 'stop' them accessing it but certainly makes it hard to get to). I believe that would also be solved if we follow @laurii's suggestion in #30.

I'm removing the Display Modes example from the issue summary since that is working by design. The remaining use cases are valid and should be addressed one way or another, either in core or by guiding how contrib modules should approach this.

Had a chance to catch up with @ckrina today. The design that @laurii referenced in #30 is indeed the latest iteration of the design to accommodate this issue. Let’s update the MR and/or close the current MR and open a new one for this approach.

Here is a representative screenshot:

Here is a link to the Figma: https://www.figma.com/design/VCPAxetieAC2pzw7hgX3ij/Drupal-Admin-UI---De...

A couple of caveats to keep in mind:

• "Overview" is a working title for this link. Seeking some UX input on that. Other considerations might be "See all".
• The Figma designs show a fourth level ("Great Grandchild Item"). This is the subject of #3425084: Support Deeper Navigation Levels and is out of scope here.

I've also updated the issue description to match.

Thanks for the updates!

Not a hill I'm willing to die on but FWIW...
I don't quite agree and the UX research backs that up:

1. e.g. version 2 (current MR) lower rate of failure, lower time to complete task
2. A less clear UX article on it from NN hints at this too in item 12, but the separation only becomes clear when you actually visit the example they point to)

But I am fully aware I am late to the party and decisions have been made, so fully happy for you to ignore this; just saying my piece :)

Next steps
Before this gets work on I have a few questions:

1. From #30: "We could potentially automate that; the pages we don't want to display in the navigation are always rendered by \Drupal\system\Controller\SystemController". Actually thinking about this further:
   1. There is value in consistency particularly if it won't be obvious to the end user why some show overview and some not
   2. There is value in the overviews: some people actually might rely on the overview to understand what the options are in an area; ie, the descriptions do have value (or otherwise why are we providing descriptions in the first place). If we sometimes skip the overview we make that value inaccessible

   So would be good to agree on whether we add that to scope or not (my vote is not FWIW)
    
   

2. Do we just output 'Overview' and link to the parent, overriding its label effectively? Or do we make it controllable; override the menu link in some sort of pre-processing of the navigation menu links, but still allowing modules to override it back in case 'Overview' as a label is inappropriate (e.g., what if its not providing an overview?)

Then from there we can update issue summary and get to work on it (which I am happy to help with).

I was also around for some of those early discussions and so was going to say something similar to what @lauriii said in comment #26, so I'll try not to repeat, but I still support the decisions that were made around thsi.

Paragraphs is an interesting example, because what it does is non-standard, there are no example in core of these kind of "add" links appearing in the menu in this way, neither are there examples of all the different "types" appearing in the menu. Similarly, admin_toolbar adding links for all the Views a site has is also very non-standard. In both of these examples, if you have a lot of Paragraph types or a lot of Views, they don't all appear in the menu, so you end up going to the full page anyway. I think it's really important that we have consistent patterns for how things should be done.

I also still agree that in the vast majority of cases the Overview links are not useful, therefor adding them results in a net-negative user experience because it adds more visual noise. For screen reader users, having a lot of links that all say "Overview" is really unhelpful and could be confusing. Not just because they have to hear the word "Overview" every time the user opens a sub-menu, but also because if a screen reader user asks for a list of all links on the page, having a bunch of links that read simply "Overview" or "Add" without any other context could be very confusing.

So my feeling is that we shouldn't encourage a pattern which results in all of these generally redundant "Overview" links.

I think we can probably find a pattern that addresses what Paragraphs, Views and Admin toolbar are attempting to do, but right now I don't know what that pattern should be.

The split button proposed by @scott_euser in #6 seems like the most intuitive direction here. I also share the concerns raised about the additional ‘Overview’ injected links.

@ckina and @lauriii could you give this a second look?

This affects a number of modules I have co-maintainership of such as:

• Menu Position
• Calendar (by way of dependencyViews Templates)
• Feeds

And simple_block as mentioned previously and core's Display modes all under Structure I might add. "Overview" link seems sensible for Configuration at a glance but not Structure. Though I believe the Split button approach is more versatile.

The issue with split buttons, in the context of menu items, is that it introduces different interactions patterns depending on which part of the menu item the user interacts with. It can be challenging to communicate those different interactions to users and complicates the interface. These can also be challenging to use on touch devices, and for users who struggle with precise mouse clicks/taps (because the "arrow" click-area is usually quite small and there usually isn't any space between the click-area for the link text).

There's a really good article from Nielsen Norman Group that goes into much more detail as to why these should not be used for navigation:

Don’t Use Split Buttons for Navigation Menus - nngroup.com

That article also proposes some alternative option, some of which we've discussed here, like providing a link to the "landing page" inside the sub-menu, which is perfectly fine; But as we've discussed if we do this for all sub-menus, in the vast majority of cases those "Overview" links are entirely redundant.

Ultimately I don't think we're going to find a good technical solution here, and that's because this is not a technical problem, it's an information architecture problem.

In cases where the "Overview" page is relevant, because maybe that's the content type list or similar, we should simply provide that as a link in the sub-menu, and we should call that link something like "All content types". Or ultimately reorganize the entire admin menu (but that's probably out of scope here 😀). What we shouldn't do is add a lot of repeating links which use the label "Overview".

I think when we treat this as a problem of information architecture, and not a technical problem, we'll end up with a much better user experience.

Yes, “reorganizing the entire admin menu” is definitely out of scope! 😄 The issue largely stems from contrib modules adding their own admin pages under existing ones or creating nested subpages (but core too as screenshoted in #44). Addressing this by fixing contrib would likely require more effort and education than working with the existing patterns commonly used.

The article raises valid concerns about split buttons in navigation, it might overstate their universal unsuitability. Instead of relying solely on these arguments, we should test those concerns ourselves, perhaps against @scott_euser propposal in #6, to see how they hold up in practice. A balanced approach that evaluates context, user needs, and new design tricks might reveal a more nuanced view of this pattern. Safe enough to try?

We've been using the 'Overview' approach patch from #3478869: Add "All" or overview links to parent links as a stop gap because users were getting so frustrated they couldn't access the 2nd level when a third level item was added. In our particular example it was a combination of the commerce and scheduler that was making it impossible to access the commerce dashboard.

In this particular use case I don't believe commerce should define a submenu link below 'Products' because as far as it's aware, it has no other children. However, we also can't put the onus on 'scheduler' because any number of modules could extend commerce in ways that they feel warrant adding children to the 'Products' menu item and then you have an issue with many modules determining how to keep from breaking the 2nd level 'Products' link.

I'll be honest, so long as we treat the dynamic between the 1st and 2nd level different from the 2nd and 3rd level, where the 1st level is linked at the top of 2nd level, but 2nd level is not accessible at all if 3rd level exists, I believe we'll always have this confusion. I think this also includes hiding/showing behavior as related to \Drupal\system\Controller\SystemController. Perhaps an additional property could be added for Menu links to define an 'Override' title when children exist, but again that seems like something that should be consistent between the 1st and 2nd levels as well.

It's for these reasons that I think the split button is most elegant, but I'd even take the 'Overview' solution over no solution.

I think we are at an impasse and better not to let perfect be the enemy of good. Overview is certainly a million times better than inaccessible. Adding an MR + test coverage with overview in place.

This is a guess as I believe my questions in #42 have not been answered. My concern with this approach is that contrib modules cannot change 'Overview' to something else when 'Overview' is not appropriate. In which case instead of adding via Twig, it may be better to modify NavigationMenuBlock::build() and loop through the tree to add in the additional links. Then contrib would be able to change 'Overview' to something else. I suppose that could be a follow-up (again not to let perfect be enemy of good)?

After MR, default drupal 11 install + enable Navigation module.

Updated issue summary

Thanks for the review, resolved the feedback, ready for review again. Thanks!

Works fine for me! Resolves a very frustrating issue that I couldn't escape from since I upgraded to Gin Afdmin theme 4.x

Reviewed the code change and installed the patch, resolves the main issue neatly, which is making the 2nd level menu accessible. I agree with Scott that we shouldn't let "perfect be the enemy of good here" and there are certainly ways that it can be improved but the current state is just frustrating and this serves well for core menu items.

Hi,

Thanks for the issue and the work done on it.

I confirm too, that in current state at least it removes blocker of the current state.

Attaching patch from MR for Composer usage.

I'd say Overview is probably the lesser of two evils. On the one hand its not ideal, but on the other hand not being able to get to actually useful pages is also not ideal, and is arguably much worse from a UX perspective.

I also wondered if maybe we could do something clever and auto detect if a parent item is a "useful" page, but that requires us to define what a "useful" page is, and that's probably an unsolvable task given the variety of different implementation of "useful" pages in contrib and custom. That in itself would also not be great because it would introduce an inconsistent pattern where sometimes there's an "overview" link and sometimes there isn't.

> I also wondered if maybe we could do something clever and auto detect if a parent item is a "useful" page, but that requires us to define what a "useful" page is, and that's probably an unsolvable task given the variety of different implementation of "useful" pages in contrib and custom

I think it's not that unsolvable. We can treat it as an ignore list and only list things we know are save to exclude. On top of my mind, there's one obvious candidate, and that is the controller being \Drupal\system\Controller\SystemController::systemAdminMenuBlockPage. That should cover all the config overview pages that just list the links below. I don't know how readily that information is available where we need it, we'd need to fetch the route object I guess.

Also interested in this being resolved, I've already been getting bug reports about this in contrib projects, so I don't want to hold this up, could be a follow-up to optimize it?

First, thank you all for pushing to solve this and all the work that happened so far.

Agreed with @aaronmchale and @berdir here, which is the main follow-up planned after removing the link from the title+dropdown because of the UX issues it was causing. I can't find the issue so I guess we forgot to open it with all the work happening the months we were sprinting to get Navigation into core.

How to determine what gets a "see all / see the parent/whatever" is what needs to be defined. I don't have a strong opinion on that and I'm sure we can come out with a good strategy if we can research patterns/needs. What I do know for sure is that adding an "Overview" to all sub-menus is not the right strategy because it creates a worst experience: it's adding extra links that in most cases will only get you to a page with the exact same links you already have in the menu (old admin list pages).

So I'm moving this to Needs works to define out the best strategy to print those "Overview" links only when they are really needed.

Some of the first ideas we had was to print anything that wasn't an admin list page, which is what @berdir is already suggesting in #59. I'd be happy to ship only with that (I understand you all need this) and we make incremental improvements later.

PD. @joelpittet the results of the tests for the navigation can be found in #3401352: [PLAN] Usability Testing and Research on the Toolbar, the ones specifically about the splitbutton pattern in #3379160: Toolbar Prototype Usability Testing Phase I.

Okay I believe this should do it, but needs new tests again I assume:

With new MR, no Overview in Display Modes:

But yes Overview in Paragraphs:

I attempted to add tests myself but could use a hand - getting the mocking working to use the navigation menu tree and give access to the route name is a bit of a learning curve for me. Wondering whether it would be okay to make a functional test out of it and leverage menu_test test module from system as that appears to already have a good mix of systemAdminMenuBlockPage. Otherwise worried this gets stuck for lack of tests and continues to be a problem in contrib.

Wondering whether it would be okay to make a functional test out of it and leverage menu_test test module from system as that appears to already have a good mix of systemAdminMenuBlockPage

A functional test seems fine here, or if it's just the links that are needed a kernel test might be enough?

Using this patch on a project and it works quite well! Would be great to get it in before 11.2.

Pushed up MR 12148.

Based this on work done in the previous MR 10900, but I decided to create a new menu tree manipulator to inject the overview links instead. This provides a lot of different ways for contrib or custom modules to override this new behavior, whether by altering/decorating the new manipulator service, implementing hook_navigation_menu_link_tree_alter(), or altering the block render array.

Also, since there were at least a couple comments expressing concern about multiple links titled "Overview", I changed it so that the new child link's title is the same as the parent's, except with " overview" appended. I guess that will be awkward if someone titles a menu link "Something overview", so the child link title becomes "Something overview overview", but I think that is an edge case and the link can be altered as mentioned above.

Added a unit test for the new manipulator service. I think the unit test class has pretty exhaustive coverage, so while I did also update the navigation block kernel test as well, I added fewer cases there because I think the coverage would be mostly duplicative.

This should ready for review again. If the other approach is preferred, we can probably grab the kernel test changes in MR 12148 and add them to 10900.

Any updates on this?

Personally, I'd prefer if the menu item containing children could itself be clicked with ctrl+click or middle-click in order to open its link in a new window. That's how admin_toolbar works. You hover to open the submenu, and you can click to open its page.

This would make it so that Permissions and Roles could still be in a submenu of the People parent item, and the People menu entry could still be clicked to go to /admin/people.

I would consider the solution proposed in #3480322: Navigating to first level menu items is not obvious to be the best approach to solving the problem, because it solves both the issue there and this issue.

I also support the addition of Overview links in combination with that, but the menu structure suggested in the related issue is the best overall solution.

Rolled MR 10900 into a patch against 11.1.x

Personally I prefer the original approach, adding just 'Overview' is simpler and cleaner and avoids repetition. Screenshots below showing the difference between these. I've asked @ckrina to weigh in so we can have a final decision and ideally get this committed soon.

MR 10900:

MR 12148:

The wording of the menu link is a trivial change to either MR whichever way, and I'm not strongly opinionated either way.

That said, my one small pushback to just using "Overview" is that it can be considered an accessibility issue on a page for there to be multiple links with the same text that go to different URLs.

Personally, I'd prefer if the menu item containing children could itself be clicked with ctrl+click or middle-click in order to open its link in a new window. That's how admin_toolbar works. You hover to open the submenu, and you can click to open its page

I believe (from memory) this was discussed earlier in this issue and we agreed that it wouldn't be ideal as there's some accessibility issue around that approach.

Personally, I'd prefer if the menu item containing children could itself be clicked with ctrl+click or middle-click in order to open its link in a new window. That's how admin_toolbar works. You hover to open the submenu, and you can click to open its page

This is not an standard pattern, and we don't want users to have to learn custom patterns. We already did user tests that proved that the item containing children has to perform the action of opening the children menu, and adding other options confused the users (even if they knew they could do it).

Thanks @pameeela for adding the screenshots! For the rest, please remember that issues with UI changes need screenshots to be evaluated (and you're adding a new approach like in here) :)

Also, since there were at least a couple comments expressing concern about multiple links titled "Overview", I changed it so that the new child link's title is the same as the parent's, except with " overview" appended.

Let's keep the initial approach from MR 10900 that uses the Overview text for all the original reasons: repetitive pattern that can be remembered, shorter solution to avoid longer lines in a small space (remember translations: just in Spanish it's "Descripción general"). If people really really have a big concern with this we can add it to the new round of user tests.

I decided to create a new menu tree manipulator to inject the overview links instead. This provides a lot of different ways for contrib or custom modules to override this new behavior, whether by altering/decorating the new manipulator service, implementing hook_navigation_menu_link_tree_alter(), or altering the block render array.

Thanks for the thoughtful work on this! I understand you've built in flexibility, but before moving forward, I'd love to understand the problem this is solving. What specific product need or requirement drove the decision to add this new menu tree manipulator? Since this was flagged as a stable blocker to get Navigation into Stable (adding the missing tag), we need to make sure we're addressing a real pain point rather than adding complexity in the name of future flexibility. Is there any specific need you've detected in a project? Understanding the 'why' will help us evaluate whether this approach is the right fit, or if we're fine with the previous path to achieve the same goal.

TL;DR:
MR 12148 updated to have link text say "Overview"

MR 10900 has testing added (the kernel test from 12148). The test is failing, so it needs looking at.

we need to make sure we're addressing a real pain point rather than adding complexity in the name of future flexibility. Is there any specific need you've detected in a project?

I'm not sure it's that much more complex. The code to add the links is essentially just moved to live in its own place. There's a bonus that it's also unit testable.

As for flexibility, I don't know about specific needs, other than some comments in this issue talking about there being various contrib/custom solutions to this problem already in place, so the flexibility is to try to accommodate whatever those may be.

Meanwhile, I have copied the kernel test changes from 12148 to 10900. Unfortunately, there's a test failure, so some investigation is needed whether there's an issue with the test applied to this MR, or if there's something in the original code changes that aren't covered. I'm in the middle of traveling, but I can try to look at what's going on later.

After reviewing both MRs, my opinion is that we should follow the approach suggested in 12148.

The manipulator approach adds useful flexibility for the future without significantly impacting the current codebase — I think it's a solid direction.

Checked and tested locally code in MR 12148 and it looks good to me.

I’ve added a couple of comments to the MR, but this is very close to being ready!

I have updated MR 12148 based on MR feedback. It's ready for review again.

Separate note: My assumption is that this has missed 11.2.0. In case that it's still possible to get in, and if adding a menu manipualtor service is a blocker to that at this point, I have resolved the test issues on MR 10900 and gotten to functional parity on the front end with the latest changes in MR 12148.

That's awesome, thanks @godotislate! @catch said this can probably get committed during RC since the module is not stable yet.

Feedback has been addressed and the new 3rd level item functionality works according to previous comments.

I would say this can be marked as RTBC now.

Since Navigation is still an experimental module, and since this is resolving a major, user-facing bug, this is eligible for backport during RC (or even a patch release). The beta experimental phase is precisely for fixing user-facing issues like this. The small risk of the API addition of the service and new methods in an experimental module is outweighed by the benefit of resolving this issue for users.

Crediting @catch who also +1ed this as an RM signoff for the backport in Slack.

This patch is for Drupal 10.4.8, based on merge request !10900.

While this is a bug fix, there's a nice user-facing change that we could potentially put into the 11.2.0 announcement so tagging just in case.

fwiw while I don't find menu tree manipulators particularly intuitive, it does help to avoid a long 'alter method of doom' that's trying to handle too many different things, and given I think there's a non-zero chance this approach could be revisited again (e.g. if we ever move away from SystemAdminMenuBlockPage as a way to build the admin structure, or if a new design approach appears), it's good to have it encapsulated.

Did my best with issue credit but this is a long issue with lots of contributors so it may not be perfect.

Committed/pushed to 11.x and 11.2.x, thanks!