search

JeremyEnglert / JointsWP

A blank WordPress theme built with Foundation 6, giving you all the power and flexibility you need to build complex, mobile friendly websites without having to start from scratch.
http://jointswp.com
853 stars 272 forks source link

problem with active menu item #397

Open benthien opened 5 years ago

benthien commented 5 years ago

I inserted code in header.php that produces a top bar for medium and higher and an offcanvas menu for small devices. It works fine except the active menu item is not highlighted. I put class="active" on the menu item and made the following changes in _settings.scss

$dropdown-menu-item-color-active: $white; $dropdown-menu-item-background-active: get-color(primary);

I made similar changes for the offcanvas menu and that works fine.

garretthyder commented 5 years ago

Hi @benthien are you putting the active class on the li or the a element as I believe it should be on the li. Hope it's that simple otherwise will delve deeper. Cheers

benthien commented 5 years ago

I put the class on the link.

George

⁣George Benthien 511 Savoy Street San Diego, CA 92106 619-224-4317

Sent from TypeApp ​

On Apr 8, 2019, 5:02 PM, at 5:02 PM, Garrett Hyder notifications@github.com wrote:

Hi @benthien are you putting the active class on the li or the a element as I believe it should be on the li. Hope it's that simple otherwise will delve deeper. Cheers

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/JeremyEnglert/JointsWP/issues/397#issuecomment-481051029

garretthyder commented 5 years ago

Thanks for confirming @benthien moving the active class to the li element should resolve your issue. Let us know if you have any other issues. Cheers

benthien commented 5 years ago

You misunderstood my reply. I always had the active class on the link. My issue hasn't been resolved.

⁣George Benthien 511 Savoy Street San Diego, CA 92106 619-224-4317

Sent from TypeApp ​

On Apr 8, 2019, 8:05 PM, at 8:05 PM, Garrett Hyder notifications@github.com wrote:

Thanks for confirming @benthien moving the active class to the li element should resolve your issue. Let us know if you have any other issues. Cheers

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/JeremyEnglert/JointsWP/issues/397#issuecomment-481083660

garretthyder commented 5 years ago

