Toolbar sets the active-trail to the last clicked menu item, not the current menu item.

tagging

stevector, thank you for the patch. I'd like to consider a couple use cases to see if we can expand on the work you've done and improve the user experience even more.

When I first put this code in, I was thinking of these cases:

1. I'm navigating the administration menu in the Toolbar. I open the content top-level item to expose the Comments item. I click Comments and find a specific one in the list. I click to view that specific comment on a post. This means I've navigated from the administration theme's list of comments to a content item. I still expect the Content menu item to be expanded and the Comments menu item visible so I can click it and get back to the master list quickly. The requires that the Toolbar somehow knows that Content was opened by the user.
2. I type admin/modules into the URL bar in my browser. I expect the Admin menu item to be open in my Toolbar and the Extend menu item to be styled as active.
3. I open the Configuration menu item to expose the sub-items in the Toolbar. I navigate through a link in my theme to a new page. I expect the Configuration menu item to still be open on the subsequent page load.

Really what we're trying to build here are some smarts into the toolbar so that it feels like it's remembering what you were doing through page loads.

What do you think? Maybe we can improve the behavior of what's there rather than pulling out it?

Hm.

1) Is not what active-trail is about — the active trail always shows the current active trail of the current page. What you're talking about there is a history trail of last visited pages. Which is a nice feature on its own that can be found in various other applications, typically presented as a breadcrumb-alike list of reverse-chronologically ordered links. However, that's fundamentally different to the active trail and needs its own implementation. I actually think we have a concrete feature request issue for that in the core queue already.

2) is I think, what this patch here fixes.

3) is an expectation I'm not able to follow - I've the impression it sounds similar to 1).

I think that the patch here is correct - highlighting the active trail is a very simple job and should be bound to the currently shown page only.

Ok, active trail is distinct from the current open menu item.

I do think it's valuable to retain the current open menu item and re-open it across page loads. Here's a scenario. 1) I open the Configuration menu item. 2) I click on a node is a node listing page and navigate to a node detail page. 3) The Configuration menu collapses with this patch applied. Here's a video

http://screencast.com/t/WI8KvCKSjq

Well, TBH, what you call valuable there is a plain confusion from my perspective :)

If I'm not on a page that is not in the active trail of any administrative link, then I do not expect any administrative link to be highlighted.

This becomes clear very easily when navigating to some completely unrelated non-admin page after visiting a particular admin page. In that case, there's no relation at all, and thus no reason for any admin link to be in the active trail or be enabled.

Furthermore, /node/* pages are detached from the administrative pages - by design. There's no guarantee that there should be a relation between the two in the first place.

I think what you're having in mind is much rather along the lines of Contextual Administration module, which introduces entirely new router paths for entities, depending on where they appear in list views. So e.g. when viewing nodes on admin/content/node, and clicking the "edit" link on a particular one, then you would actually go to admin/content/node/123/edit instead of node/123/edit — i.e., ultimately retaining the previous administrative context for the following page. I know that @EclipseGc wants to make that happen (or at least possible) for D8, and it would be the proper handling to retain context, so I do not think that Toolbar should try to fake the active trail in any way.

This confused me a little too - I was constantly thinking (using the horizontal), do I have two menu's active? Although there is certainly value in having the admin in its previous state if you happen to be cruising between the page and the toolbar, there is also many cases where this isn't valuable.

Perhaps we can identify the cases where it, and where it isn't valuable?

I'm not putting up an objection to the patch here. If I can make my case more clear for stickiness of open menus, I'll do so in another issue. I've fine moving forward with this one.

I'm not putting up an objection to the patch here. If I can make my case more clear for stickiness of open menus, I'll do so in another issue. I've fine moving forward with this one.

Having done quite some front-end patch testing in the past week, I confirm this behaviour seemed buggy to me. "If I'm not on a page that is not in the active trail of any administrative link, then I do not expect any administrative link to be highlighted." pretty much sums it up. I'd frequently expand the toolbar from a page like node/1 and then be like "What? Why is 'People' highlighted?"

I think there is value in gathering / showing historic "menu trail" kind of navigation, but I don't think the toolbar feels like quite the right place to do it.

This patch restores behaviour to what I would expect. I am in no position to review the JS, though. :D So leaving "needs review."

This patch resolves the issue. It's been up for a review for a couple days. I'd like to suggest that we commit it.

Okie doke. Committed and pushed to 8.x. Thanks, stevector!