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

Drilldown height calculation with long titles #8320

Merged
merged 1 commit into from
Sep 26, 2016
Merged

Conversation

gakimball
Copy link
Contributor

The Drilldown plugin reads the height of each menu inside of it, and sets the height of the whole menu to match the tallest one. However, the calculated height will be wrong if any menu items differ in height, as reported in #7070.

This is because the total menu height is calculated by taking the height of the first item in the list, and multiplying it by the number of items in the menu.

This change instead just calculates the height of the entire menu all at once, so it will be accurate.

However, the call to .getBoundingClientRect() manages to be inaccurate. In the test case I put together, the height calculated is always off by 10 pixels, and I'm not sure why.

To finish this fix, we need the height calculated accurately.

…tion to check the entire height of each submenu
@@ -39,7 +39,7 @@ $drilldown-background: $white !default;
top: 0;
#{$global-left}: 100%;
z-index: -1;
height: 100%;
// height: 100%;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kball @gakimball I'm finding that this line introduces a bug when a submenu is not as tall as the view port where the submenu's parent still shows below the last item of the active submenu. Setting

min-height: 100%;

seems to do the trick.

I hesitate to put in a PR because #9239 includes much more comprehensive updates for sizing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this appears to be responsible for #9368. Looking into 9239 now.

@guyvanbael
Copy link

Not really working correctly when one of the submenus are longer.
schermafbeelding 2016-11-23 om 13 02 48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants