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

Test if JQuery can be fully removed by upgrading Bootstrap #1233

Open
Jaifroid opened this issue Mar 16, 2024 · 8 comments
Open

Test if JQuery can be fully removed by upgrading Bootstrap #1233

Jaifroid opened this issue Mar 16, 2024 · 8 comments
Assignees
Labels
cleanup dependencies Pull requests that update a dependency file
Milestone

Comments

@Jaifroid
Copy link
Member

I have now removed all JQuery from Kiwix JS code, but as mentioned in #1225 and #367, our version of Bootstrap still requires it, so we still have to load it.

We need to test whether we can upgrade Bootstrap to the next version that no longer uses JQuery, while still keeping backward compatibility with IE11+, Firefox OS, etc. It's possible that Babel can provide that compatibility, even if the Bootstrap library doesn't support older browsers any more (which I believe is the case). It needs testing.

@Jaifroid Jaifroid added cleanup dependencies Pull requests that update a dependency file labels Mar 16, 2024
@Jaifroid Jaifroid added this to the v4.2 milestone Mar 16, 2024
@AritraLeo
Copy link

Before jumping in I'd like to know if I am right or wrong.
Firstly I'll have to check for compatible version of bootstrap and update package.json accordingly. Then, Search the codebase for any instances where jQuery is directly used with Bootstrap components or functionality. Replace these jQuery-dependent code snippets with their vanilla JavaScript equivalents. Finally, test the application in various browsers, including older ones like IE11+ and Firefox OS, to ensure that the upgraded Bootstrap version maintains compatibility.
Is it ?

@Jaifroid
Copy link
Member Author

Actually, for this issue, you just need to update Bootstrap to a version that doesn't rely on JQuery and remove any mention of JQuery from index.html and from rollup.config.js. Note that for legacy reasons, Bootstrap is attached in index.html, but it might be fine to keep it there just for testing. We have an automated test for IE11, so that should be enough. You would be able to test manually in IE mode in Microsoft Edge if it needs debugging (assuming you have access to a Windows machine), or I can explain how to run IE11 (there's a hidden secret to doing it).

It might be necessary to adapt some of our Bootstrap classes in index.html -- I'm not sure. I wouldn't jump to the latest Bootstrap, as it might not work with older browsers. I'd just go one major version up from our current version.

If necessary, you might need some guidance on how to make sure the bootsrap dependency is rewritten by Babel (in rollup.config.js), but that's a bridge we could cross after the initial checks are done.

@Jaifroid
Copy link
Member Author

Let me know if you want to try. Take a look at CONTRIBUTING.md in this Repo to get started (read it carefully all the way through -- there are some gotchas!).

@AritraLeo
Copy link

Let me know if you want to try. Take a look at CONTRIBUTING.md in this Repo to get started (read it carefully all the way through -- there are some gotchas!).

Ya sure I would love to give it a shot! Thanks for such a descriptive response and sharing the CONTRIBUTING.md file I'll read it carefully as well !

@AritraLeo
Copy link

Usually I start working on a issue directly but as per CONTRIBUTING guide I should ask you to assign me the issue.
I'll raise a PR with initial changes requested shortly!

@THEBOSS0369
Copy link
Collaborator

Hey @Jaifroid ! Can i work on this issue ,for that <section> tag issue on mwoffliner as i wasn't able to start server so i will be waiting till i get that issue fixed and will be working on this issue.

So for this issue this will be my approach

  1. I will first upgrade to Bootstrap v5 as newer version might not work on older versions, i will do it by updating package.json
  2. I will try to remove any direct reference to jquery from index.js and rollup.config.js
  3. will do Test on the Edge's PE mode as you suggested

Will this approach would work?
Thanks

@THEBOSS0369
Copy link
Collaborator

Hey @Jaifroid ! Shall i go ahead and work on this issue with the approach i gave above? Thanks

@Jaifroid
Copy link
Member Author

@THEBOSS0369 I just need to check that @AritraLeo doesn't want to finish his PR which was actually already very close to solving this issue, but wasn't passing tests, so let's wait a few more days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

3 participants