Skip to content
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 bootstrap and jquery versions #3295

Closed
wants to merge 2 commits into from

Conversation

bkmgit
Copy link
Contributor

@bkmgit bkmgit commented Nov 27, 2023

No description provided.

@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 27, 2023

@lminiero

  • jsdelivr is used in virtualbg.html so assume it is ok as a source for upgraded versions
  • Have not updated docs yet, need to examine a little more
  • Colors have changed somewhat in bootswatch/5.4.2/cerulean/bootstrap.min.css from bootswatch/3.4.0/cerulean/bootstrap.min.css is it ok to have such changes?

@bkmgit bkmgit changed the title WIP: Upgrade bootstrap and jquery versions Upgrade bootstrap and jquery versions Nov 27, 2023
@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 27, 2023

Separate pull request for documentation #3296

@lminiero
Copy link
Member

Thanks! This looks like a huge refactoring, so I'll need time to review it.

From a quick glance, the change of bootstrap and font-awesome versions already involved many changes in the HTML code, but there's elements we sometimes create dynamically in the JavaScript sources, which assume the older versions are in use: that's the case, for instance, any time we create a new item with an icon somewhere, or when we create new divs dynamically. I expect some weirdness to happen when testing the demos more extensively. I'll try to do that in the next few days and let you know if I spot anything specific that needs fixing (here and in the docs PR).

@lminiero
Copy link
Member

  • Colors have changed somewhat in bootswatch/5.4.2/cerulean/bootstrap.min.css from bootswatch/3.4.0/cerulean/bootstrap.min.css is it ok to have such changes?

Depends if the color scheme is completely different. If it is, we may have to switch to a different bootswatch template, at least or the sake of consistency. I'll have a look at that too.

@lminiero
Copy link
Member

Mh it doesn't look like you changed anything related to boostrap, updating to v5... all pages look very bad, because none of the bootstrap styling is applying anywhere. This looks exactly like when I first started to experiment with boostrap v5.x myself. A proper update of version requires changing how the different types of divs are created: panels, menus, navbars, etc. The new version has a different syntax than v3, which is why all elements look like they have no styling at all.

As it is, this isn't an effort I could merge, sorry.

@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 27, 2023

Updated main navbar, but have not changed other divs such as menus and panels. Can do so, but try to keep a similar look.

For a deployment see
https://janus.kichakatokizito.solutions

screen shot of updated version in this pull request:
NewBootstrap

screen shot of current version at https://janus.conf.meetecho.com/ :
CurrentBootstrap

@lminiero
Copy link
Member

screen shot of updated version in this pull request:

Yeah, and that already shows some styling not being applied (e.g., container and such). If you open the EchoTest demo, for instance, all elements are basically on top of each other (as they are in your screenshot too), panels aren't panels, and there's elements that should be hidden and aren't (the video box on the bottom). The navbar itself is actually broken, if you double check, since if you watch it on mobile there's no button that collapses the menu (which suggests it may be more a problem with menus than the navbar, but still it's an issue).

Unfortunately, as I explained, upgrading to bootstrap 5 is a huge endeavour that I tried already, and eventually gave up on for lack of time and because it didn't look like I was making (enough) progress. You're free to try as well (I may try again too, sooner or later, maybe going through bootstrap 4 first to see if migration guides help), but I have to anticipate that if the look and feel won't be the same as the demos we have now (at least functionally) we'll just stick with the devil we know and decline the PR.

@lminiero
Copy link
Member

FYI, I've started attempting an upgrade to bootstrap 4, and it that "succeeds", I'll see how hard it is to move to 5 (I assume differences between 4 and 5 should be more manageable than those from 3 to 5). Should I get anywhere I'll post a new PR and notify you here.

@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 28, 2023

Have a pull request for font awesome. #3298
Maybe easier to do this piece by piece. If of interest, can then try to upgrade Jquery or remove it entirely from everything apart from the documentation. It seems to only be used for an ajax request, which could be programmed directly. Newer versions of bootstrap do not use Jquery. Other change made is to move javascrpt loading after main html and css loading to give a better user experience. Waiting outcome of your bootstrap upgrade, no need to replicate that if you will do it.

@bkmgit bkmgit closed this Nov 28, 2023
@lminiero
Copy link
Member

It seems to only be used for an ajax request, which could be programmed directly

We already use fetch, I think we have support for jquery's ajax stuff just as a fallback since it's what we used a long time ago, but I don't think anyone uses it anymore. We do make a heavy use of jquery for selectors though, in the demos code: I do realize it's something that can be done with plain JS, but it's also overly verbose and frankly a PITA. I'm sure a jquery-like wrapper can be done without using jquery, but I'm not that skilled of a web developer.

Anyway, please hold on to this until I have a PR ready for bootstrap 4/5. There will be a ton of code changes there already, and I'd rather make any further changes on top of that, rather than painfully merge any contribution back to that after the fact.

@lminiero
Copy link
Member

PS: we're also currently using jquery.blockUI, which does require jquery at the moment. As such, any attempt to get rid of jquery for good, will also involve finding a proper replacement for that functionality.

@Frenzie
Copy link

Frenzie commented Dec 7, 2023

PS: we're also currently using jquery.blockUI, which does require jquery at the moment. As such, any attempt to get rid of jquery for good, will also involve finding a proper replacement for that functionality.

Perhaps showModal() with some CSS trickery? Though I think that blockui thing is little more than an overlay with an element on top of it.

https://alexradulescu.github.io/freeze-ui/ seems to be written to be pretty much a drop-in replacement.

@lminiero
Copy link
Member

lminiero commented Dec 7, 2023

https://alexradulescu.github.io/freeze-ui/ seems to be written to be pretty much a drop-in replacement.

I bumped into that one too, but it didn't look like it would fit the bill. We don't use BlockUI just to block the entire page, but also individual elements in some demos. That said, for the moment I don't plan to get rid of jQuery.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants