milohuang / reverie

Reverie is a versatile HTML5 responsive WordPress framework based on ZURB's Foundation.
http://theakiba.com/reverie/
MIT License
916 stars 196 forks source link

wp_nav_menu and has-flyout class #61

Closed hCante closed 11 years ago

hCante commented 12 years ago

In the demo, the drop-down menu seems to work correctly but locally does not work for me unless I add the following code to functions.php (Basically adding the class "has-flyout") I'm using reverie 3.0.0 and wp 3.4.1. That class does not seem to be on the theme.

add_filter('wp_nav_menu_objects', function ($items) {
    $hasSub = function ($menu_item_id, &$items) {
        foreach ($items as $item) {
            if ($item->menu_item_parent && $item->menu_item_parent==$menu_item_id) {
                return true;
            }
        }
        return false;
    };
    foreach ($items as &$item) {
        if ($hasSub($item->ID, &$items)) {
            $item->classes[] = 'has-flyout';
        }
    }
    return $items;    
});
milohuang commented 12 years ago

True, that's why you need to add the class has-flyout manually in the WP dashboard.

Just run your snippet and it works great. Will include it.

hCante commented 12 years ago

great, thanks.

jonjennings commented 12 years ago

This bit of code appears to break on both Bluehost and Bluefur (and maybe other hosts with Blue in their name... ) with WP 3.4.1. It runs fine on a local LAMP install though. I'm wondering if the fact that both hosts are on PHP 5.2.17 and my LAMP stack is running 5.3 might be a factor.

ETA: I switched my Bluehost account to run PHP 5.3 & it runs fine now. So seems it is a language issue. I wonder if the code could be rewritten to work on PHP 5.2 - I'm sure a lot of hosting providers are still running that.

awshout commented 12 years ago

Anonymous functions do require PHP 5.3. I was also looking into another way to achieve this with PHP

As an alternative, this class can be added using jQuery $('.nav-bar>li').has('ul').addClass("has-flyout");

milohuang commented 12 years ago

Thanks folks for the test. It works on my local MAMP but not on the server (missed this part). The code appears to be from the codex of WP: http://codex.wordpress.org/Function_Reference/wp_nav_menu#How_to_add_a_parent_class_for_menu_item

JQuery does work. If there is no other way, will do it wil Jquery.

hCante commented 12 years ago

You guys are right. it works for me on the live server too, but I'm on PHP 5.3.13.

jonjennings commented 12 years ago

There's probably a better way, but unrolling all the functions would fix it. Something like:

add_filter('wp_nav_menu_objects', 'parse_nav_menu_objects');

function parse_nav_menu_objects($items) {
    foreach ($items as &$item) {
        if (has_submenu($item->ID, &$items)) {
            $item->classes[] = 'has-flyout';
        }
    }
    return $items;    
};

function has_submenu($menu_item_id, &$items) {
    foreach ($items as $item) {
        if ($item->menu_item_parent && $item->menu_item_parent==$menu_item_id) {
            return true;
        }
    }
    return false;
};

[That doesn't seem to give me flyout sub-menus. But I'm new to Reverie & Foundation and I don't appear to get that with the original code on PHP 5.3, so I figure there's something that I need to do for that :-) ]

petergus commented 12 years ago

im also not very knowledgeable here but i used this code recently to add first and last classes to wp_nav_menu.... maybe something similar could be done? http://gist.github.com/3048476

awshout commented 12 years ago

Two different ideas... The first is this:

add_filter( 'nav_menu_css_class', 'check_for_submenu', 10, 2);
    function check_for_submenu($classes, $item) {
    global $wpdb;
    $has_children = $wpdb->get_var("SELECT COUNT(meta_id) FROM wp_postmeta WHERE meta_key='_menu_item_menu_item_parent' AND meta_value='".$item->ID."'");
    if ($has_children > 0) array_push($classes,'has-flyout');
    return $classes; 
}

and the second is a complete rewrite of the walker (plus other things): https://github.com/awshout/reverie-awfoundrie/commit/1f5e6b0d770b6cd698f9f8da00d24f9948c8b296

milohuang commented 12 years ago

Interesting awshout, the first method works for me. Thanks, will include it!

jonjennings commented 12 years ago

It's not wise to be running SQL queries directly on the WP database. The database layout does change occasionally & code that queries it directly may break. If there's a way to access that information via the API, it's much preferable to take that route. Secondly, the table name is dependent on the table prefix chosen during installation. "wp_" is the default but for many installations it'll be something else. Again, accessing the information via the published API should separate us from having to allow for this. Apologies that I don't have a more positive suggestion :)

awshout commented 12 years ago

Very good points. I was just going to respond and say using the walker would be a better way to go about it. My code for the walker isn't perfect, but I'm working on it.

ElectronsCloud commented 12 years ago

There is no way I can make it work (at least using PHP 5.2x). Anybody knows how to do it using JavaScript?

deeve007 commented 12 years ago

Was just about to post about this issue, but see it's been spotted by many. Would really like to work out a good solution, I was interested in starting to use this as my default dev theme.

milohuang commented 12 years ago

As awshot mentioned, you can use $('.nav-bar>li').has('ul').addClass("has-flyout"); for jQuery method. Add it to your app.js

deeve007 commented 12 years ago

That actually does't work. An arrow appears indication a dropdown, but no dropdown appears when rolling over. This method works to some extent: http://codex.wordpress.org/Function_Reference/wp_nav_menu#How_to_add_a_parent_class_for_menu_item

However it only works for the first level dropdown. If you have another level, it doesn't appear, when I would have presumed it should flyout horizontally to the right, no?

deeve007 commented 12 years ago

Here's a screenshot showing the issue I'm having: http://dl.dropbox.com/u/220361/reverie_bug.jpg

This is on a clean install of wordpress and reverie, with just the wordpress codex solution above added to functions.php, so that "has-flyout" has been added to parent li's.

You can see that the The class has been added to li's as required, but that the second level dropdown ("SMH") isn't appearing properly.

awshout commented 12 years ago

Unfortunately, the Foundation nav bar doesn't support another level http://foundation.zurb.com/docs/navigation.php

deeve007 commented 12 years ago

Oh really? Damn, that may be an issue, and this is so much a better base theme than skeleton.

deeve007 commented 12 years ago

Okay, this gives some hope I think: https://github.com/zurb/foundation/issues/330

Are they saying that branch they refer to has code for multi-level dropdowns? Am reviewing it now to try to work out what's going on, and what code I may need to update. Any help appreciated, I'm not a coding guru. And if we work it out, we could add something to this code as a branch or addon, no?

awshout commented 12 years ago

They have a top-bar branch that includes a fixed menu at the top of the page. It does support multi-level menus and could be modified to fit your needs. I've been working with it off and on to get it working with WordPress, but it has been a bit tricky.

petergus commented 12 years ago

i have created a menu based on skeletons (well basically 'forked' :) ) and integrated it with Reverie. Added superfish and lavalamp to boot! so far it works pretty good, and i plan to post it to my hub soon.

deeve007 commented 12 years ago

Hmm, I had a comment deleted just above this one??

I had basically suggested that I might need to work on the CSS just for that third level, and maybe post as an "add on" or similar for reverie?

gorelog commented 12 years ago

I got it to work by using the jQuery solution as stated above and then add this to the CSS:

.nav-bar li.has-flyout:hover > ul.flyout { display: block; }

This is the jQuery: $('.nav-bar>li').has('ul').addClass("has-flyout");

Note: I've only tested on the latest Firefox and Safari.