The Admin Toolbar module does not display the "Updates report" menu item.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 2961757-13-admin_toolbar.patch | 1.77 KB | pasqualle |
The Admin Toolbar module does not display the "Updates report" menu item.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 2961757-13-admin_toolbar.patch | 1.77 KB | pasqualle |
Comments
Comment #2
pasqualleComment #4
pasqualleComment #5
jhodgdonHm. I'm not sure about that test failure, looks like it's unrelated to this patch.
But anyway... Why should we need a hook_menu_links_discovered_alter() here? Maybe we should address the root of the problem -- why is it not showing up in the menu links that *are* discovered automatically by the menu system?
Comment #6
jhodgdonIn other words, we have a config_update_ui.links.task.yml that adds this to the Configuration Manager's admin page. This is the same way that the links in core/modules/config/config.links.task.yml are added to that page. Are they showing up in Admin Toolbar? Because there is no hook_menu_links_discovered_alter() in config.module in Core.
Comment #7
jhodgdonThe failing test problem is from this related issue.
Comment #8
pasqualleI do not know why it works like this, but this is how it works. See:
https://cgit.drupalcode.org/admin_toolbar/tree/admin_toolbar_tools/admin...
Comment #9
jhodgdonOh, I see, they've special-cased certain Core modules. It seems to me that that philosophy is the problem -- Admin Toolbar should be looking at the local task tree so that contrib modules that add to these local tasks are automatically detected, rather than special casing some modules to get their menu tasks into the toolbar.
Comment #10
jhodgdonWhat I'm saying is that they've basically made it so that every contrib module that adds to Core's local tasks has to implement this hook if they want to be listed in Admin Toolbar. They've also implemented the hook on behalf of a bunch of core modules. They should have instead solved the problem in a more systematic way, by looking at the local tasks. This would have (a) greatly simplified their code and (b) made it so contrib modules didn't have to implement this hook in order to get listed.
Comment #11
pasqualleI guess the admin toolbar does not want to display all the menu items and local tasks as it could make the menu too large and can slow down the admin interface. Killing the whole purpose of the module: "providing a fast access to all administration pages".
Even these special cased core local tasks are moved into separate module admin_toolbar_tools.
I guess this is the reason why it choose to use a hook, but I might be wrong.
Comment #12
jhodgdonSo... I'm seeing now that this big hook implementation is part of the Admin Toolbar Extra Tools module, and its effect is that it adds a bunch of the Core local tasks (some? all? I don't know) as links to the core Tools menu (which is the stated purpose of this add-on module). These links are present no matter what someone is doing with the Tools menu -- they've used a Core hook, not a hook that would be specific to Admin Toolbar. Which means that if we implement this core hook in our module, and someone is *not* using the Admin Toolbar Extra Tools menu, we've added our local tasks to the Tools menu, and they shouldn't be there (the rest of the Core links that are added by Admin Toolbar Extra Tools won't be there either).
So I think what we could do in this module is go ahead and implement the hook, but only add the links if the Admin Toolbar Extra Tools module is installed (otherwise, just return and do nothing). Presumably if someone installs that module, they would also want our module's pages added to the "extra tools" (and hence to the Admin Toolbar). So if you put that module exists check into the patch, with a comment as to why we're doing this, I'll be willing to commit it.
Thanks!
Comment #13
pasqualleComment #14
jhodgdonLooks OK, but can you add a comment to explain why we are doing this? And I assume you've tested and this patch works to add links to your site? Thanks!
Comment #16
jhodgdonThis is low-risk, since it only happens if the admin_toolbar_tools module is present... so I went ahead and added a comment and committed this. Sorry for the delay -- I have been out on vacation for a few weeks. Thanks!