Migrate to AdminLTE 4 / Bootstrap 5 - #3809
Conversation
I love that you made a step in this direction. It's a good thing to modernize our code base. Reviewing will take some time, but this can be Pi-hole v7 (web) one day |
|
It wasn't going to happen any time soon without some heavy AI legwork! It's clearly not perfect - I've not looked at all the themes, and some of the elements are going to need a bit of polish |
|
There are other differences between v2.4 and the new v4, not described above. Examples:
|
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. |
|
Merged current The only real overlap was Font Awesome: both branches had independently bumped it to v7 ( While I was at it I went back over the whole UI on the merged branch to make sure nothing regressed:
Nothing functional left blocking from my side. The remaining polish - leaning the themes harder on AdminLTE 4's CSS variables, and whether |
There was a problem hiding this comment.
Marking this ready. I have merged current development in and gone back over the whole UI on the merged branch - every page in all themes, and the plugin swaps that carried the most risk (Tom Select, bootstrap5-toggle, the collapse cards and tab switching) - and I am happy with where it is.
Approving from my side. Leaving the actual merge for now so we get a chance for wider testing, and for @rdwebdesign's remaining theme/boxed points to be settled.
darkexplosiveqwx
left a comment
There was a problem hiding this comment.
Marking this ready. I have merged current
developmentin and gone back over the whole UI on the merged branch - every page in all themes, and the plugin swaps that carried the most risk (Tom Select,bootstrap5-toggle, the collapse cards and tab switching) - and I am happy with where it is.Approving from my side. Leaving the actual merge for now so we get a chance for wider testing, and for @rdwebdesign's remaining theme/
boxedpoints to be settled.
I have checked out this web branch on my local install and think that there are quite a few things needing refinement.
Would you consider this stable enough from large changes that I can do some testing?
Some inital testing feedback (will provide more context, if testing is wanted):
- login page shows 2FA, even when disabled
- if no diagnostics are available, it still shows the orange notification bubble, just without any number
Yes, that's also what I meant: ready for going into |
|
I think we could add more adjustments before merging this PR. I'll take a look at it over the next few days, but I see no reason to rush it. |
|
True, I will prepare a few polishing commits and push it to this branch. Should give a better basis for discussion. |
|
It should look nicer now. My sweep for improvements for this PR is through now and I'd be good to merge into |
The two obvious issues I raised are fixed. |
|
Fixed by 3c68d2c |
d831524 to
2e8677a
Compare
2e8677a to
4dbae30
Compare
|
Thanks for testing.
Personally, I think we shouldn't try to achieve the exact same visual. It is a new theme. It will have a different visual. Also, some of the small differences will be handled before thee migration is completed.
This is how the new theme works. Maybe we could disable the "open on hover" effect. We need to test the alternatives. The layout for datatables elements (number of entries per page, search, pagination) needs a separated PR, after this one is merged.
Actually, these buttons are working (they will work if you try to click on the minute button and then submit). The issue are the styles: the colors are not changing. |
|
Ok, how does the team continue with this PR. Merge soon and fix things piece by piece in separate PRs or fix the most obvious issues before? |
|
On the sidebar:
|
I'm pinging you in our MM conversation. |
- add hidden "password" label, for screen readers - fix markup for toggle password visibility button - remove the old .form-group class; - remove inline styles and use proper bootstrap 5 classes for margin, padding and font sizes (some inline styles will be removed later) Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
- Fix domains page layout for small screens - Adjust error pages colors; - Fix alignment (small screens) for "#domains-list" header elements Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
be00866 to
7faecbf
Compare
The new theme doesn't use background colors for striped tables and to show selected rows. Instead, it adds box-shadow to table cells to simulate background colors. The original theme uses a opaque color and white text, breaking the Pi-hle themes. The fix replaces the original box-shadow with a semi-transparent shadow and uses the theme text color. Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
7faecbf to
5dfbff2
Compare
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
I think your right example (having two lighter colors) would improve the situation. I still prefer the indention for sub-menus. I tried to allow/block a domain from the query log but the buttons don't work at all. |
|
Dashboard bar graphs:
Client/Domains/Lists
Settings/API
|
|
Thanks. The Graphics zoom can be checked later, but the group selector is broken. I tested and when I removed all groups, I couldn't re-add groups anymore. |
|
The popup when flushing the network table cannot be closed: Screencast.From.2026-07-27.23-17-43.mp4(You cannot see my mouse, but you can see the buttons reacting and it not closing) The "Yes, ..." button does in fact trigger the API action. |
- move buttons to the end of the modal-header - remove the "hidden" attribute from the QR code element - replace the old `.col-md-offset-3` class with the new `.offset-md-3` Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
e081d45 to
bc01ac0
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
|
Conflicts have been resolved. |
- put nav-tabs inside a card; - the tabs (<ul> element) should be inside card-header; - the tab-content should be inside card-body; - adjust the margins of the nav-tab and remove the rounded corners TODO: in some themes, the the selected tab background color and the tab-pane background are not matching. Also, all themes are showing different colors than we had before the migration. This is just a cosmetic issue and it will be adjusted later. Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
The previous theme used the `hidden-lg` class to hide this element on screens larger than 1200px. Now, we need to use `d-xl-none` to do the same.
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
101fc80 to
527fe63
Compare









Summary
Migrates the web interface from AdminLTE 2.4.18 / Bootstrap 3.4.1 to AdminLTE 4 / Bootstrap 5, and upgrades the other frontend dependencies that no longer had a Bootstrap-3-era version to pair with (
bootstrap-select→ Tom Select,bootstrap-toggle→bootstrap5-toggle,icheck-bootstrap→ native.form-check, DataTables/select2 to their-bs5builds, plus Font Awesome and remaining vendor bumps).49f6bd0d):.wrapper/.main-header/.main-sidebar/.content-wrapper→ AdminLTE4's.app-wrapper/.app-header/.app-sidebar/.app-main/.app-content.b0d95fcd): mechanical but reviewed conversion across all 19 affected templates, including collapse widgets and color variants.6657d28a): droppedicheck-bootstrapfor Bootstrap 5's native.form-check/.form-switch.6deb2e79): DataTables/select2 to-bs5builds,bootstrap-selectrewritten against Tom Select,bootstrap-toggleswapped for the maintainedbootstrap5-togglefork, jquery-confirm/daterangepicker/bstreeview class names updated for BS5.42646493).b049ef9a): Bootstrap 5's Tab component tracks the active state on the link itself, not the surrounding<li>.9f1d2155,afd69379,075921f4): boxed layout, badge alignment, settings/tools sidebar submenus navigating away instead of expanding, white flash between page loads on theautotheme, day theme rendering dark, abootstrap5-toggleinitialization deadlock that silently broke the Basic/Expert and All/Modified settings toggles, the All Settings tab bar's layout, and several CSPimg-src 'self'violations from Bootstrap 5 (and its plugins') inlinedata:URI icons — re-hosted as local SVGs so nothing is blocked by FTL's webserver CSP.Why
AdminLTE's maintainers are explicit that v3→v4 has no incremental upgrade path, so this necessarily touches nearly every
.lptemplate. Bootstrap 3 is long past EOL and several of the jQuery plugins we depended on (bootstrap-select,bootstrap-toggle) have no Bootstrap-5-compatible fork from their original maintainers, so those needed replacement rather than a version bump.Test plan
npm run test(prettier + xo) passes with only pre-existing warnings, no new errorsNotes for reviewers
This is a large, mechanical-but-reviewed diff by nature of the v3→v4 jump. Opening as a draft for early feedback on direction before final polish.