I've seen this issue in Chrome, Firefox and Safari on Mac with version 8.1.5 installed. I didn't check Windows/IE.

Due to a padding-top of 39px, the vertical toolbar tray can't be scrolled to reveal the last item (which is "Help")

You can see this behavior from the attached gif. It's a bit hard to see from the gif, but if you look closely, there's a menu item "Help" at the bottom before the menu expands, then an attempt is made to scroll down to "Help" again, but the scrolling can't go beyond "Reports", meaning there's no way to access the "Help" item from the vertical sidebar when there's a scroll bar shown.

Comments

svenryen created an issue. See original summary.

svenryen’s picture

The attached patch fixes the issue.

To reproduce:
1. Make the window narrow enough to display the toolbar tray as a sidebar
2. Expand some menu items in the toolbar tray so that the scrollbar is shown
3. Attempt to scroll to the bottom to see the last menu item in the toolbar tray.

The patch should apply to both 8.1.x-dev and 8.2.x-dev .

svenryen’s picture

Issue summary: View changes
FileSize
389.07 KB

Status: Needs review » Needs work
svenryen’s picture

svenryen’s picture

The patch fixed the issue, but had side effects for the horizontal bar. Here's a patch #6 that's been more thoroughly tested.

svenryen’s picture

A call to Drupal.displace() was required for the margin-top of the body to update properly after the orientation had been changed manually from vertical to horisontal.

svenryen’s picture

... and as a side-effect of applying the margin-bottom to fix the main issue, the min-height of 100% caused a scrollbar to always be shown for the toolbar tray, even when all items were visible and there was no need for a scrollbar. That's been fixed in the attached patch. Sorry for the long thread of patches and comments, it should be ready for review now.

svenryen’s picture

Issue summary: View changes
svenryen’s picture

Issue summary: View changes
svenryen’s picture

fusionx1’s picture

I was able to replicate the issue and tested #11, its finally working smoothly. Kudos to Sven

mccrodp’s picture

Status: Needs review » Reviewed & tested by the community

I reproduced the error on 8.1.7 following the steps in #2 and this patch fixes the problem. Great job Sven.

mccrodp’s picture

Issue summary: View changes
FileSize
64.18 KB
75.29 KB

Before:

After:

svenryen’s picture

Title: Can't scroll vertical toolbar tray to reveal last item (help) » Can't scroll vertical toolbar tray to reveal last item, applies to both mobile and desktop breakpoints (help)
Version: 8.1.5 » 8.1.x-dev

I checked the git commit history for the file in question in 8.1.x-dev and 8.2.x-dev as of today. The file in question has not been changes since 2015-11-12 23:46:06. I'm changing the version to reflect this is still an issue in 8.1.x-dev.

svenryen’s picture

Title: Can't scroll vertical toolbar tray to reveal last item, applies to both mobile and desktop breakpoints (help) » Can't scroll vertical toolbar tray to reveal last item, applies to both mobile and desktop breakpoints
xjm’s picture

Assigned: Unassigned » Cottser
Cottser’s picture

Assigned: Cottser » Unassigned
Issue tags: +JavaScript, +Needs subsystem maintainer review

Attempting to tag for JS subsystem maintainer review, I'm not familiar enough with displace to know if calling it like this is good or not.

Thanks for the work on this @svenryen!

droplet’s picture

Status: Reviewed & tested by the community » Needs work

this is overcomplicated at my first sight.

We could either drop `padding-bottom` to .toolbar-tray-vertical. Or `max-height: calc(100% - height)`

(IMO, with the `max-height` workaround we can leave the style there all time, we only hit the limitation at edge case.)

Drupal.displace is redundant in this case. On each toolbar toggle, we called `Drupal.toolbar.ToolbarVisualView.render` function that has a Drupal.displace calls at the final point.

To committers,
I'd like to have a final review if time is allowed :) I want to make sure there're no conflicts with #2542050: Toolbar implementation creates super annoying re-rendering. for our future work.

Thanks All!

Cottser’s picture

@droplet thanks for the lightning quick review! Yes you can definitely have final review, this is a bugfix so it can go in past beta freeze I think.

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.

nod_’s picture

It's been reviewed.