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

Multiple Menus #32

Closed
starflower opened this issue Apr 29, 2013 · 10 comments
Closed

Multiple Menus #32

starflower opened this issue Apr 29, 2013 · 10 comments

Comments

@starflower
Copy link

I have two menus in my navigational area. One for languages and one for the main navigation, which are different lists. How do I set up this as one responsive Nav?

@juliankrispel
Copy link

Not supported I guess, I'll be making a pull request shortly for this.

@giorgio79
Copy link

Fantastic! Most sites have a main menu, plus a user menu, so this update is essential. What is the hold up with getting this committed? More reviews are needed?

@momendo
Copy link

momendo commented May 31, 2013

@viljamis seems to be more responsive on Twitter than on Github. @juliankrispel What do you need? A review or maybe you need to summerize the patch for @viljamis

@juliankrispel
Copy link

@momendo I've summarized it well enough in my pull request I believe. the plugin author just needs to react. I can't accept a pull request for him, because it's not my repository.

@arielsalminen
Copy link
Owner

@momendo @juliankrispel The reason why I haven't merged this yet is that this doesn't seem to calculate the height correctly. For me it returns height always as "undefined" if the new option is used, which then breaks the transition.

Another issue, but not that big one, is that this pull request doesn't follow the guidelines which can be found here: https://github.com/viljamis/responsive-nav.js/blob/master/CONTRIBUTING.md

@juliankrispel
Copy link

thanks for responding @viljamis I'll update my pull request shortly

@arielsalminen
Copy link
Owner

@juliankrispel I think the problem is this line (L386) and that it's never actually true:

if (typeof nav.inner === "array") {  }

If you update the pull, can you also format all the code as the rest of the code has been formatted (using double quotation marks and similar spacing) and make sure that there are no typos? (I spotted few).

Also, is this line (L382) really needed?

window.e = nav.inner;

@juliankrispel
Copy link

will do! thanks again! Ouch, did I leave that in there. My bad, I appear to have rushed that pull request...

@giorgio79
Copy link

Looks like here is a working patch #45

@arielsalminen
Copy link
Owner

This is now supported.

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

No branches or pull requests

5 participants