Problem/Motivation

In prototype_preprocess_breadcrumb() we're always making the assumption that we want the current page in the breadcrumb trail. IMO this isn't something Prototype should decide, as other modules handle this much cleaner. Leveraging a module like Custom Breadcrumbs makes this exposable, and it cost me some time figuring out why my settings weren't respected. Looking into the theme for this didn't cross my mind for a while. We should remove this logic and return that functionality to modules.

Issue fork prototype-3557204

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jnettik created an issue. See original summary.

jnettik opened merge request !206

jnettik’s picture

jnettik
Primary language English
Location Denver, CO
commented
Status: Active » Needs review

Removed preprocess from theme. Needs review.

jldust’s picture

jldust commented
Status: Needs review » Postponed (maintainer needs more info)

@jnettik Regarding the breadcrumbs the reason that was add was to support accessibility, can you confirm that the suggested modules supports that or another solution to maintain that feature?

jnettik’s picture

jnettik
Primary language English
Location Denver, CO
commented

What is the accessibility problem this solves? Are we supposed to always have a home link on breadcrumbs? The Custom Breadcrumbs module does have settings to add this.

jldust’s picture

jldust commented
Assigned: Unassigned » jldust
Status: Postponed (maintainer needs more info) » Needs review

Noting that as of 10.3 some of this code is now redundant and should be reconsidered

jldust’s picture

jldust commented

In addition to the .theme logic, I've updated the component to support this change.

pixelwhip’s picture

pixelwhip commented
Status: Needs review » Reviewed & tested by the community

Code looks good. There are some unrelated changes but those look to just be code formatting improvements. I've tested this locally and it works as expected.

  • jldust committed 0297d856 on 5.x authored by jnettik 
    #3557204: Remove current page breadcrumb from theme.
jldust’s picture

jldust commented

This has been merged in and will be included in the next tagged release.

jldust’s picture

jldust commented
Status: Reviewed & tested by the community » Fixed

This has been included in the most recent release

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

jldust’s picture

jldust commented
Status: Fixed » Closed (fixed)