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

resize event not triggered after expand treemenu #1547

Closed
Th3M3 opened this issue Jun 21, 2017 · 14 comments
Closed

resize event not triggered after expand treemenu #1547

Th3M3 opened this issue Jun 21, 2017 · 14 comments

Comments

@Th3M3
Copy link

Th3M3 commented Jun 21, 2017

I try to update v2.1.2 to actual v2.4.
But when I update old app.min.js to new adminlte.min.js, I get the following issue:

If I use a content page that is smaller then the browser' height and expand one of the treeview-menus on the left sidebar, the footer should move down to always stay on page's bottom.
This was workig in the older versions, but does not in v2.4

I also tried to reproduce it with the demo page:
With the browser's developert tools, I removed some of the content, did a browser resize and swithed back to fullscreen, and than expanded one of the treemenu.

This is what I get before expand e.g. the Charts-Menu (everything is as axpected):
adminlte1

And after expand Charts:
adminlte2
The Footer is not aligned to bottom and there are two scrollbars on the right.
It goes to normal if I collapse the menu or do a screen resize - but the resize event sould get triggered automatically.

@almasaeed2010
Copy link
Contributor

@Th3M3 thanks for the raising the issue!

From my tests so far, this seems to be caused by the change in header height during the transition between the stacked header and the normal header. Trying to figure out a solution right now. Let me know if you find something that could be helpful.

Thanks!

almasaeed2010 added a commit that referenced this issue Jun 25, 2017
@almasaeed2010
Copy link
Contributor

@Th3M3 we just submitted a fix on the master. Would you care to test it?

Thanks!

@Th3M3
Copy link
Author

Th3M3 commented Jun 25, 2017

I have updated files adminlte.min.js, AdminLTE.min.css, skin-blue.min.css on my pi-hole installation.
For first look, the issue seemd to be solved.
But after further tests, the footer still sometimes not aligned to the bottom.

Here a screenshot (using Opera on Windows) with Developer Tools open and <body> element selected:
devel1

At another test in Microsoft Edge, the gap between footer and bottom is even more.

After zoom the page to 110%, the pages work correctly on Microsoft Edge.
But on Opera, I again get two scollbars on the right side and the footer is again not aligned to site's bottom. (I'm not sure if this is a opera browser bug).
What I found out is that the <div class="content-wrapper"> gets style min-height: 464px.
But after I expand and collapse one of the left tree-menus, the content-wrapper gets min-height: 463px and then there is only one scrollbar on the right.

devel2

The next days I will try to change the js to log the heights variables into Developer Tool's console, to find out what is going on there in detail....

@almasaeed2010
Copy link
Contributor

So the issue only occurs on Opera?

@Th3M3
Copy link
Author

Th3M3 commented Jun 25, 2017

Edit: Maybe jQuery needs to be updated too to get the footers .outerHeight() correctly.

I will try this out and will report here.

@almasaeed2010
Copy link
Contributor

I tested on Opera, Safari, FF and Chrome but I could not reproduce on the demo. If you test your browser using the Demo it would be great!

@Th3M3
Copy link
Author

Th3M3 commented Jun 25, 2017

So the issue only occurs on Opera?

At 100% zoom, the gap is still there like you see on my screenshot, on Opera and Microsoft Edge. Haven't tested other browsers.

@Th3M3
Copy link
Author

Th3M3 commented Jun 26, 2017

I tested on Opera, Safari, FF and Chrome but I could not reproduce on the demo. If you test your browser using the Demo it would be great!

Are you sure to have updated the files in the demo?
I have still the same issues there, using Opera and Microsoft Edge

@almasaeed2010
Copy link
Contributor

Sorry I meant testing by downloading the master branch.

@Th3M3
Copy link
Author

Th3M3 commented Jun 27, 2017

Now I had time to test it again.

But it still not work as expected:
If I collapse expand a menu wich has only a few items, the footer don't moves down, but a scollbar appears on the right an I can scroll the page to this:

test1

But if I collapse expand a menu-tree with at least six items, the footer move down to page's bottom - as it should be.

Same behavior on Opera and Microsoft Edge.

Edit: Sorry - mistaken collapse and expand

@Th3M3
Copy link
Author

Th3M3 commented Jun 28, 2017

The following things I found out:

  • jquery 3.2.1 don't work correctly to me. It gets wrong height and outerHeight values, if browser's zoom factor is not set to 100%.
    With jQuery 3.1.1 the values are correctly.

  • in adminlte(.min).js I found an issue:

    • The sidebar's height should be the full element's height (including margin and padding)
    • While comparing sidebarHeight with windowHeight, it's needed to also add the header's height to get the overall height, because the menu-sidebar is placed under the header.
    • I also would add a function to round up the neg variable. In my case, in Opera I always had a scrollbar to scroll only 1px, because I need to use a page zoom factor.
    • added setTimeout function before read the controlSidebar's height, because without it sometimes returned a insufficient value.

Here my suggestion to the code. My changed lines are marked with a red > on the left.

    // Get window height and the wrapper height
    var footerHeight  = $(Selector.mainFooter).outerHeight() || 0
    var neg           = $(Selector.mainHeader).outerHeight() + footerHeight
    var windowHeight  = $(window).height()
>   var sidebarHeight = $(Selector.sidebar).outerHeight() || 0
>   var headerHeight = $(Selector.mainHeader).outerHeight() || 0

    // Set the min-height of the content and sidebar based on
    // the height of the document.
    if ($('body').hasClass(ClassName.fixed)) {
      $(Selector.contentWrapper).css('min-height', windowHeight - footerHeight)
    } else {
      var postSetHeight

>     if (windowHeight >= sidebarHeight + headerHeight) {
>        $(Selector.contentWrapper).css('min-height', windowHeight - Math.ceil(neg))
>        postSetHeight = windowHeight - Math.ceil(neg)
      } else {
>        $(Selector.contentWrapper).css('min-height', sidebarHeight - footerHeight)
>        postSetHeight = sidebarHeight - footerHeight
      }

      // Fix for the control sidebar height
      var $controlSidebar = $(Selector.controlSidebar)
>     setTimeout(function(){
         if (typeof $controlSidebar !== 'undefined') {
           if ($controlSidebar.height() > postSetHeight)
             $(Selector.contentWrapper).css('min-height', $controlSidebar.height())
         }
>     }, 0);
    }
  }

Please review/test out my code. For me it works fine.

@jaydenseah
Copy link

jaydenseah commented Jul 6, 2017

Hi Th3M3, your solution on editing the codes in adminlte.js works properly. You should try contributing this changes. By the way, I'm still using the default Jquery provided in the bower_components. Thanks!

@Th3M3
Copy link
Author

Th3M3 commented Jul 6, 2017

@jaydenseah Thank you for your testing and feedback.
Glad to hear that it works for you as expected.

I have created a pull request for my changes.

Regarding my jquery issue, I did not notice that there is a bower_components folder in the branch.
After I downloaded the master branch, I had no such folder. Seems that I did something wrong.
I will try it again when i have more free time.

@jaydenseah
Copy link

@Th3M3 you're most welcome.

Regarding the bower_components part, try downloading the master branch, and you should be able to see it. Cheers!

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

4 participants