Sometimes the active trail may have to be partial. For example, suppose I have a blog post "My Blog Post". On most sites, there won't be a menu link for every single blog post—so the best active trail for this post might be ""News" -> "Blog posts". That trail is partial, since it doesn't include the title of the current page, "My Blog Post".

menu_breadcrumb doesn't deal with this well. It sees the same active trail for the "Blog posts" page, and for the "My Blog Post" page, so they will both get the same breadcrumbs. That feels inconsistent, since one is conceptually the parent of the other. A better behavior would be if the breadcrumbs for "My Blog Post" looked like "News" -> "Blog posts" -> "My Blog Post".

Here is one way to reproduce this:

  • Install menu_breadcrumb.
  • Create a basic page "Blog posts". Give it a menu link at the top level of the main menu.
  • Create an article "My Post". Do not give it a menu link.
  • Install context_active_trail and context_ui.
  • Create a new context. Add the condition "Node Bundle is of type 'article'". Add the reaction "Active trail", and select the menu item "Blog posts". Uncheck "Also set breadcrumbs". Save.
  • Visit "Blog posts". Its breadcrumbs are "Home" -> "Blog posts".
  • Visit "My Post". Its breadcrumbs are also "Home" -> "Blog posts", even though it's logically beneath "Blog posts"
CommentFileSizeAuthor
#20 2804943-partial-trails-20.patch6.26 KBvasi
#13 2804943-partial-trails-13.patch4.37 KBvasi
#4 2804943-partial-trails-4.patch4.16 KBvasi

Comments

vasi created an issue. See original summary.

vasi’s picture

vasi commented
Title: Poor handling of partial active trail » Improve handling of partial active trail
vasi’s picture

vasi commented

One solution to this is for menu_breadcrumb to automatically add the current page to the end of the links it looks at, unless it's already there. That would cause no change for pages that have a complete active trail, since their link will already be at the end of the active trail. But it would improve pages that have partial active trails. We'd end up with:

Page "Blog posts": Breadcrumbs are "Home" -> "News" -> "Blog posts"
Page "My Blog Post": Breadcrumbs are "Home" -> "News" -> "Blog posts" -> "My Blog Post"

vasi’s picture

vasi commented
Status: Active » Needs review
StatusFileSize
new 2804943-partial-trails-4.patch4.16 KB

And here's a patch that does that.

mvc’s picture

mvc
they
Primary language English
Location Montréal, CA
commented

It might be helpful for this to check the config options append_current_page and current_page_as_link.

vasi’s picture

vasi commented

Re #5: The patch just updates the $links array earlier on. Those options should still be handled properly by the existing code. Eg, if I deselect "Append current page to breadcrumb", I get:

Page "Blog posts": Breadcrumbs are "Home" -> "News"
Page "My Blog Post": Breadcrumbs are "Home" -> "News" -> "Blog posts"

rphair’s picture

rphair commented

