Skip to content
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

Merged
merged 6 commits into from
May 24, 2017
Merged

[5.5] Add patterns to routeIs method #19267

merged 6 commits into from
May 24, 2017

Conversation

Lloople
Copy link
Contributor

@Lloople Lloople commented May 18, 2017

Same behaviour as fullUrlIs. We can check for a group named routes following conventions. For example on a sidebar, checking for users.* we can set active to the dropdown when we are on users.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 :)

{
return $this->route() && $this->route()->named($name);
if ((! $route = $this->route()) || ! $routeName = $route->getName()) {
Copy link
Contributor

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

Copy link
Contributor Author

@Lloople Lloople May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without that parenthesis around the assignment of $route, the IDE is showing the next use of $route as an error (undefined variable 'route') on the assignment of $routeName.

undefined_variable_route

Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor Author

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

@Lloople Lloople changed the title [5.5] Add patterns to routeIs method [5.4] Add patterns to routeIs method May 20, 2017
@Lloople Lloople changed the base branch from master to 5.4 May 20, 2017 21:40
@Lloople Lloople changed the base branch from 5.4 to master May 20, 2017 21:40
@Lloople Lloople changed the title [5.4] Add patterns to routeIs method [5.5] Add patterns to routeIs method May 20, 2017
@ConnorVG
Copy link
Contributor

Thanks for the credit 😉

@taylorotwell
Copy link
Member

If there are added here do they need to be added to the route named method?

@Lloople
Copy link
Contributor Author

Lloople commented May 22, 2017

@taylorotwell since we can access the route through different classes or helpers, I think that this logic should move to the named method as you said, and call that method in routeIs. I can do the change tomorrow afternoon

@taylorotwell taylorotwell merged commit 0d66555 into laravel:master May 24, 2017
@taylorotwell
Copy link
Member

Updated all calls to use argument unpacking and variadic arguments.

@Lloople Lloople deleted the master branch May 24, 2017 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants