ColorlibHQ / AdminLTE

AdminLTE - Free admin dashboard template based on Bootstrap 5
https://adminlte.io
MIT License
44.08k stars 18.17k forks source link

[BUG] Sidebar Menu pops out when screen size changes #4557

Open burdittw opened 2 years ago

burdittw commented 2 years ago

When you click either button to hide or shrink the sidebar menu and then change the size of the browser, the sidebar will go back to being open instead of keeping its current state.

How to test: 1) Click the button to hide the menu. 2) Click and hold the side of the browser window and change the size of the window. 3) Sidebar menu will pop open.

Maybe the state needs to be saved in local storage or cookie.

darkknight20032001 commented 2 years ago

@burdittw can u give the html link where the changes need to be done??? please???

burdittw commented 2 years ago

I don't know as I have not dug into it. I was working with a grid in my work project, and I was checking the responsiveness of the grid when I noticed the sidebar menu popping out. I am Knee deep with my work project so I don't know if I will have time to look at any code in AdminLTE for a while. I have to have a working demo of a huge project by the end of the month.

I am not sure where the sidebar state is being held, maybe it is not and that is the problem. I was just thinking out loud that the state of the element needs to be save somewhere so that on a window resize does not drop the sidebar's state of open, closed, or shrunk. I wish I had time to look into it, but I don't right now.

darkknight20032001 commented 2 years ago

ok no problem, i am seeing it

burdittw commented 2 years ago

I have figured out why this is happening. In the file push-menu.ts there is code that checks on resize if this tagged as LAYOUT_MOBILE or not. This fires on window resize and that is causing the issue. Here is the section of that file that is causing the issue:

  addSidebaBreakPoint(): void {
    const bodyClass = document.body.classList
    const widthOutput = window.innerWidth

    if (widthOutput < Defaults.onLayouMobile) {
      bodyClass.add(CLASS_NAME_LAYOUT_MOBILE)
    }

    if (widthOutput >= Defaults.onLayouMobile) {  <== THIS IS WHAT IS BREAKING IT
      bodyClass.remove(CLASS_NAME_LAYOUT_MOBILE)
      this.expand()
    }
  }

I did a test and changed that if statement to:

if ((widthOutput >= Defaults.onLayouMobile) && (bodyClass.contains(CLASS_NAME_LAYOUT_MOBILE))) {

This gets it more into line with what it should do, but this is nowhere close to the solution. This should give someone a starting point to look at this section of code to get it working correctly. The mini-menu has the same issue as when you expand the screen past the 992px it will flip to the full menu, and it will be out in full view.

Anyways this is a hack fix if you need it now, but it needs some thought and work to figure out all the classes being added to the and what it should be looking for more than just the width of the screen.

darkknight20032001 commented 2 years ago

ok @burdittw .... thnx

burdittw commented 2 years ago

I have started a major rework of this project and am almost finished. This work will include fixes/enhancements as follows:

  1. Sidebar will now remember the setting when you resize it.
  2. Sidebar can now be adjusted to the left or right side using the data attribute "data-sidebar-position".
  3. I removed the use of the meta tag "color-scheme" as this is used by the browsers to determine the color scheme of your desktop theme or if you set your browser to a theme.
  4. New data attributes have been added or changed to better handle things in the CSS and JavaScript. The new ones are "data-theme", "data-layout", and the "data-sidebar-position" They should be self-explanatory.
  5. The "adminlte.scss" file now has the imports and variables setup the way the bootstrap documentation states to do it. There is an open item for this work.
  6. So, so much scss and ts has changed to accommodate these changes...I am writing this trying to remember all of what I changed of the top of my head that I am not sure if I have captured all that I have done.

I hope to push this all up to my fork sometime this week. Not sure if it will be welcomed or not into the actual project, but it is what I have done to make this project workable and flexable for others to use as well.

So be on the lookout for it this week sometime!

burdittw commented 2 years ago

Today I finished the layout-fixed.scss and the push-menu.ts. Tomorrow I will work on cleaning up the layout-mobile.scss to make sure it is displaying the layout correctly with the sidebar closed and sidebar mini mode in both left and right views. I also added a CTA button on the sidebar. Not really much to it other than to demonstrate how to add one and the css to make it work. Once I have this done then I think I just need to double check all the code and remove completely anything I no longer need, as to what I have changed in what I have built.

lenamtl commented 2 years ago

@burdittw let me know if you publish a fork

burdittw commented 2 years ago

@lenamtl I have pushed my code and a dist folder so people can see it in action to my fork of the AdminLTE.

Fork burdittw/AdminLTE/v4-dev-2725-scss has been posted with many changes and fixes.

lenamtl commented 2 years ago

@burdittw in your fork does the dist reflect the src. I do no use any builder so I just want to know if the files in dist have all changes.

Thanks a lot.

burdittw commented 2 years ago

@lenamtl

in your fork does the dist reflect the src. I do no use any builder so I just want to know if the files in dist have all changes.

Yes it does have all the changes...but I ran into a problem with it yesterday and that is the light and dark mode are not working right. I am getting ready to dig into and see if I can figure it out and get it fixed. Other than some odd coloring's it does everything I put in the notes.

burdittw commented 2 years ago

@lenamtl new update has posted tonight...migrated over to Bootstrap 5.3 to fix the theming and added a toggle switch so you can test it out. Grab the newest dist for the latest build.

lenamtl commented 2 years ago

Hi @burdittw, I made some tests and add bug and comment to your Fork repo

burdittw commented 2 years ago

@lenamtl

Thanks for the feedback. When I get some time, maybe this weekend I will see if I can take a look into these. I also noticed the "User Profile" drop down it not setup to use the theme as well. Not a big deal for me right now as I have to try and get a semi working demo of my work project up by the end of the month.

lesterdev commented 2 years ago

Will this bug be fixed in adminlte v3?

REJack commented 1 year ago

Will this bug be fixed in adminlte v3?

That's a AdminLTE v4 issue, not for v3.x and there it's not a bug it's a feature (for real) checkout my latest comment in #4787.