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

[BUG] Pushmenu plugin not working nicely when using data-auto-collapse-size #3561

Open
dfsmania opened this issue Apr 3, 2021 · 12 comments
Open

Comments

@dfsmania
Copy link

dfsmania commented Apr 3, 2021

Describe the bug
The pushmenu plugin is not working nicely if you setup the data-auto-collapse-size to a value less than 992px (or LG breakpoint). The problem is related to the style that is applied to the .content-wrapper, main-header, main-footer, and other elements at the @media (max-width: 991.98px). For example:

@media (max-width: 991.98px) {
  body:not(.sidebar-mini-md):not(.sidebar-mini-xs):not(.layout-top-nav) .content-wrapper,
  body:not(.sidebar-mini-md):not(.sidebar-mini-xs):not(.layout-top-nav) .main-footer,
  body:not(.sidebar-mini-md):not(.sidebar-mini-xs):not(.layout-top-nav) .main-header {
    margin-left: 0;
  }
}

As noted, a margin-left property is applied to the elements but the sidebar will remain open, because we have changed the default data-auto-collapse-size of the pushmenu plugin to a lower value:

  /**
   * --------------------------------------------
   * AdminLTE PushMenu.js
   * License MIT
   * --------------------------------------------
   */
  /**
   * Constants
   * ====================================================
   */

  var NAME$5 = 'PushMenu';
  var DATA_KEY$5 = 'lte.pushmenu';
  var EVENT_KEY$2 = "." + DATA_KEY$5;
  var JQUERY_NO_CONFLICT$5 = $__default['default'].fn[NAME$5];
  var EVENT_COLLAPSED$1 = "collapsed" + EVENT_KEY$2;
  var EVENT_SHOWN = "shown" + EVENT_KEY$2;
  var SELECTOR_TOGGLE_BUTTON$1 = '[data-widget="pushmenu"]';
  var SELECTOR_BODY = 'body';
  var SELECTOR_OVERLAY = '#sidebar-overlay';
  var SELECTOR_WRAPPER = '.wrapper';
  var CLASS_NAME_COLLAPSED = 'sidebar-collapse';
  var CLASS_NAME_OPEN$3 = 'sidebar-open';
  var CLASS_NAME_IS_OPENING$1 = 'sidebar-is-opening';
  var CLASS_NAME_CLOSED = 'sidebar-closed';
  var Default$5 = {
    autoCollapseSize: 992,   <====== HERE!!!!!
    enableRemember: false,
    noTransitionAfterReload: true
  };

This will result on a visual issue, as shown next, where you have no way to collapse the sidebar and you can't see the entire .content-wrapper content when the screen width is lower than 992px and above the configured value that will trigger the auto collapse:

Screenshot_2021-04-03 Laradmin Home

More information about this issue can be found on:

jeroennoten/Laravel-AdminLTE#822

To Reproduce
Steps to reproduce the behavior:

  1. Enable data-auto-collapse-size as per documented here: https://adminlte.io/docs/3.1//javascript/push-menu.html with a value lower than 992 (for example 600).
  2. Play with the browser's width to reach it under 992px and above the configured value.
  3. Check the error explained before.

Expected behavior
The .content-wrapper and other element should get the special style related to sidebar collapse mode when you reach the configured value, not on the hardcoded 992px.

Environment (please complete the following information):

  • AdminLTE Version: v3.1.0
  • Operating System: Not relevant
  • Browser (Version): Not relevant

I'm not really sure if this will be easy to be solved, because it involves a mixture between Javascript and CSS compatibility. However, I wanted to describe the problem so you all know about it.

@REJack
Copy link
Collaborator

REJack commented Apr 6, 2021

You should add .sidebar-mini-md for sizes lower than 992px. 😄

@dfsmania
Copy link
Author

dfsmania commented Apr 7, 2021

Thanks @REJack I will try that tomorrow!

@dfsmania
Copy link
Author

dfsmania commented Apr 7, 2021

I tried to use .sidebar-mini-md but do not works properly, because now the next style is applied to the elements, and the toggle button remains hidden as before:

.sidebar-mini-md .content-wrapper, .sidebar-mini-md .main-footer, .sidebar-mini-md .main-header {
    margin-left: 4.6rem;
}

Screenshot_2021-04-07 Laradmin Home

I believe, the current css layout is not compatible with the mentioned pushmenu plugin configuration, i.e, the css layout applies left margin property to the main elements at specific sizes (hardcoded ones), but the pushmenu plugin is in charge to collapse/expand the sidebar on a size value that can be configured by the user and will be very different from the hardcoded ones.

@danny007in
Copy link
Collaborator

danny007in commented Apr 8, 2021

u need to use any single class sidebar-mini or sidebar-mini-md

@dfsmania
Copy link
Author

dfsmania commented Apr 8, 2021

@danny007in the body has the next classes on my previous comment:

class="layout-fixed layout-navbar-fixed sidebar-mini-md"

Again, I really believe the problem is that the CSS of the layout use fixed breakpoints to setup .content-wrapper, .main-header and .main-footer left margin. But sidebar won't collapse at those fixed points, since we have configured it to a custom value (600px for example). Maybe you should think to limit data-auto-collapse-size property of the pushmenu plugin to fixed values, like xs, sm, md, lg and try to make it compatible with the current layout CSS. However, I'm not familiarized with this package to say that will be a nice solution.

@REJack
Copy link
Collaborator

REJack commented Jul 29, 2021

@Shidersz I will change the data-auto-collapse-size's with v4, otherwise it can cause breaks by users.

@REJack
Copy link
Collaborator

REJack commented Jul 29, 2021

But I will check if I can fix it for now 😄

@sergiy-petrov
Copy link

@REJack is it the only issue that blocks you from releasing 3.2 version?

@REJack
Copy link
Collaborator

REJack commented Jan 9, 2022

Actually not, I've waiting for #4123. If it's not fixed till wendsday I will ignore it since I can't modify it by my self.

I'v not found a way to fix it without breaking it for others.

@sergiy-petrov
Copy link

@REJack so seems #4123 is merged now

just curious - is it possible now to release 3.2.0 version or there are any other blockers?

@REJack REJack added this to the v4.0.0 milestone Feb 7, 2022
@danny007in
Copy link
Collaborator

is this issue still exist ?

@dfsmania
Copy link
Author

dfsmania commented Feb 9, 2023

@danny007in This issue is still present on AdminLTE v3.2.0, if you follow the timeline of the discussion there is no plan to fix this on versions v3.x.x because it will introduce breaking changes. On the other hand, @REJack proposed to re-design the data-auto-collapse-size option of the pushmenu plugin for v.4, but I'm not sure what has been done at this moment...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To do
Development

No branches or pull requests

4 participants