-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Upgrade adminLTE to v3 #7740
Upgrade adminLTE to v3 #7740
Conversation
It requires to target 5.x, isn't it ? |
For the moment there are some PR on the frontend that didn't land in 5.x yet (I think). Also I would like to understand what changes are needed to be compatible with AdminLTE 3. This way we can decide if it is worth to do it on 4.x or 5.x. Of coure it will be done probably on 5.x since it will require a lot of changes. Still I don't know yet how many things needs to be changed. |
92277ed
to
9ec6e98
Compare
Dropping bootstrap3 in favor of bootstrap4 is a BC-break to me since it can break the html/css a user added on a custom templates. Also, we're dropping xeditable and the related feature. So it will be 5.x. But I'm curious to see how hard it is to have this update. |
For now I did the changes needed to be able to compile assets, my next step would be to install this version on a custom project to see how it renders As you said it will be landing on 5.x unless something magic happens like something like this working great: https://www.npmjs.com/package/bootstrap-4-migrate |
Hey @jordisala1991 I tried to use your branch to see how the new admin looks like, but I saw many things are not working questions
Edit 1: or do you think is easier just one person work on that |
@jordisala1991 I'm fixing the sidebar menu on the left side old one: what is missing
when I fix that I can open a MR for your branch |
I need to override this submenu's I need to add |
You should look at |
@@ -53,11 +53,6 @@ Encore.setOutputPath('./src/Resources/public') | |||
|
|||
.copyFiles([ | |||
{ from: './assets/images/', pattern: /\.(png|gif)$/, to: 'images/[name].[ext]' }, | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's important to skins?
because I'm getting this error
GET http://localhost:8001/bundles/sonataadmin/admin-lte-skins/skin-black.min.css net::ERR_ABORTED 404 (Not Found)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't find the skins on the admin-lte v3, maybe we need to remove that feature too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this
When updating to admin lte, maybe you can look for some custom html attributes like I tried this few years ago: https://github.com/sonata-project/SonataAdminBundle/pull/3312/files This would close #5132 |
Could you please rebase your PR and fix merge conflicts? |
Nice 👏👏 |
Fixed sidebar, now it opens and closes. Also minor fixes on styles for the sidebar and the header bar. There is still a lot of work to do. |
Could you please rebase your PR and fix merge conflicts? |
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
👀 |
Hi @jordisala1991, can you give a short update what parts were missing / done? I'd like to continue on this topic in the next weeks. |
This pr was missing a lot of things, but it is really old, I cant remember exactly. My last attempt was with bootstrap directly, last released version contains everything that we might need. Adminlte is no longer needed imho. |
Subject
I am targeting this branch, because this breaks BC.
Changelog