With the D8 version (#1137920-114: Fix toolbar on small screen sizes and redesign toolbar for desktop), "People" has no child items. But if you go to admin/people, "permissions" and "roles" are available as tabs, and seem like useful things to have available in the toolbar as well. There's probably other similar examples. The technical reason why it's not showing up is the way menu_tree_output() treats hidden = -1, but we can fix / workaround that if we can come up with a clear UX rule around which tabs we want replicated in the toolbar and which ones we don't.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tkoleary’s picture

@effulgentsia I totally agree that there is no clear rule now for what is part of the admin IA and what is a "local task" (tab). Part of me wants to say nothing is a local task when we have a deep menu that can expose it all, but I think that is not the answer either. One example of items that are tabs now which should probably not be menu items is the themes tabs in appearance/settings.

I think I need to do some research on how other systems deal with this but my gut feeling is that there should be a bright line, ie. nothing that is a local task should also appear in the menu.

JohnAlbin’s picture

Part of me wants to say nothing is a local task when we have a deep menu that can expose it all, but I think that is not the answer either.

Yes, both the Appearance page and the Blocks Admin page (which is being redone) currently have tabs that are a list of all the enabled themes on the site. That makes me not want to include tabs in the navigation. Also, the admin/config/people/accounts/fields tab feels too granular for tabs.

There are counter arguments as well.

Which makes me start to think that the "best solution" would be that we do case-by-case decision on whether the tabs should be included in the menu hierarchy. :-\ Someone prove me wrong, please. Yet-another-key in hook_menu just for "show tabs in toolbar" makes me *sigh*.

Shyamala’s picture

tagging

Shyamala’s picture

Issue tags: +#d8ux, +toolbar-followup

tagging

effulgentsia’s picture

Title: Should some/all local tasks be available in the toolbar nav? » Decide which local tasks should be available in the expanded toolbar navigation
Project: Navbar » Drupal core
Version: 7.x-1.x-dev » 8.x-dev
Component: User interface » toolbar.module
Category: bug » task

Moving to the core queue and retitling for #2.

effulgentsia’s picture

Title: Decide which local tasks should be available in the expanded toolbar navigation » Decide which local tasks to add to toolbar navigation

Shortening title for mobile friendliness.

sun’s picture

admin_menu's rule to this is very simple:

- Default local tasks are excluded.

- All other local tasks are included.

Please note that you see many many more local tasks in admin_menu, because it implements an additional, partial subtree merge process to additionally expose individual configuration entities with their regular menu links, which would normally require dynamic request parameters to appear as tabs/tasks.

The actual/regular amount of local tasks in the system, especially when omitting default local tasks, is actually not very large.

For the horizontally oriented menu - if it would have dropdowns - that works very well.

For the vertically oriented menu I'm not sure... but I actually have fundamental usage/interface/interaction problems with that design and implementation in the first place, which likely explains why I'm not able to make sense of tasks in there.

Wim Leers’s picture

Title: Decide which local tasks to add to toolbar navigation » Decide which local tasks to add to the toolbar menu tray — mimick D7 admin_menu?
Issue summary: View changes
Issue tags: +Usability, +DrupalCamp Ghent 2014, +sprint

I think #7 makes a ton of sense.

In any case, the current behavior is just plain weird and keeps tripping me up. E.g. under "Content" you see "Comments", and nothing else. But if you click on either "Content" or "Comments", you see that there also is a "Files" tab! That "Files" tab should have appeared in the menu tree rendered in the tray.

tkoleary’s picture

@wim leers

"Files" tab! That "Files" tab should have appeared in the menu tree rendered in the tray.

Yes! I noticed that too. Thanks for pointing it out.

Wim Leers’s picture

Issue tags: -sprint
Bojhan’s picture

I am in support of @sun his plan, that sounds sensible and what the vertical toolbar should have been doing in the first place (sigh, said it in the initial issue 100 times).

Wim Leers’s picture

Is this indeed normal, or rather major?

The example I gave in #8 (which is a symptom of this issue) is a major WTF for me.

pixelmord’s picture

Priority: Normal » Major

I think this is major, because it is just very confusing. -> UX

Bojhan’s picture

Priority: Major » Normal

I am not sure if this is the right issue to do that, neither that it should be bumped to major. Very few people seem to be upset by this, and the UX impact can only be solved trough loads and loads of work.

pixelmord’s picture

Assigned: Unassigned » pixelmord

@Bojhan: o.k. fair enough, but it bugs me, will create a patch to add the local tasks to the toolbar menu.
It just bugs me, so will try to solve that.

pixelmord’s picture

Status: Active » Needs review
FileSize
5.26 KB

So here's a patch that adds all local tasks for a given route to the toolbar menu.

lauriii’s picture

Can you please provide steps how this can be tested manually? We also need automated test coverage for this.

  1. +++ b/core/modules/toolbar/toolbar.module
    @@ -8,6 +8,8 @@
    +use Drupal\Core\Menu\LocalTaskManager;
    +use Drupal\Core\Menu\LocalTaskInterface;
    

    Unused use statements

  2. +++ b/core/modules/toolbar/toolbar.module
    @@ -314,13 +319,49 @@ function _toolbar_do_get_rendered_subtrees(array $data) {
    +    $local_tasks_bui = $local_task_manager->getTasksBuild($element->link->getRouteName());
    

    Unused variable

  3. +++ b/core/modules/toolbar/toolbar.module
    @@ -350,3 +391,45 @@ function _toolbar_get_subtrees_hash() {
    +
    

    Is missing docblock. Since this is a procedural function it should be renamed to _build_tasks_as_menu_tree()

  4. +++ b/core/modules/toolbar/toolbar.module
    @@ -350,3 +391,45 @@ function _toolbar_get_subtrees_hash() {
    +  $menu_tree = array(
    +    '#sorted' => TRUE,
    +    '#theme' => 'menu__toolbar__admin',
    +    '#items' => array(),
    +  );
    

    Since Drupal 8 supports only PHP 5.5, new array syntax can be used like $menu_tree = [];

  5. +++ b/core/modules/toolbar/toolbar.module
    @@ -350,3 +391,45 @@ function _toolbar_get_subtrees_hash() {
    +  // Pre-fetch all routes involved in the tree. This reduces the number
    +  // of SQL queries that would otherwise be triggered by the access manager.
    

    The right column has still space so something can be moved on the first line

  6. +++ b/core/modules/toolbar/toolbar.module
    @@ -350,3 +391,45 @@ function _toolbar_get_subtrees_hash() {
    +      if ($access) {
    +
    +        $link = array(
    

    Unnecessary extra line change

  7. +++ b/core/modules/toolbar/toolbar.module
    @@ -350,3 +391,45 @@ function _toolbar_get_subtrees_hash() {
    +  }
    +  return $menu_tree;
    

    Before return we could add extra line change

pixelmord’s picture

Thanks for the review, looked to fix those mentioned issues, new patch attached.

pixelmord’s picture

Manual tests could be done as follows:

Put the toolbar in sidebar display to have the collapsible menu items shown with their children.

Without the patch you have only one sub menu item under Content (Comments). If you open the Content or Comment menu link, you see that there's another tab for a local task called files. Or with appearance you have no submenu items at all, even though there are the tasks "update" and "settings"

Applying the patch should add those local tasks to the toolbar menu as children of the main menu items. Both situations should be covered:

  • a) if the menu item already has children from the admin menu the local tasks will be added
  • b) if the item has no children also all available local tasks will be added
  • c) no duplicates should be added.

I will see to add automated tests

pixelmord’s picture

Unfortunately creating a test is hard, because the toolbar submenus are rendered via JavaScript, so I can not test for the presence of those links.

But manually testing I found a bug that the local tasks were not attached to the render array when there already was a regular submenu item.
I fixed that and attached a new patch.

tkoleary’s picture

@pixelmord tested it on simplytest.me

Works for admin/people/permissions etc. but not:

admin/content/files

Or things like: admin/structure/block/block-content that are another level down

If we can see:

  • Display modes
    • form modes
    • view modes

We should also see:

  • Block layout
    • Blocks
    • Custom blocks

And while we're at it, why an admin list for form modes and view modes and not a "Display modes" page with view modes and form modes tabs?

pixelmord’s picture

Status: Needs review » Needs work

@tkoleary: You are right about the 3rd level menu items representing local tasks. I will look into that.

admin/content/files however works for me in all my tests and on simplytest.me

webchick’s picture

Since this issue mentions admin_menu, linking a related issue talking about drop-downs.

pixelmord’s picture

Here's a new patch that adds also 3rd level items from local tasks.

This will add the above mentioned:

  • Block layout
    • Custom block library

Currently I avoid adding any local tasks that would duplicate items already in the menu.
However that removes also some of the secondary local tasks , because the current approach of nesting the menu in general prevents that due to the fact that we do not repeat the default (primary) local task in the hierarchy, so then there's no parent to some of the secondary local task.

This is more an UX/UI issue. This will also get even more interesting, when we think about using a horizontal dropdown concept that is triggered by hover. For me it was always a little confusing in admin_menu that you had to remember clicking on the parent item and then getting on a page where the parent item has a "sibling" tab that would be found as a "child" in the menu hierarchy.

Example using the menu branch mentioned above:

  • Block layout
    • Block layout (this is the repetition we avoid currently)
      • Bartik
      • Classy
      • Seven
    • Custom block library
      • Blocks (unfortunately this is a repetition of the parent route again)
      • Types

I personally would not mind repeating the default local task, but we might need exceptions, because we wouldn't want to do that in case no secondary tasks need to be added as children, or if it would be the only child of its parent with the same route.

pixelmord’s picture

Status: Needs work » Needs review

Needs review for code and usability because of the above mentioned issues with the way create the hierarchy while trying to omit the default local tasks.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

svenryen’s picture

Is any work still being done on this issue? I landed here because I could only see "Comments" under "Content" and not "Files". I must say I support fixing this.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

nod_’s picture

Version: 8.8.x-dev » 9.1.x-dev
Status: Needs review » Needs work

This need at least a reroll

nod_’s picture

That would also help the related issue

mrinalini9’s picture

Assigned: Unassigned » mrinalini9
mrinalini9’s picture

Assigned: mrinalini9 » Unassigned
Status: Needs work » Needs review
FileSize
7.39 KB

Rerolled patch for 9.1.x, please review.

Status: Needs review » Needs work

The last submitted patch, 37: toolbar-local-tasks-1811998-37.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Hardik_Patel_12’s picture

Assigned: Unassigned » Hardik_Patel_12
Hardik_Patel_12’s picture

Tried to solve failure test cases.Kindly review a new patch.

Status: Needs review » Needs work

The last submitted patch, 40: toolbar-local-tasks-1811998-40.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

ravi.shankar’s picture

Status: Needs work » Needs review
FileSize
6.97 KB
9.6 KB

Here I have tried to fix failed tests and fixed some CS issues.

Status: Needs review » Needs work

The last submitted patch, 42: 1811998-42.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

codersukanta’s picture

Assigned: Unassigned » codersukanta
codersukanta’s picture

Assigned: codersukanta » Unassigned
Status: Needs work » Needs review
FileSize
7.93 KB
1017 bytes

It's my bad there are lots of dependency on AssertPageCacheContextsAndTagsTrait.php. So we cant modify it we need to work on the codebase.

Status: Needs review » Needs work

The last submitted patch, 45: 1811998-45.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

codersukanta’s picture

Status: Needs work » Needs review
FileSize
3.42 KB
10.33 KB

Updated patch

Status: Needs review » Needs work

The last submitted patch, 47: 1811998-46.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Hardik_Patel_12’s picture

Re-rolling the patch.

Hardik_Patel_12’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 49: 1811998-49.patch, failed testing. View results

vsujeetkumar’s picture

Status: Needs work » Needs review
FileSize
10.4 KB
947 bytes

Fixed tests, Please review.

Status: Needs review » Needs work

The last submitted patch, 52: 1811998_52.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.