RobertBoes / inertia-breadcrumbs

Laravel package to automatically share breadcrumbs to Inertia
MIT License
44 stars 11 forks source link

[Feature Request] Support `null` breadcrumb URLs #14

Closed shengslogar closed 6 months ago

shengslogar commented 7 months ago

"Request a feature" links out to discussions, which isn't enabled on this repo. If there's a better place to file this, let me know.

I can't speak to other collectors, but diglactic/laravel-breadcrumbs supports null URLs, while the collector interface doesn't. (null URLs are used to define a parent breadcrumb that doesn't link anywhere, for visual purposes.)

https://github.com/RobertBoes/inertia-breadcrumbs/blob/d1a9f519d9afd806c2c51cf59063ca29f69831dc/src/Collectors/AbstractBreadcrumbCollector.php#L17-L24

This causes a TypeError exception with the following code, while using the RobertBoes\InertiaBreadcrumbs\Collectors\DiglacticBreadcrumbsCollector collector:

use Diglactic\Breadcrumbs\Breadcrumbs;
use Diglactic\Breadcrumbs\Generator as Trail;

Breadcrumbs::for('route.name', fn(Trail $trail) => $trail->push('Some Breadcrumb Without a Route', null));

It doesn't look like isCurrentUrl is overridden by any collectors, so updating the method signature there seems like a straightforward change. Let me know if a PR would be helpful.

Thanks for this package. I was thrilled to stumble upon it last year.

RobertBoes commented 7 months ago

Hey, thanks for the report, might've been an oversight on my part. Feel free to send a PR if you want to, otherwise I'll take a look at it in the coming days 👍