Skip to content

Implement exact-active class for exact url matches - resolves #68 #74

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

Merged
merged 3 commits into from
Nov 5, 2018
Merged

Implement exact-active class for exact url matches - resolves #68 #74

merged 3 commits into from
Nov 5, 2018

Conversation

Joucke
Copy link
Contributor

@Joucke Joucke commented Oct 12, 2018

        $this->menu = Menu::new()
            ->setExactActive(true)
            ->link('/', 'Home')
            ->link('/people', 'People')
            ->link('/people/sebastian', 'Sebastian')
            ->setActive('/people/sebastian');

now renders to

            <ul>
                <li><a href="/">Home</a></li>
                <li class="active"><a href="/people">People</a></li>
                <li class="active exact-active"><a href="/people/sebastian">Sebastian</a></li>
            </ul>

Tests are in

  • MenuSetActiveTest@it_can_set_items_exact_active
  • MenuSetActiveTest@it_only_sets_exact_active_on_exact_url_match
  • MenuSetActiveTest@it_can_render_a_custom_exact_active_class

Spatie\Menu\ExactUrlChecker is a clone of Spatie\Menu\ActiveUrlChecker, and only returns true if the url is an exact match.

@Joucke Joucke changed the title Implement exact-active class for exact url matches - fixes #68 Implement exact-active class for exact url matches - resolves #68 Oct 12, 2018
@Joucke
Copy link
Contributor Author

Joucke commented Oct 19, 2018

@sebastiandedeyne Please let me know if I need to improve this. Also, is the CI problem caused by something I did, or is a setting messed up somewhere?

Copy link
Member

@sebastiandedeyne sebastiandedeyne left a comment

Choose a reason for hiding this comment

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

Looks great! I think we should just always add the class when doing the active check though, don't really see the point in maintaining a flag for it.

@Joucke
Copy link
Contributor Author

Joucke commented Nov 2, 2018

@sebastiandedeyne should I let you know I added requested changes, or are you notified automatically? No idea how this works from owner pov, just asking :)

@sebastiandedeyne sebastiandedeyne merged commit deed817 into spatie:master Nov 5, 2018
@sebastiandedeyne
Copy link
Member

Perfect, thanks!

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.

2 participants