Toolbar vertical orientation link not expanding to fill the space.

This change happened here #1855066: In the "menu" toolbar tray, clicking/tapping white space should show the child level

Tested in Firefox 35.0 & Chromium 39.0.2171.65 on Linux and seems to fix the reported issue

Tested on Safari Version 8.0.2 (10600.2.5) and Chrome Version 40.0.2214.85 beta (64-bit) on OXS Yosemite 10.10.1. Fixed reported issue

Tested and Reviewed in Chrome and Firefox on linux.
also tested in windows - opera, chrome,firefox and ie 11.

attached screenshots for linux.

Screenshot on Chrome:

I'd like some feedback from Wim before we commit this. We were discussing this in #1855066: In the "menu" toolbar tray, clicking/tapping white space should show the child level

Applied the patch and works as expected.

Tested on Firefox + Chrome on OSX.

Chrome (OSX)

Firefox (OSX)

This requires more discussion. Lets leave it at 'Needs review" for now, please.

Oops

With the toolbar in vertical orientation, clicking in the middle of the whitespace between the "Structure" link text and the arrow…

HEAD

… toggles the child level, i.e. it's equivalent with clicking the arrow. This is the behavior that #1855066: In the "menu" toolbar tray, clicking/tapping white space should show the child level introduced (as that issue title also says).

this patch

… opens the "Structure" page, i.e. it's equivalent with clicking the "Structure" link.

Sorry :(

I don't have supporting UX patterns, only the UX that benjifisher — the person who opened that other issue — and I found frustrating: those arrows are frustratingly small hit areas on touch devices:

I vote for making the white space do the same as the icon. For one thing, the icon is small and easy to miss with my fat fingers. For another thing, anything that reduces the chance of an accidental page load (slow) compared to changing the state of the menu (fast) is a good idea IMHO.

I'd swear #1855066: In the "menu" toolbar tray, clicking/tapping white space should show the child level got a usability review, but apparently/clearly it did not. So if the behavior introduced there should be undone for UX reasons, so be it. This is also what Lewis Nyman said in #1855066-22: In the "menu" toolbar tray, clicking/tapping white space should show the child level: I think it's more what is the expected behaviour based on common implementations of this UI component.

I don't have supporting UX patterns, only the UX that benjifisher — the person who opened that other issue — and I found frustrating: those arrows are frustratingly small hit areas on touch devices:

But the solution goes too far the other way, the hit areas for the arrows are now too big in some situations, and the parent link is too small.

I don't think this is an either/or issue. We can solve both of these problems by increasing the width of the arrow manually while maintaining the full width behaviour added in the patch in this issue.

Sounds good!

Sorry for the delay on my part here. And for getting it wrong the first time.

Working as expected. The whole button is getting the hover/active state effect and the sub-menu button area is getting greyed out when clicking on the toggle arrow.

Changes look good! I found some css standards that should to be fixed before committing

1. +++ b/core/modules/toolbar/css/toolbar.icons.css
   @@ -59,6 +59,15 @@
   +.toolbar .toolbar-tray-vertical .menu a {
   +  padding-left: 2.75em;
   +  padding-right: 4em;
   +}
   

   For direction specific property/values, add the comment /* LTR */ on the same line preceded by a single space.

2. +++ b/core/modules/toolbar/css/toolbar.icons.css
   @@ -59,6 +59,15 @@
   +[dir="rtl"]  .toolbar .toolbar-tray-vertical .menu a {
   

   Found two spaces after [dir="rtl"], expected one space.

3. +++ b/core/modules/toolbar/css/toolbar.menu.css
   @@ -14,7 +14,7 @@
    .toolbar .toolbar-box a {
   -  display: inline-block;
   +  display: block;
    }
   

   display: block; is the default for links in the toolbar, so this rule can be removed. See toolbar.module.css line 41/44.

   .toolbar a {
     display: block;
     line-height: 1;
   }

@joelpittet Thanks for picking this up.

I hope I got the LTR comment right, I'd much rather put the LTR stuff in [dir="ltr"] stuff and be explicit about it.

You are not the first to argue this and there are more that agree :). Let's follow the current standards for now and leave this discussion for a followup.

Just a minor issue left:

+++ b/core/modules/toolbar/css/toolbar.icons.css
@@ -61,9 +61,9 @@
 .toolbar .toolbar-tray-vertical .menu a {
   padding-left: 2.75em;
-  padding-right: 4em;
+  padding-right: 4em; /* LTR */
 }
-[dir="rtl"]  .toolbar .toolbar-tray-vertical .menu a {
+[dir="rtl"] .toolbar .toolbar-tray-vertical .menu a {
   padding-left: 4em;
   padding-right: 2.75em;
 }

The /* LTR */ comment is per attribute, so it should be added to padding-right as well as padding-left.

Cheers! All CSS coding standards have been addressed.

Screenshot after for good measure:

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 0fe7e02 and pushed to 8.0.x. Thanks!