-
Notifications
You must be signed in to change notification settings - Fork 175
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
Update bootstrap 4.6.0 to 5.3.0 #340
Update bootstrap 4.6.0 to 5.3.0 #340
Conversation
Moving this out of 1.5.8, it should be bundled with the update of django: #360 |
This is a bit of a pain. I thought it would be fine to land in 1.6.0 but it requires more work than expected so I am leaving it out of scope for now, we can do it in a later dot release to avoid delaying 1.6.0 further. |
Searched a bit and there's some upgrade resources here: |
Bootstrap is up to 5.3 now: https://blog.getbootstrap.com/2023/05/30/bootstrap-5-3-0/ I may take this opportunity to revisit and clean up the templates but this is on my radar. |
Some notes about upgrading bootstrap from 4 to 5:
|
I spent some time on the accordion for the playbook search. Lots of cleanup and I put it into it's own partial. Here's a quick video showing the comparison between the current implementation and the work in progress: Peek.2023-07-01.01-12.mp4 |
Spent (probably too much) time theming the accordion, what it looks like in light and dark: Peek.2023-07-01.15-37.mp4Peek.2023-07-01.15-38.mp4 |
4a3b703
to
5dd57f4
Compare
5dd57f4
to
3ab8bbd
Compare
Build failed. ✔️ ara-tox-py3 SUCCESS in 3m 18s |
Build failed. ✔️ ara-tox-py3 SUCCESS in 3m 47s |
the GIFs looking really promising. Thanks for this hard work! |
Much appreciated, thank you @hille721 ! I am expecting to be able to make some more progress this week. In any case, once I feel it is ready, I will put it up on something like dev.demo.recordsansible.org so it is easy to compare side-by-side with the live demo running the same data. |
I've fixed and updated the task result detail page: I also fixed the pygments css theming for viewing files but I haven't yet figured out the right way to re-write the line highlighting that was done with jquery (that we no longer have): ara/ara/ui/templates/file.html Lines 22 to 34 in 578ef81
|
I think I've got something working for the file line highlighting. I dislike javascript and better ideas are welcomed but this appears to do the job without jquery: <!-- Line highlighting -->
<script>
document.addEventListener("DOMContentLoaded", function(event) {
if (window.location.hash.length) {
// Recover the line number in the URL anchor and highlight it
var anchor = window.location.hash;
var span = document.querySelector(anchor);
span.classList.add("hll");
}
document.addEventListener("click", function(e) {
// When a link is clicked
if(e.target.tagName == "A") {
// Remove the previously highlighted line if there is one
if (window.location.hash.length) {
var anchor = window.location.hash;
var span = document.querySelector(anchor);
span.classList.remove("hll")
}
// Highlight the new one
var anchor = e.target.getAttribute("href");
var span = document.querySelector(anchor);
span.classList.add("hll");
}
});
});
</script> |
Build succeeded. ✔️ ara-tox-py3 SUCCESS in 3m 29s |
Build succeeded. ✔️ ara-tox-py3 SUCCESS in 3m 18s |
@hille721 I believe to have fixed the two mobile responsiveness oddities you've picked up on, thanks ! |
I've set up a temporary demo which is set up from this branch: https://dev.demo.recordsansible.org It is using the exact same data as https://demo.recordsansible.org in order to make it easy to compare between the two. |
By the way: I don't know where this is going yet but I am attempting trying to clean up and simplify bits of django templating. It's been pretty experimental and I'm not really happy with how it's looking right now so there is more work to do. For example, there's new partial templates (meant to be included) under the It's out of scope of the bootstrap update and the UI refresh but they both touch pretty much every template so it's an opportunity to work through that at the same time. The templating has grown organically over the years. We didn't necessarily make re-usable/standardized components that could be logically re-grouped and easily included in larger blocks of templating to simplify things where it makes sense. With the power of experience and hindsight, we can do better :p |
Hi @ianw, thanks for looking !
I've noticed this behavior in the previous version of bootstrap as well but I haven't researched it. Just to confirm, can you still scroll horizontally even though there's no horizontal scroll bar ? I don't know what's the right fix but it's not the best UX for sure. |
@ianw btw I also tried to make sure that the static generation still works properly, here's one from this PR from Zuul: https://b860d1e0b452f4998247-bef371a08164594183d849ff8c4e5a1f.ssl.cf1.rackcdn.com/340/7e2734008a4327dae7b3131c4a45f3a147f91c24/check/ara-basic-ansible-core-2.15/3f70c2e/server/static/index.html Let me know if anything stands out to you :) |
Yes, if I click to select the page, I can use left/right to move but the page still doesn't show any sort of horizontal scroll-bar to indicate the length. |
Thanks for considering this I would say there seems to be a lot of whitespace, and the columns perhaps showing too much of what I don't want to know, causing hiding of what I do. Perhaps the labels could be folded into a row, and the task name could be on a row so you can see the whole thing too. I definitely like the full dates (rather than "X hours ago") because often you need to line up timestamps with system logs, etc -- but the format for that and the duration may be a bit verbose and could drop zeros maybe? |
@ianw I've created an issue to capture this: #498 I'm leaving it out of scope of this particular PR, especially since the previous version of bootstrap has the same behavior. |
Build succeeded. ✔️ ara-tox-py3 SUCCESS in 3m 13s |
I agree that labels are problematic when there's a couple of them or they are too long. I've known about this but don't believe it was recorded in an issue so I've gone ahead and created one for it: #499 For dates it's tricky. The verbose format relieves the confusion from american date formatting versus the rest of the world and I think that's a good thing -- in other words, is this december 7th or july 12th: 12/7/2023 ? Perhaps the timezone could be put in a tooltip which would free up 6 characters worth of width. For durations we could do something as well. I'll see if I can think of something. Thanks for the feedback @ianw :) |
My last commit to the PR fixes the tags tooltip and includes a minor fix for the ara_record page. With this, I think I am done with more or less a first full pass throughout the entire UI. |
Build failed. ✔️ ara-tox-py3 SUCCESS in 3m 15s |
6663747
to
265ac04
Compare
I fixed the file responsiveness issue I commented on 2 days ago: #340 (comment) To be clear, this isn't a bootstrap regression. It was also an issue before but hey, it's fixed now: ara-file.mp4 |
265ac04
to
abc56a7
Compare
Build succeeded. ✔️ ara-tox-py3 SUCCESS in 3m 26s |
For posterity I've recorded a small video of the mobile UI: ara-mobile-resized-v2.mp4 |
I ran djlint which is a linter for django template things, maybe we can add it to the linter CI job but in the meantime:
|
I've finished a last round of review, cleanup and fixing. I think this is good to go but I need to fix the commit history a bit. Forposterity, all the work split into milestones were pushed to another branch: https://github.com/dmsimard/ara/tree/update-static-assets-archive |
Build succeeded. ✔️ ara-tox-py3 SUCCESS in 3m 32s |
This is a squashed commit that encompasses all the work that has gone into updating to bootstrap 5 and django templating cleanup. About boostrap and CSS: - Update bootstrap CSS from 4.6.0 to 5.3.0 and fix broken layout and components as a result of the update - Removed separate light/dark themes via bootstrap-darkly and bootstrap-flatly: bootstrap 5.3 features a new built-in dark theme - Re-worked the dark/light theme selection to match the new bootstrap built-in dark theme including pygments highlighting for pretty-printed output - Removed jquery, it is no longer required with bootstrap - Re-worked implementation of file line highlighting since it relied on jquery - Fixed tooltip implementation (i.e, for task tags) since the implementation in bootstrap had changed About site-wide minor cleanups and improvements: - Font size made generally larger and more consistent - Improved the about and CLI argument modals - Improved display for the report and CLI argument buttons - Improved the playbook report header card - Adjusted search accordions to match new bootstrap theme - Improved size and consistency of many card headers - Improvements to responsiveness of layout at smaller (e.g, mobile) resolutions About django templating: - Large chunks of templating were moved out to partials/tables and partials/search in order to improve readability. - Round of template cleanups and fixes as reported by djlint
b9e8397
to
52ecd45
Compare
Build succeeded. ✔️ ara-tox-py3 SUCCESS in 3m 42s |
This is good enough and we can iterate on this if there are issues. This PR has been in progress since Nov 20, 2021 and it's finally merging ! |
No description provided.