I'm not sure if this is being discussed elsewhere - or if it is an issue at all. But, I thought I'd raise the issue.

In researching this issue #1490386: Hide breadcrumb trail in Admin for small screens - I set up Drupal 8 to view on my mobile device (iPhone - iOS6). As I navigated the admin interface (Seven), I realized that I was unable to view the breadcrumbs because they were blocked, along with much of the page, by the menu. When I selected a menu item, I expected the menu to go away and to see the page I had just selected. My first reaction, was that it was impossible to "hide" the admin menu and I began the process of reporting this as a bug.

Finally, I realized that I just needed to click on the menu item again, to close it. After feeling kinda stupid for a minute, I began to wonder if this might be a legitimate usability issue.

I'm not sure if this is standard behavior for this kind of mobile interface, because I don't actually do a lot of mobile web browsing myself. I do some. However, it seemed very counter-intuitive to me that selecting a menu item did not automatically close the menu.

The attached screen shot is using my browser, but I tested on my iPhone and got the same results.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

Title: Seven Theme - Navigation Menu Blocking Page - Usability Issue » Navigation Menu Blocking Page - Usability Issue
Component: Seven theme » toolbar.module

This involves the behaviour of the toolbar, Seven doesn't have control over this. I agree with the point raised though.

nod_’s picture

Issue tags: +JavaScript

tag

Wim Leers’s picture

Title: Navigation Menu Blocking Page - Usability Issue » Clicking menu links in the administration menu tray should close the admin menu tray
Issue summary: View changes
Issue tags: +js-novice, +Novice, +DrupalCamp Ghent 2014

Currently, Drupal 8's JS only listens for clicks on the toggles (the downward arrows), not on the links. The links are just regular links.

In order to implement this behavior, we must add an event listener for other click events.

So we currently have

    $(document)
      .on('click.toolbar', '.toolbar-handle', toggleClickHandler);

And we want another one to handle the regular links. All of those menu links are wrapped in a .toolbar-box div. So this is should be a matter of writing

    $(document)
      .on('click.toolbar', '.toolbar-box a', linkClickHandler);

And then in linkClickHandler tell ToolbarModel to disable its active tab (Drupal.toolbar.models.toolbarModel.set('activeTab', null)).

Wim Leers’s picture

mcjim’s picture

I just gave the code in #3 a try (thanks Wim) and it works great. The only thing we need to add is a check for whether the user has manually toggled the vertical orientation of the admin menu, in which case we'd want the menu to stay on screen.

mcjim’s picture

We could maybe check for toolbar-fixed on the body?

Tom Verhaeghe’s picture

#3 helped me a lot to find a solution. I also used Drupal.toolbar.models.toolbarModel.attributes.isFixed to check if the toolbar's position is fixed.

Tom Verhaeghe’s picture

Status: Active » Needs review
Wim Leers’s picture

Status: Needs review » Needs work

Works great, thanks! :)


Code review:

  1. +++ b/core/modules/toolbar/js/toolbar.menu.js
    @@ -38,6 +38,19 @@
    +     * Handle clicks from any toolbar-box link to decide whether to hide or show the menu.
    

    80 col rule: comments should not exceed 80 columns (characters) per line.

    Also: this can simply omit "to decide whether to hide or show the menu". That is covered by the comment in the function body.
    Hence please change that to something like "Handle clicks from a menu item link."

  2. +++ b/core/modules/toolbar/js/toolbar.menu.js
    @@ -38,6 +38,19 @@
    +      // If the menu is fixed and blocking anything on the page, then don't set disable the active tab after clicking.
    

    80 col rule again.

    I think the comment could be clearer:

    • "menu is fixed" -> "toolbar is positioned fixed"
    • "and blocking anything on the page" -> "(and hence hiding content underneath)"
    • "don't set disable" -> "deactivate"

    So something like "If the toolbar is positioned fixed (and hence hiding content underneath), then users expect clicks in the administration menu tray to take them to that destination but for the menu tray to be closed after clicking: otherwise the toolbar itself is obstructing the view of the destination they chose."

  3. +++ b/core/modules/toolbar/js/toolbar.menu.js
    @@ -38,6 +38,19 @@
    +      if (!Drupal.toolbar.models.toolbarModel.attributes.isFixed) {
    

    Please use toolbarModel.get('isFixed'); we should not access attributes directly. In JS, there is no such thing as language-enforced private or protected properties, but we enforce it by convention.

Wim Leers’s picture

Title: Clicking menu links in the administration menu tray should close the admin menu tray » Clicking menu links in the administration menu tray should close the admin menu tray, while in a narrow viewport where the toolbar is positioned on top of the content
Tom Verhaeghe’s picture

Status: Needs work » Needs review
FileSize
1.39 KB
1.17 KB

Fixing #9:
- Altered the way the attributes are accessed.
- Changed comments and descriptions to be more clear.
- Fixing code standards violation.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

Great, thanks!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 7c37dac and pushed to 8.0.x. Thanks!

  • alexpott committed 7c37dac on 8.0.x
    Issue #2103247 by Tom Verhaeghe | stpaultim: Fixed Clicking menu links...

Status: Fixed » Closed (fixed)

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