Hi @benthien if your active class is on the link move it to it's parent

  • . The Foundation CSS expects it on the
  • and not the link element. Hope that clarifies.

    • benthien commented 5 years ago

    The various items on the top bar are in a list beginning with

    garretthyder commented 5 years ago

    Thanks @benthien looking at my site the .active css chain is .menu .active > a. So the .active class is on the li but there's also a .menu class on the ul. Is the menu class present in your build? Cheers

    benthien commented 5 years ago

    Below is the actual code I am using. The only page that uses WordPress is the blog. This code produces a top-bar for medium and above and an off-canvas menu for small. I use the same code in my other non-WordPress pages except that the links use relative addresses rather than absolute. The active class on blog produces the right result in the off-canvas menu, but produces no highlighting on the top-bar. Other than that everything works perfectly. In this case I can produce the highlighting on blog using css, but foundation normally does this automatically.

    Menu

    <!—off-canvas menu for small-->

    From: Garrett Hyder notifications@github.com Sent: Tuesday, April 9, 2019 5:02 PM To: JeremyEnglert/JointsWP JointsWP@noreply.github.com Cc: benthien benthien@cox.net; Mention mention@noreply.github.com Subject: Re: [JeremyEnglert/JointsWP] problem with active menu item (#397)

    Thanks @benthien https://github.com/benthien looking at my site the .active css chain is .menu .active > a. So the .active class is on the li but there's also a .menu class on the ul. Is the menu class present in your build? Cheers

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JeremyEnglert/JointsWP/issues/397#issuecomment-481483409 , or mute the thread https://github.com/notifications/unsubscribe-auth/ALf2hdGIAFBGJlwTexJ_6u8bxLZzJEz2ks5vfSnbgaJpZM4cjKm6 . https://github.com/notifications/beacon/ALf2hWtzPAu8Lau78ngPuh5Mjaf7__aKks5vfSnbgaJpZM4cjKm6.gif

    garretthyder commented 5 years ago

    Thanks @benthien that's definitely odd, your markup looks right. After you made the scss change did you npm run build to compile the SCSS to CSS? Also when you inspect the active element that should be highlighted (inspect the .active > a element) is there any CSS for the .active class on the element?

    benthien commented 5 years ago

    I inspected the blog element on the top-bar. It was obvious that the active color setting was being overridden by another css element. Since the code looked right, I figured that I must have entered a wrong setting in _settings.scss. I reinstalled the JointsWP files and slowly made changes. I made changes in the _settings file one by one and didn’t make any changes in the dropdown menu section. I was able to get the result I wanted, including active element highlighting, without any changes in the drop down menu. Here are the settings I originally made in _settings

    $dropdownmenu-background: $black;

    $dropdownmenu-submenu-background: $black;

    $dropdown-menu-item-color-active: $white;

    $dropdown-menu-item-background-active: get-color(primary);

    Leaving these settings at their original values we have

    $dropdownmenu-background: null;

    $dropdownmenu-submenu-background: $white;

    $dropdown-menu-item-color-active: get-color(primary);

    $dropdown-menu-item-background-active: transparent;

    I don’t understand the original settings, but they work.

    Thanks for your help

    George

    From: Garrett Hyder notifications@github.com Sent: Thursday, April 11, 2019 4:49 PM To: JeremyEnglert/JointsWP JointsWP@noreply.github.com Cc: benthien benthien@cox.net; Mention mention@noreply.github.com Subject: Re: [JeremyEnglert/JointsWP] problem with active menu item (#397)

    Thanks @benthien https://github.com/benthien that's definitely odd, your markup looks right. After you made the scss change did you npm run build to compile the SCSS to CSS? Also when you inspect the active element that should be highlighted (inspect the .active > a element) is there any CSS for the .active class on the element?

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JeremyEnglert/JointsWP/issues/397#issuecomment-482375423 , or mute the thread https://github.com/notifications/unsubscribe-auth/ALf2hRKk0ZFYIYBxoYMx3djy1sPHNBWHks5vf8nvgaJpZM4cjKm6 . https://github.com/notifications/beacon/ALf2hUJvTBBHMYyugsrLQsjCB94AQvuTks5vf8nvgaJpZM4cjKm6.gif

    garretthyder commented 5 years ago

    Thanks for that @benthien testing on a clean install I was able to reproduce your issue and found there's actually a JointsWP issue in place right now.

    It seems in Foundation 6.4.0 the active class was renamed is-active but active was still supported; "Menus now use is-active instead if active for active state. The old active works but is deprecated and will be removed." Release Notes - https://github.com/zurb/foundation-sites/releases/tag/v6.4.0

    The current foundation docs support this; "Add the class .is-active to any

  • to create an active state. You could apply this server-side to mark the active page, or dynamically with JavaScript." https://foundation.zurb.com/sites/docs/menu.html#active-state

    In JointsWP it seems we're still applying 'active' instead of 'is-active'; https://github.com/JeremyEnglert/JointsWP/blob/master/functions/menu.php#L84-L91

    I've updated this with back-compat in mind by leaving the 'active' class and simply introducing the 'is-active' class as well.

    Give it a try and let me know if that helps with your styling.

    @JeremyEnglert can you take a look as well my PR is in #398.

    Thanks

    • benthien commented 5 years ago

    If I leave $dropdown-menu-item-background-active at its default value of transparent in _settings.scss, class=”active ” produces the desired highlighting. If I inspect the active item on the top-bar, it is getting the background color from the css selector

    .menu .active>a. However, the behavior of is-active is different. In this case it gets the background color for the active item from

    .dropdown.menu>li.is-active>a which is by default transparent. I can change this by setting $dropdown-menu-item-background-active to get-color(primary). That produces the correct background on the active item, but also shows this background on any other dropdown menu item that I hover over. I think this is a foundation issue and not a jointsWP issue as I see similar behavior on non-wordpress pages.

    George Benthien

    From: Garrett Hyder notifications@github.com Sent: Sunday, April 14, 2019 3:31 PM To: JeremyEnglert/JointsWP JointsWP@noreply.github.com Cc: benthien benthien@cox.net; Mention mention@noreply.github.com Subject: Re: [JeremyEnglert/JointsWP] problem with active menu item (#397)

    Thanks for that @benthien https://github.com/benthien testing on a clean install I was able to reproduce your issue and found there's actually a JointsWP issue in place right now.

    It seems in Foundation 6.4.0 the active class was renamed is-active but active was still supported; "Menus now use is-active instead if active for active state. The old active works but is deprecated and will be removed." Release Notes - https://github.com/zurb/foundation-sites/releases/tag/v6.4.0

    The current foundation docs support this; "Add the class .is-active to any

    In JointsWP it seems we're still applying 'active' instead of 'is-active'; https://github.com/JeremyEnglert/JointsWP/blob/master/functions/menu.php#L84-L91

    I've updated this with back-compat in mind by leaving the 'active' class and simply introducing the 'is-active' class as well.

    Give it a try and let me know if that helps with your styling.

    @JeremyEnglert https://github.com/JeremyEnglert can you take a look as well my PR is in #398 https://github.com/JeremyEnglert/JointsWP/pull/398 .

    Thanks

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JeremyEnglert/JointsWP/issues/397#issuecomment-483063917 , or mute the thread https://github.com/notifications/unsubscribe-auth/ALf2hZMLilDnbqsdyJObOjkjz3T041M5ks5vg6wJgaJpZM4cjKm6 . https://github.com/notifications/beacon/ALf2hQdCwGjmk8KQz4_smfWAA6z_b2Ydks5vg6wJgaJpZM4cjKm6.gif