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

Responsive menu sub-menu bug #50

Open
khawkins98 opened this issue Nov 2, 2016 · 4 comments
Open

Responsive menu sub-menu bug #50

khawkins98 opened this issue Nov 2, 2016 · 4 comments
Milestone

Comments

@khawkins98
Copy link
Contributor

khawkins98 commented Nov 2, 2016

(Posting on behalf of Ardan Patwardhan)

The responsive "Also in this section" will sometimes not render sub-menus correctly. The behvaiour only occurs when there is one sub-menu in the sub-menu. If there are two, everything is fine.

The bug was first noticed on http://wwwdev.ebi.ac.uk/pdbe/emdb/emdb_2017

Attempts to destroy and/or re-init the foundation dropdown menu plugin do not seem to help.

In the meantime there is a workaround by adding an invisible child menu item:

<li style="">
  <a href="#">A workaround</a> 
  <ul class="menu"> <li><a href="#">for a bug where the dropdown menu fails sometimes unless there are two submenus in the submenu</a></li></ul>
</li>

Other single-child menus seem to work fine. Unsure if this is a Foundation bug or how we're making the responsive menu, though all appears to be fine.

We could push this workaround into foundationExtendEBI.js

            var responsiveMenuSubMenuBugFix = '<li class="bug-fix-placeholder" style="display:none !important;"><a href="#">A workaround</a> <ul class="menu"> <li><a href="#">for a bug where the dropdown menu fails sometimes unless there are two submenus in the submenu</a></li></ul>  </li>';
            $(localMenuClass).append('<li class="extra-items-menu"><a href="#">Also in this section</a><ul class="menu">'+responsiveMenuSubMenuBugFix+'</ul></li>');
@khawkins98 khawkins98 added this to the 1.1 milestone Nov 2, 2016
khawkins98 added a commit that referenced this issue Nov 3, 2016
Avoids bug #50 — also makes more efficient
@khawkins98
Copy link
Contributor Author

Going to leave this one open but move Milestone v1.2. -- Hopefully by then we'll better understand what's causing the need for this workaround, or the source issue will have been resolved.

@khawkins98 khawkins98 modified the milestones: 1.2, 1.1 Nov 4, 2016
@ntung
Copy link

ntung commented Nov 24, 2016

It seems to me that this does not work on our site.

@khawkins98
Copy link
Contributor Author

As v1.2 is in RC, kicking this one to v1.3 -- which will bring a good bit of improvements to the JS in general.

@khawkins98
Copy link
Contributor Author

As this workaround seems to work well, I'm hesitant to fix it for v1.3. May push to v1.4.

@khawkins98 khawkins98 modified the milestones: 1.3, v1.4 Dec 6, 2017
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

2 participants