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

F6.5 Drilldown Submenu height calculation wrong when toggled #11416

Closed
4 tasks done
magic-77 opened this issue Jul 21, 2018 · 1 comment · Fixed by #11425
Closed
4 tasks done

F6.5 Drilldown Submenu height calculation wrong when toggled #11416

magic-77 opened this issue Jul 21, 2018 · 1 comment · Fixed by #11425

Comments

@magic-77
Copy link

magic-77 commented Jul 21, 2018

I'm using the Drilldown Menu in combination with Responsive Navigation and Top Bar.

What should happen?

When Drilldown is closed and reopened after navigating to a submenu, height for ".is-drilldown"
should be height of ".is-drilldown-submenu.is-active"

What happens instead?

seems that height for ".is-drilldown" is recalculated from "ul.drilldown".
This results in overlapping active Submenu over Main Menu, if Main Menu is heighter
than Submenu

Possible Solution

Test Case and/or Steps to Reproduce

Test Case:
https://codepen.io/magic77/pen/vagxYL

How to reproduce:

  1. Scale your browser down to see the toggle Button
  2. Open Mobile Menu, and click "Produkte OPEN ME"
  3. Click "Produkte 2 OPEN ME"
  4. Close and reopen Mobile Menu

Context

Your Environment

  • Foundation version(s) used: 6.5.0-rc.2
  • Browser(s) name and version(s): Chrome 67.0.3396.99
  • Operating System and version (desktop or mobile): Mint Linux 18.3, Desktop

Checklist (all required):

  • I have read and follow the CONTRIBUTING.md document.
  • There are no other issues similar to this one.
  • The issue title is descriptive.
  • The template is correctly filled.
@magic-77 magic-77 changed the title F6.5 Drilldown height Submenu calculation wrong when toggled F6.5 Drilldown Submenu height calculation wrong when toggled Jul 21, 2018
@ncoden ncoden added the 🐛bug label Jul 30, 2018
@ncoden ncoden self-assigned this Jul 30, 2018
ncoden added a commit to ncoden/foundation-sites that referenced this issue Jul 30, 2018
…oundation#11416

Changes:
- Save the currently opened sub-menu as `$currentMenu`
- When calculating the Drilldown wrapper height, use the currently opened menu height instead of the primary menu.

Closes foundation#11416
ncoden added a commit to ncoden/foundation-sites that referenced this issue Jul 30, 2018
…oundation#11416

Changes:
- Save the currently opened sub-menu as `$currentMenu`
- When calculating the Drilldown wrapper height, use the currently opened menu height instead of the primary menu.

Closes foundation#11416
@ncoden ncoden added the PR open label Jul 30, 2018
@ncoden
Copy link
Contributor

ncoden commented Jul 30, 2018

Hi @magic-77. Thank you for the report and the test case! I'm working on a fix.
See #11425

ncoden added a commit to ncoden/foundation-sites that referenced this issue Jul 31, 2018
ncoden added a commit that referenced this issue Aug 4, 2018
fix: open Drilldrown with the currently opened (sub)menu height #11416
ncoden added a commit to ncoden/foundation-sites that referenced this issue Aug 25, 2018
…eight-11416 for v6.5.0

c7ce8d5 fix: set the Drilldown height for on the currently opened (sub)menu foundation#11416
43c1fac test: add unit tests for Drilldown resizing on toggling
7f7dfd8 test: check the Drilldrown wrapper height instead of the element height
90c4e45 fix: set a default for the current menu in Drilldown before it is used
1db6a1f test: add unit test for the Drilldown height when reopened from a submenu foundation#11416
d4617ac clean: remove infinite timeout used for development in Drilldown tests

Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
@ncoden ncoden mentioned this issue Sep 10, 2018
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants