In the "menu" toolbar tray, clicking/tapping white space should show the child level

Closed #1850238: Should the menu item itself trigger expanding to the next level as a duplicate, but note that there's extra information there. Please read it. It's a *very* short issue (only 2 comments).

I gave it a try:

• When you click the 'white space', the menu should drop. Now I'd just like to know how to trigger the linkClickHandler when clicking on the .toolbar-box element...
• When a very long menu title is filled in (change Content into something else), the link text isn't displaying as good as it was before. I suppose I'd better use a to wrap the inner text and style that properly. Any hints on that?

• To do this, we need #2103247: 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 to land. Because then, we have an event handler for the clicking on the link itself. Then we can use JS's event bubbling to our advantage. If we then change
  

  .on('click.toolbar', '.toolbar-handle', toggleClickHandler)
  

  to

  .on('click.toolbar', '.toolbar-box', toggleClickHandler)
  

  , then toggleClickHandler gets called for every click event that happens within .toolbar-box, so also for the white space in the screenshot in the issue summary. This almost gets us there; there is but one problem: it also means that any time you click a link (i.e. linkClickHandler is called), we toggle also (i.e. toggleClickhandler is also called!). That's not our goal.

  For this, JS allows you to stop propagation in an event handler, so that the event doesn't bubble up further. Due to event bubbling, the event handler of the elements closer to the event are called first, hence .toolbar-box a's event handler (i.e. linkClickHandler) is called before .toolbar-box's event handler (i.e. toggleClickHandler). So if we stop propagation in linkClickHandler, toggleClickHandler won't be called anymore, and voila: problem solved. You stop event propagation by calling event.stopPropagation().

• I experimented a bit locally and came up with an alternative approach. See attached patch.

Applied the patch from issue #2103247 and changed the triggering element (.toolbar-handle) to the parent element (.toolbar-box) for toggleClickHandle.

Also, changed some CSS, making .toolbar-box's display attribute inline-block and removing paddings.

#2103247: 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 has now landed (congrats!), so now you can pull out that part of the patch :)

I'll reroll that tonight :-)

Wonderful, thanks :)

Rerolled previous patch

I messed up in the previous patch; accidentaly defining 2 linkClickHandlers.

Works perfectly in manual testing. Looks visually identical.

Beta evaluation added to the IS.

Committed cba3bf6 and pushed to 8.0.x. Thanks!

This introduced a regression in the focus styling of the tray links, which look kind of bad now:

Also I've found this change really annoying since it was introduced. Maybe we can tweak it but just increasing the width of the dropdown trigger a bit more instead of reducing the link so much?

• What exactly looks bad? The fact that a smaller part of each "row" gets the gray background now? It never was the entire row, IIRC?
• Despite me only wanting to fix this to make the Toolbar component issue queue more manageable again, I've actually found this to be very handy. What exactly annoys you about it? The one thing that I've found to be annoying is that if you're at a level where there are no more children, clicking the whitespace area does not redirect us to the link. That'd be a further improvement. Would that address your concerns?

Sorry! I had a busy week and forgot about this issue.

What exactly looks bad? The fact that a smaller part of each "row" gets the gray background now? It never was the entire row, IIRC?

From beta3:

Despite me only wanting to fix this to make the Toolbar component issue queue more manageable again, I've actually found this to be very handy. What exactly annoys you about it?

I think the intention of this issue is good, it just needs some fine tuning, take the image in #18

The width of the handle hit area is disproportional, there is definitely a middle area between both where it's much closer to the menu item than the handle, I expect the menu item to activate there.

Ideally we could have something like this, where the handle hit area is big enough to prevent accidental taps on the menu item but not so bit that it throws off intentional taps on the menu item.

Ok so it was the entire row that got the gray background.

I agree with the visual problem (the entire row should still get that grayness), but I disagree with the ideal hit areas you propose. "text == menu item hit area" is simple. "text plus some white space === menu item hit area" is impossible to convey, even more so because the width of the text varies, which would make that nearly impossible to implement.

The visual problem is clear. But do you really have problems with the hit area?

I have had problems yes, but let's try and make this more objective.

which would make that nearly impossible to implement.

This is simple to implement, I already played around with it in the web inspector, but let's not go into implementation details until we are sure we should change it.

"text == menu item hit area" is simple. "text plus some white space === menu item hit area" is impossible to convey

It's not about what is possible to convey, because the current behaviour is also not communicated, it's just falling back to the less destructive behaviour. I think it's more what is the expected behaviour based on common implementations of this UI component. If we agree that this is the point then hopefully some research into current implementations could resolve this?

I noticed that this also affects items that don't have handles. I'll write a proposal patch soon.

I've posted a patch not knowing this discussion was happening: #2409427: Toolbar vertical orientation link not expanding to fill the space.

I agree it should be clickable as the link.

Let's move the follow up discussion there.