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.
Comment | File | Size | Author |
---|---|---|---|
#11 | interdiff.txt | 1.17 KB | Tom Verhaeghe |
#11 | clicking_menu_links_in-2103247-11.patch | 1.39 KB | Tom Verhaeghe |
Comments
Comment #1
LewisNyman CreditAttribution: LewisNyman commentedThis involves the behaviour of the toolbar, Seven doesn't have control over this. I agree with the point raised though.
Comment #2
nod_tag
Comment #3
Wim LeersCurrently, 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
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 writingAnd then in
linkClickHandler
tellToolbarModel
to disable its active tab (Drupal.toolbar.models.toolbarModel.set('activeTab', null)
).Comment #4
Wim Leerslewisnyman +1'd this issue at #1800584-14: Allow the toolbar tray to be dismissed with a swipe event.
Comment #5
mcjim CreditAttribution: mcjim commentedI 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.
Comment #6
mcjim CreditAttribution: mcjim commentedWe could maybe check for
toolbar-fixed
on the body?Comment #7
Tom Verhaeghe CreditAttribution: Tom Verhaeghe commented#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.Comment #8
Tom Verhaeghe CreditAttribution: Tom Verhaeghe commentedComment #9
Wim LeersWorks great, thanks! :)
Code review:
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."
80 col rule again.
I think the comment could be clearer:
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."
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.Comment #10
Wim LeersComment #11
Tom Verhaeghe CreditAttribution: Tom Verhaeghe commentedFixing #9:
- Altered the way the attributes are accessed.
- Changed comments and descriptions to be more clear.
- Fixing code standards violation.
Comment #12
Wim LeersGreat, thanks!
Comment #13
alexpottThis 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!