-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
[5.5] Add patterns to routeIs method #19267
Conversation
src/Illuminate/Http/Request.php
Outdated
{ | ||
return $this->route() && $this->route()->named($name); | ||
if ((! $route = $this->route()) || ! $routeName = $route->getName()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Useless extra brackets; if (! $route = $this->route() || ! $routeName = $route->getName()) {
is just fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working fine with PHP 7.0.12 without the extra brackets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Salomoni Tried without brackets, not passing tests. Adding brackets again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lucasmichot is there any action I need to do in order to get this PR checked? First time PRing here and don't know if I'm missing something
Thanks for the credit 😉 |
If there are added here do they need to be added to the route |
@taylorotwell since we can access the route through different classes or helpers, I think that this logic should move to the |
Updated all calls to use argument unpacking and variadic arguments. |
Same behaviour as
fullUrlIs
. We can check for a group named routes following conventions. For example on a sidebar, checking forusers.*
we can set active to the dropdown when we are onusers.edit
,users.index
, etc.Not sure if this should point to 5.4. I can do another pull request if needed
Thanks to @ConnorVG for helping with this :)