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

Removing the jQuery dependency #3

Open
robinpokorny opened this issue Sep 17, 2013 · 4 comments
Open

Removing the jQuery dependency #3

robinpokorny opened this issue Sep 17, 2013 · 4 comments

Comments

@robinpokorny
Copy link

Hi Kumail,

this project is just a great idea!

I was a bit curious why it needs jQuery so I looked into the code and saw that the use can easily be replaced with pure JavaScript.

Therefore I propose to remove the dependency.

@kumailht
Copy link
Owner

@foleyatwork has removed the jQuery dependency in his fork. https://github.com/foleyatwork/responsive-elements

I'd love to merge it in once it gets tested well enough and works rock solid on all platforms and scenarios. For now, jQuery seems a bit more stable. But in time, I'd like to remove the dependancy.

@foleyatwork
Copy link

I think three things need to happen before we can remove the dependency on Kumail's official version.

  1. A RegEx add/remove class function instead of el.classList, which is not widely supported. This should be pretty easy, there are a lot out there. If anyone has a favorite please let me know.
  2. Lots of cross browser testing. Hopefully since this project has gotten a lot of traction we can get a bunch of people to help identify/patch bugs.
  3. General clean-up. I think my translation code was okay, but I know I'm not the most brilliant programmer in the world. If you think any part of the code sucks, tell me and I'll fix it in the non-JQuery version.

@kumailht
Copy link
Owner

The current library supports almost all versions of IE. I don't mind dropping support for older versions but it would be nice to support atleast IE8 and above.

It is worth noting that el.clientWidth is implemented differently across browsers:
https://developer.mozilla.org/en-US/docs/Web/API/element.clientWidth
http://stackoverflow.com/questions/833699/clientheight-clientwidth-returning-different-values-on-different-browsers

I really like how you abstracted out the breakpoint parsing into the parseBreakpointClasses method. Good work! :)

A more practical approach would be to convert all the easy, no-brainer stuff to just javascript. Like removing $.each, $.ready, $.attr, etc. A separate pull request for each bit would make it easier to test (unit tests and manual testing).

@crispen-smith
Copy link

regarding the idea that classList is "not widely supported"... there are very good shims. Wouldn't it be better to conditionally load a shim rather than loosing the work that classList provides as a native object in browsers that do support it?

Also, by using a conditionally loaded shim approach if there are other plugins within a project that may have (or will) shim in classList there's no risk of loading multiple regex solutions for those plugins.

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

No branches or pull requests

4 participants