build(deps): bump bootstrap from 4.4.1 to 5.5.3
- Resolves #1138
Notes:
- popper.js also updated (required for updated bootstrap) (could also use bootstrap bundle and not include popper.js individually?) - edit: opted to use bootstrap bundle per discussion below
- used jsdelivr for bootstrap
- updated jquery (not sure if this is a dependency of bootstrap, but might as well update it), also moved it to jsdelivr for consistency - edit: opted to remove jquery per discussion below
Known Remaining Tasks:
- [x] general
- [x] hyperlinks are underlined now, they didn't use to be
- [x] content of pages are slightly wider now on full screen page (personally I think it looks better)
- [x] The
<hr>separator underneath the home page title used to be black, now it's grey
- [x] navbar (mostly complete)
- [x] navbar-toggler (button) has slightly larger border radius than bs4... should this be changed to match the bs4 version or use the new default... edit: using new default (let me know if you want it changed)
- [x] when I hover over a dropdown menu item ("RESOURCES"), the text colour of the word seems to lag in its transition, or perhaps it has an animation, rather than immediately changing colour like the background changes colour immediately
- [x] When a menu dropdown is open (by clicking on it), the background colour of it should be a darker grey. Right now it's the same light gray that's identical to the hover colour
- [x] When a menu dropdown is open, and the mouse moves outside, then the hover state on the menu item completely disappears
And many unknowns to work review and work through.
Comparison sites
Original bs4: https://reenignearcher.github.io/beautiful-jekyll/ Updated bs5.3: https://reenignearcher.github.io/beautiful-jekyll-bs53-test/
navbar-toggler (button) has slightly larger border radius than bs4... should this be changed to match the bs4 version or use the new default?
@daattali before I continue work on this PR, can you let me know your opinion on this?
- Regarding popper: now that I think about it, is it even needed to include it? Can popper.js just be removed? If not, then using the bundle would be better than individually loading it
- if I'm not mistaken, jquery is no longer required in BS5. If that's the case, then I would opt to remove jquery and translate the beautifuljekyll.js file to vanilla javascript. There's also some hacky code somewhere in the codebase that tries to ensure jquery isn't loaded multiple times, it'll be nice to remove that code. This may be a breaking change for some people who might rely on having jquery in their custom JS code, but the percent of people who use custom JS is very small, and I'm confident that they'll be able to look at the changelog and figure out this breaking change (this will need to be listed very clearly as a BREAKING CHANGE)
- what do you mean by "content of pages is wider"? Did bs5 change the standard widths of the grid system?
- navbar-toggler: can you post screenshots of before vs after
Regarding popper: now that I think about it, is it even needed to include it? Can popper.js just be removed?
It's needed for dropdowns in the navbar. See item 2 here: https://getbootstrap.com/docs/5.3/getting-started/introduction/#quick-start ... I'll go with the bundle method.
if I'm not mistaken, jquery is no longer required in BS5.
I'll try removing it and see if there's any repercussions. I saw the code you mention about not loading jquery twice, I'll remove that as well.
what do you mean by "content of pages is wider"? Did bs5 change the standard widths of the grid system?
There's not much difference, but it's slightly wider. I kind of like the new width better, as the old one often felt too narrow.
bs5.3:
bs4.4:
navbar-toggler: can you post screenshots of before vs after
The radius is just slightly larger.
bs5.3:
bs4.4:
I removed jquery in https://github.com/daattali/beautiful-jekyll/pull/1375/commits/ba84b36315c864e70a6d9f0e98238eb7da4555f7
Note: It's still needed by staticman-comments.
Edit: didn't realize the staticman.js was part of this repo. I will update it as well.
Edit2: jquery removed from staticman.js here: https://github.com/daattali/beautiful-jekyll/pull/1375/commits/98188bba57c36bc0ae23156a3b161d626df83db7 (I don't have a way to test this one)
Edit3: All commits merged into one now.
All sounds good. RE page width: what's the reason for the difference? Is a medium/large container simply a little wider now, or did they make the margins smaller, or ...?
All sounds good. RE page width: what's the reason for the difference? Is a medium/large container simply a little wider now, or did they make the margins smaller, or ...?
I'm not 100% sure. I assume something from here: https://getbootstrap.com/docs/5.0/migration/#grid-updates or maybe: https://getbootstrap.com/docs/5.0/migration/#rtl
If it's possible for you to have two websites up, one for the old and one for the new bs version, to make comparing easy, that would be very helpful. If that's possible
If it's possible for you to have two websites up, one for the old and one for the new bs version, to make comparing easy, that would be very helpful. If that's possible
Original bs4: https://reenignearcher.github.io/beautiful-jekyll/ Updated bs5.3: https://reenignearcher.github.io/beautiful-jekyll-bs53-test/
Thanks! Looks very good. It seems like the extra width is because of the new xxl (> 1400 px) breakpoint. That's fine.
A few differences I noticed:
- The
<hr>separator underneath the home page title used to be black, now it's grey - navbar: when I hover over a dropdown menu item ("RESOURCES"), the text colour of the word seems to lag in its transition, or perhaps it has an animation, rather than immediately changing colour like the background changes colour immediately
- When a menu dropdown is open (by clicking on it), the background colour of it should be a darker grey. Right now it's the same light gray that's identical to the hover colour
- When a menu dropdown is open, and the mouse moves outside, then the hover state on the menu item completely disappears
- You already know about the issue of links having underlines
A few differences I noticed
Thanks, I updated the PR body and will start addressing these.
The UI testing will also have to involve using all the different config parameters and the different YAML options
I've addressed these items, except:
when I hover over a dropdown menu item ("RESOURCES"), the text colour of the word seems to lag in its transition, or perhaps it has an animation, rather than immediately changing colour like the background changes colour immediately
Which I can't replicate. (Maybe it was fixed by addressing the hover issue).
Test site updated as well.
Have you tested it as thoroughly as you can, with all the different options and page types and browsers (mobile, safari, chrome, etc)? Is this ready for a final look over from me?
Have you tested it as thoroughly as you can
Not yet. Sorry, I'm a little slow with this as I'm working on several projects simultaneously.
with all the different options and page types
I've only built with the default config and pages available to it so far. If you have something specific you want tested please share it (I'm not an expert on all the options you have that would be affected by bootstrap)
Or if you have a repo that could have the updated theme applied I could fork it and cross check it that way.
Otherwise, looking through the config, my guess is these could possibly have some impact.
- title-img
- navbar-var-length
and browsers (mobile, safari, chrome, etc)?
I can test Firefox and Chrome on desktop and mobile. I can't easily test Safari (other than using the dev tools in Firefox... not sure how closely that emulates Safari)
Is this ready for a final look over from me?
Not yet, I will mark it as ready for review (and ping you) when I think it's 100% complete.
No rush, this definitely takes time (and testing can take even more time than coding!)
All the options that need to be tested are all the fields in the _config.yml file, and the YAML parameters that are described in the README.
Don't worry about safari, I can test on a friend's Mac if everything else seems fine.
@daattali sorry for the delay, but this is ready for review. I tested it as thoroughly as I can with different options. I only noticed one more difference not mentioned previously. Let me know if you want it addressed.
- The navbar title and list items are slightly closer to the edge of the page. e.g. the title is slightly further left, and the most right side object is slightly further right.
I think links used to be underlined and now they are not?
Anything else needed for this?
I added the main changes to the changelog as well.
I'm in the process of testing it.
There are two small areas that need to be fixed:
-
Underlines on links. I've noticed there are other places where there's underlines where they shouldn't be. Perhaps this needs to be tackled at the root, see if there's some sort of a CSS variable that is being used by bootstrap that changed between version 4 and 5? In any case, here are the 4 cases that I've noticed where the underlines need to be removed:
- In the navbar, the title (left side) and the menu items all get underlines on hover
- When you're on a blog post page, the buttons at the bottom "<-- Previous Post"/"Next Post -->", when you hover over them you'll notice an underline appearing between the text and the arrow
- On the
"/tagspage, when you hover over any of the tag buttons, the text in the button gets underlined - If you use the YAML
layout: minimal, all the links get underlined
-
The cover image (
cover-img) YAML parameter. It works fine when there is only one image. But there are two issues that need to be fixed when there's an array. I know you had to wrestle a lot with the removal of jQuery, it's alllllmost there. It'll be amazing when this is fixed and jquery is not needed!- To test the case of multiple images, you can use this YAML:
cover-img: - /assets/img/thumb.png - /assets/img/crepe.jpg: This is a crepe - When there is an array of images, the small caption box in the bottom right of the image should only be shown when an image has a caption. In this PR, whenever there are multiple images, the caption box is shown even when there's nothing in it
- The fading of the image isn't quite right. Only the image should get faded, not the entire div (the title/subtitle/metadata/etc should not get faded). I remember this part took me a very long time to figure out 10 years ago, hopefully today with modern JS it won't be too difficult...
- To test the case of multiple images, you can use this YAML:
@VincentTam it's been a while since I summoned you to this repo :) Would you be able to test this PR and verify that staticman still works?
In the navbar, the title (left side) and the menu items all get underlines on hover
When you're on a blog post page, the buttons at the bottom "<-- Previous Post"/"Next Post -->", when you hover over them you'll notice an underline appearing between the text and the arrow
On the
"/tagspage, when you hover over any of the tag buttons, the text in the button gets underlined
Nice catch... I believe these items are resolved.
- If you use the YAML
layout: minimal, all the links get underlined
How do you want to handle this? Your minimal css file right now is very minimal.
Edit: I pushed a little change for this. Do you have an example minimal file I can test this with?
- When there is an array of images, the small caption box in the bottom right of the image should only be shown when an image has a caption. In this PR, whenever there are multiple images, the caption box is shown even when there's nothing in it
This one is resolved. desc was null so added a check for that.
- The fading of the image isn't quite right. Only the image should get faded, not the entire div (the title/subtitle/metadata/etc should not get faded). I remember this part took me a very long time to figure out 10 years ago, hopefully today with modern JS it won't be too difficult...
I have no idea to solve this one. Maybe the built in https://getbootstrap.com/docs/5.3/components/carousel/ should be used?
Edit: I believe this one is solved as well. I added z-index and position attributes to the css.
I am now using this live in production (along with my other 2 PRs) at:
- https://app.lizardbyte.dev
- https://app.lizardbyte.dev/Sunshine
I will keep an eye out for issues.
@ReenigneArcher can you allow me permission to edit the PR? Because it says there are conflicts, but I can't see what they are
@daattali
I have that option disabled because I'm using this branch in production. But I've rebased it and fixed the conflicts in the changelog.
Let me know if I can do anything to help get this completed. Probably if it doesn't get merged soon I'm going to have to start maintaining my own fork.