-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Version 2.0 #8
base: master
Are you sure you want to change the base?
Version 2.0 #8
Conversation
Hi @dwightwatson , are you not considering adding breadcrumbs to your routes, for example, davejamesmiller/laravel-breadcrumbs#209? |
Please tell me, these changes will affect Breadcrumbs::for('home', function (Generator $trail) {
$trail->then('Home', route('home'));
}); Right now I have ide tips for the passed object. Are they saved using I am also worried about the possibility of using short functions: Breadcrumbs::for('home', fn (Generator $trail) =>
$trail->then('home')->add('About')
); Will there be problems using |
Registering the breadcrumb directly on a route is an interesting idea. I don't love the idea of a Not sure about IDE tips - will need to look into that. However, it's my understanding that Breadcrumbs::for('home', fn() => $this->then('home', route('home'))); Might need to rethink the method There's a lot of ideas here basically - but now that DJM's breadcrumbs package has been archived I'd like to bring mine up to speed so I can swap it out in my newer apps. |
The IDE will definitely give the wrong hints if it is used in any class: class AppServiceProvider extends ServiceProvider
{
public function boot()
{
Breadcrumbs::for('home', fn() => $this->then('home', route('home')));
}
public function foo(): bool
{
return false;
}
} Gives me a hint Yes, indeed, the route file will become larger by defining breadcrumbs, but I still think it would be a great step. I forked the current package and added the definition to the routes. It looks like this in my working application: Route::screen('notifications/{id?}', NotificationScreen::class)
->name('notifications')
->breadcrumbs(fn (Trail $trail) => $trail
->parent('platform.index')
->push('Notifications')
);
Route::screen('search/{query}', SearchScreen::class)
->name('search')
->breadcrumbs(fn (Trail $trail, string $query) => $trail
->parent('platform.index')
->push('Search')
->push($query)
);
Route::screen('roles/{roles}/edit', RoleEditScreen::class)
->name('platform.systems.roles.edit')
->breadcrumbs(fn (Trail $trail, Role $role) => $trail
->parent('platform.systems.roles')
->push('Role', route('platform.systems.roles.edit', $role))
); I understand that people may not like this, but are you considering this an additional possibility? |
I'm not overly familiar with IDEs - Sublime Text user here. I wonder if there's a magic comment I can put when calling the closure that would indicate to any IDEs what the context of I'll definitely consider allowing breadcrumbs to be added directly on routes. For now I'm just working on getting the new API working and polishing a few underlying issues I'm having. |
I didn’t succeed. Such things are already present in Laravel, for example: Route::macro('example', function ($url) {
return $this->get($url); // Here $this is the object of the Route
}); And he also gives false advice.
I would be happy to help with this. |
Cool, let me nail down some of the core bits and pieces first. As far as IDE typehinting it's not going to be a priority for me. But if there is an easy way to implement it I don't see any issue with having it. The problem I'm dealing with now is passing in route parameters. By default I'll pass in all the route parameters directly.
However I might actually want the I'm thinking calling |
Now the definition of binding in Laravel, works thanks to reflection. Installed in SubstituteBindings middleware. Which calls ImplicitRouteBinding Which in turn is tied to the performer: //src/Illuminate/Routing/Route.php:475
public function signatureParameters($subClass = null)
{
return RouteSignatureParameters::fromAction($this->action, $subClass);
} I did not see the possibility of passing an argument to change the view from
I have never had such a need. |
Upgrade Laravel 8 Version Constraint
PHP 8 support 🚀
This PR is working on a new major release of breadcrumbs, supporting the later versions of Laravel with a brand new API. It'll take advantage of the new improvements in Laravel and drop unnecessary complexities.
Some key differences:
$trail
in as the first parameter anymore - it's bound to the context as$this
.parent
andadd
withextends
andthen
which I think are more readable - not sure yet. May keep the old names in for those with preferences.