justintadlock / breadcrumb-trail

Official home of the Breadcrumb Trail plugin for WordPress.
GNU General Public License v2.0
136 stars 61 forks source link

Support multiple crumb anchors in a single item #35

Open lkraav opened 7 years ago

lkraav commented 7 years ago

Use case: e-learning environment, where courses can belong to multiple certificate programs.

Breadcrumb structure goal (pardon the ASCII limitations, vertical-align: middle would be nicer):

Certificate Uno > Track Some 
                              > Course > Lesson
Certificate Dos > Track Other 

HTML (styled into multi-line later):

<a href="/certificate/uno">Certificate Uno</a><a href="/certificate/dos/">Certificate Dos</a>

Solution: seems simple on surface, https://github.com/justintadlock/breadcrumb-trail/blob/1.1.0/inc/breadcrumbs.php#L200 should become greedy instead of lazy. Can we lose the lazy ? from matching between <a></a>?

preg_match( '/(<a.*?>)(.*)(<\/a>)/i', $item, $matches );

Side effects: none that I can see on 1.0.0, but I have yet to test 1.1.0.

Alternatively, it would help if the preg_match() call is put through a filter, so I could run my own regex on it.

justintadlock commented 7 years ago

I don't think there'd be an issue changing the regex. However, I do want to note that I have plans changing how a lot of this works in a future version to the point where I wouldn't need the regex at all.

Right now, trail items are defined like:

$items[] = '<a href="#">Label</a>';

That's pretty inflexible and means for a lot of duplicated code just making the markup. Eventually, I'd want something more along the lines of:

$trail_items[ $some_key ] = array(
    'label' => 'Example',
    'url'   => '#',
    'attr'  => array( 'class' => 'example' )
);

Then, I'd have a function that handled the markup.

I mention this because I'd want to think about that future compatibility before making a change now that would break something later.

lkraav commented 7 years ago

Oh yeah, that plan sounds fine to me. My subclass can easily adjust. PS hybrid_attr() is the best thing since sliced bread.

For time being I'm deploying a patch on top of hybrid-core-3.1. If the regex change goes forward for a 1.1.1 release or whatnot, I'll be able to drop it. Even though I can't recall right now if hybrid-core-3.x are getting any extension updates backported or not..