Another idea: extension of the active trail by taxonomy membership (e.g., for blogs) is handled by the Taxonomy Attachment patch to Menu Breadcrumb, or by sandbox module Menu Breadcrumb with Attachment by Taxonomy (which has the same logic, except that it only generates a breadcrumb when there's a menu or a taxonomy match, so it can coexist with other breadcrumb builders).

rphair’s picture

rphair commented
Status: Needs review » Fixed
Related issues: +#2754437: add Taxonomy Attachment feature

The now posted beta version does this by design. I believe your idea of a "partial trail" applicable to things like blog entries corresponds neatly to the "Taxonomy Attachment" option now available on each menu in the new version. You would only have to tick that box on the menu containing your Blog listing pages themselves (e.g., "Blog Posts" in your original example) and the entries for that blog will inherit the Blog listing page's breadcrumbs.

The first patch to support this feature was made with blogs in mind, so it would be encouraging to hear that it covers your own use case. If you have any reservations about the implementation after you have tried it, please post here in the next couple of weeks.

vasi’s picture

vasi commented
Status: Fixed » Active

@rphair: Thanks for the feedback! It's cool that you've added Taxonomy Attachment to this module. It seems to be a really cool solution for "I am categorizing my blog posts, and I want each blog post to appear underneath its category."

However, I'm not sure it really solves the problem we've been seeing, which is more like "I want every single blog post to appear underneath 'Blog'". To do this with Taxonomy Attachment, I'd have to:

  • Create a new taxonomy, with one term "Blog"
  • Add a new taxonomy reference field to my Blog content type
  • Add the "Blog" term to every single Blog node
  • Replace the current overall "Blog" basic page on my site with the "Blog" term
  • Put the "Blog" term in my menu

As you can see, it's not quite the appropriate solution!

It sounds to me like they could be merged, however. If the Taxonomy Attachment code just set the active trail, then the patch I supplied would make menu_breadcrumb turn that into an appropriate breadcrumb. And then you'd get both the active trail and breadcrumbs all at once :)

rphair’s picture

rphair commented

dear @vasi, it sounds like you have put as much thought into this as I have so please be patient with me while I keep trying to understand exactly how what you are asking for would be generally useful.

To handle your use case, which is to show all blog entries independent of categorisation (for which it uses an article's Taxonomy entity reference & Taxonomy Attachment), the uncategorised blog can be a View rather than a taxonomy term. Drupal 8 comes configured this way (the Frontpage view of all promoted Articles) but another view than the Frontpage could be set up just as easily, targeting the bundle that corresponds to your Blog, without having to apply a taxonomy term to every node in that bundle.

For instance, I did all my testing in a site that was set up exactly as you describe, with the Blog view at the top level of the site's main menu, and the blog taxonomy pages as its menu children. Therefore the blog & its categories breadcrumb as follows, based directly on the active trail of those pages:

  • Home > Blog (View of all blog entries regardless of category)
  • Home > Blog > Category1...N (taxonomy pages)

For each blog entry in a category on the main menu, we would then have, if Taxonomy Attachment is selected for the main menu:

  • Home > Blog > Category > (optional article name)

For uncategorised blog articles, it will fall back to the system-provided Path Based breadcrumb. It could also fall back on a several-line breadcrumb builder you want write yourself, to breadcrumb the uncategorised articles in a way that's consistent with the blog categories.

Yet, in the case of uncategorised articles the breadcrumbs will be neither Menu based nor Taxonomy based so I still don't see how the behaviour you suggest could be brought within the scope of this module.

The taxonomy issue aside, I've looked at your patch itself and don't see how it's not covered by the two options in the Beta version that allow you to add the title of the Taxonomy attached page, either as a link or not. The code in the current release is already adding the current page to the breadcrumb trail (not to the active trail), although it will only do this based on Taxonomy membership... since it has to define that case somehow without relying on extra config outside of this module.

If I have understood you correctly, you want Menu Breadcrumb to bridge the gap if something creates an active trail which ends up missing only the current page. I can't (yet) see how modifying the general code, which we are trying to keep simple as possible, would be justified to deal with such a small number of special cases. It would probably make more sense to add this feature to that Context module, though I would much rather tell site builders to arrange their sites so uncategorised articles were displayed simply with a View.

I am happy to keep this open until we can establish exactly what Menu Breadcrumb is supposed to be doing and not doing, while it is still in Beta. If it can handle uncategorised articles without relying upon Context Active Trail (currently used on only 11 sites) then I would definitely be interested in hearing a plan simple enough to put into generally usable code.

vasi’s picture

vasi commented

@rphair, thanks for the response.

I've looked at your patch itself and don't see how it's not covered by the two options in the Beta version that allow you to add the title of the Taxonomy attached page

I must be misunderstanding something, then. If I'm not separating blog posts into distinct categories, how would I use this to get appropriate breadcrumbs? On most of the blogs I've maintained, either posts are not categorized at all; or they're tagged with multiple tags per post.

You mentioned we could just fall back to the system-provided path-based breadcrumbs. But if we wanted that, we wouldn't be using menu_breadcrumbs in the first place :) We like the features that menu_breadcrumbs gives us, like configuring whether the current page appears, what the Home link looks like, etc.

rphair’s picture

rphair commented

dear @vasi - I like the utility of adding menu based breadcrumbs for uncategorised Blog entries, and that is why I am trying to quantify what you are asking for.

The differences in how your patch looks up the title & adds it to the breadcrumb trail, while different than the Beta version (which I wish you would please factor any suggested patch against), aren't related to the main question I have.

The patch you've provided doesn't address the still-unmet use case of a blog with uncategorised articles. It literally only adds the current page to the breadcrumb trail: and it does this for every menu ticked in the config options whether it is functionally related to that menu or not. If I am wrong about this based on a mis-reading of your code, please be patient with me and explain why.

It is against the D8 design of Menu Breadcrumb to generate Menu Breadcrumbs no matter what... if the current page isn't linked to a menu, either by being on a menu or related with a well-defined rule (for now, only taxonomy membership), it doesn't generate a menu-based breadcrumb. This may sound arbitrary compared to the Alpha version standing up and saying "I'm the one & only breadcrumb builder, no matter what!" but that design went against the D8 paradigm allowing coexistence of breadcrumb builders.

I am only asking you to please come up with another well-defined rule which you believe covers uncategorised blog membership. Then and only then could we patch it into the code & I am happy to work with you on this. Simply adding the current page to every selected menu is not a general solution since it would solve your use case but would break others... for example, allowing a Tag based breadcrumb builder to take over in that case instead.

Would you please take a closer look at the Beta version and suggest any code changes to that version? I would love to see something that quantifies & grabs the uncategorised blog membership, would happily add this as a related feature, and if I could think of such a general way of coding it I would do it right now.

vasi’s picture

vasi commented
StatusFileSize
new 2804943-partial-trails-13.patch4.37 KB

I've re-rolled against 8.x-1.x.

I completely agree that we shouldn't set the breadcrumbs for every single page on the site! My patch makes no change at all to the applies() method—so it only modifies the breadcrumb when menu_breadcrumb already is in charge.

vasi’s picture

vasi commented
Issue summary: View changes

Specifically, the only case my patch does anything at all is when:

  • There is an active trail, so the menu_breadcrumb has already decided that it applies, and
  • The active trail doesn't have the current page at the end.

In any other case, my patch does nothing. It definitely doesn't change the breadcrumbs for arbitrary pages on the site.

Some examples, assuming "Append current page to breadcrumb" is enabled:

  1. Page "Foo" is not in the menu and has no active trail: menu_breadcrumb doesn't apply, some other module handles breadcrumbs. My patch changes nothing.
  2. Page "Blog" is in the menu, and has the active trail Home -> Blog: menu_breadcrumb applies as usual, and the page gets a breadcrumb Home -> Blog. My patch changes nothing.
  3. Page "My Post" is not in the menu, but some other module gives it an active trail Home -> Blog: menu_breadcrumb applies as usual. Without my patch, this would get the breadcrumb Home -> Blog. With my patch, it gets Home -> Blog -> My Post

I hope that clarifies how this works.

vasi’s picture

vasi commented

please come up with another well-defined rule which you believe covers uncategorised blog membership

That's a good question. The problem is that this changes depending on the site! Some sites will say "every node of type 'article' is a blog post", or "every node of type 'blog'". Other sites will say "Every page with a path like '/blog/*'" is a blog post. Other sites may have their own rules.

So, assuming that defining "what is a blog post?" may differ for different sites, how can the site builder specify this?

  1. Define it in code, eg: with a hook
  2. Define it in the UI, using Drupal's conditions

Obviously I prefer #2, which is why I built Context Active Trail, since Context provides a nice UI for using conditions. However, I understand that some people may not want to install a random extra module. And maybe some sites have a very complicated idea of "what is a blog post?", that can't be defined using Drupal's conditions.

So how about we add a hook? Maybe something like hook_menu_breadcrumbs_active_trail_alter($menu_name, &$trail_ids), or hook_menu_breadcrumbs_active_link($menu_name). A site builder could implement that hook to say "the active trail for this page is Home -> Blog", and menu_breadcrumbs would do the right thing.

rphair’s picture

rphair commented

thanks @vasi - now we are really getting somewhere. It should handle your use case well, but it breaks the taxonomy attached breadcrumbs when the current page is categorised. Even when this config option is not set:

Attach taxonomy member page to breadcrumb

This option affects breadcrumb display when the current page is a member of a taxonomy whose term is on a menu with "Taxonomy Attachment" selected, when it "attaches" to the menu-based breadcrumbs of that taxonomy term. In this case that term's menu title will show as a link regardess of the "current page" options above. Set this option TRUE to also show the the current ("attached") page title as the final breadcrumb.

... it displays the page title anyway. And when that option is selected for a taxonomy member, that breadcrumb displays twice. For the case of a blog, this can be seen on every categorised blog page.

This happens because you are adding the taxonomy attached page to the menu trail-derived breadcrumb list before the logic that comes later (beginning line 260 in your patched version) which adds the current page breadcrumb only if that option is set. Maybe your addMissingCurrentPage should go inside that if statement so the check for that config option would cover it.

Could you test more with a mix of categorised & uncategorised blog entries, and with that option setting on & off? Users of your originally described method should be able to set the menu trail with context_ui without this module's forcing the page title into the breadcrumb trail... written appropriately, they would be free to choose that based on the option setting.

So how about we add a hook? Maybe something like hook_menu_breadcrumbs_active_trail_alter($menu_name, &$trail_ids), or hook_menu_breadcrumbs_active_link($menu_name).

That sounds fun & flexible except that I would hope for a better base of adoption before adding an API. The huge adoption of this module in D7 was based on its simplicity so I don't want it to appear intimidating. Between use of views & context_ui as described above we probably have the general use cases covered. If not someone will surely request the hook as a feature (2.x) once we have some stability here.

rphair’s picture

rphair commented

p.s., once the addition of the current page is fully encompassed by that option setting (taxonomy attachment or not), I will rewrite the online documentation of that option to reflect the change. Please feel free to suggest alternate wording that matches the change in behaviour.

vasi’s picture

vasi commented

Sure, I'll do some more testing and polishing tomorrow.

rphair’s picture

rphair commented

that's awesome, though if still tinkering could you please factor against the dev version since there a couple extra commits there now (my mistake to ask for the patch relative to Beta :) )

vasi’s picture

vasi commented
StatusFileSize
new 2804943-partial-trails-20.patch6.26 KB

So it turns out the logic for with-attachment and without-attachment can be simplified with this patch! It seems to work both with full active trails, partial active trails, and taxonomy attachments.

  • rphair committed 86830a7 on 8.x-1.x authored by vasi 
    Issue #2804943 by vasi, rphair: Improve handling of partial active trail
rphair’s picture

rphair commented
Status: Active » Fixed

Really good stuff. Have included patch in dev & new beta2, with instructions to do a full cache clear since 2 new services being added (reinstalling will also work).

To wrap up this thread, what would you (@vasi) think of adding a final comment for site builders attracted to the idea of using context_active_trail to establish the breadcrumbs for uncategorised blog entries? Many will be able to figure it out from the OP but it would be more accommodating to have something HOWTO style.

vasi’s picture

vasi commented

Thanks for merging!

Before writing up a readme, there's a few issues we should address:

1. If you use Taxonomy Attachment, you get good breadcrumbs—but no active trail. Now that we support partial trails, we should look into having the taxonomies affect the trail too. Maybe we need a new module that supports multiple active-trail setters, just like there can be multiple breadcrumb builders?
2. If some blog posts are categorized, and others not, using both Taxonomy Attachment and Context Active Trail doesn't work well. I think solving #1 will solve this too.
3. menu_breadcrumb has some caching problems, so that you sometimes get stale or inappropriate breadcrumbs. I've encountered a couple of different situations, I'll try to reproduce and open a new issue for that.

rphair’s picture

rphair commented

@vasi, there is still something missing from how we handle the active trail, ever since the D8 Alpha version: views without arguments don't seem to have an active trail at all, hence no breadcrumbs. Didn't see this yet since it's not a problem with taxonomy views. Would you please take a look at new issue 2820206? (It's a different problem but maybe something in common with the solution you describe above... in the meantime I am stuck & really would appreciate your help.)

rphair’s picture

rphair commented

p.s. @vasi there is less urgency about #24 since I have linked that issue to a core issue: it is reasonable enough for modules to expect the menu.active_trail service to build the active trail for all pages. If there are still bugs in that, it would probably be better to leave active trail setting out of the breadcrumb building... one criticism of breadcrumb builders in D7 was that they ended up with hundreds of lines of code to cover special cases. Looking forward to your future issue re: caching & thanks for the help.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

rgnyldz’s picture

rgnyldz commented

Hey everyone, I can see that the content type is showing on a single node (like the examples of news or blog). But I think it is not translated. Tho the link is correct and translated bu the title isn't.