idno / known

A social publishing platform.
https://withknown.com/opensource
Other
1.06k stars 194 forks source link

Javascript needs to be rewritten and namespaced #1920

Open mapkyca opened 7 years ago

mapkyca commented 7 years ago

While trying to do this:

Using lightbox plugin

I encountered this error:

Images didn't load

Some other notes:

Isolated this to the image.js Image() declaration causing some sort of namespace collision. I suspect this'll keep happening, so these libraries should be rewritten to use a namespace, or similar.

See:

https://appendto.com/2010/10/how-good-c-habits-can-encourage-bad-javascript-habits-part-1/

benwerd commented 6 years ago

There would be a lot to be said for moving to ES6, if we could.

mapkyca commented 6 years ago

Aye, that's sort of what I'm thinking. I'm not a JS expert tbh, but Known doesn't have a great deal in the core files so shouldn't be too much of a pita

mapkyca commented 6 years ago

Classes and modules would be interesting: https://webapplog.com/es6/

Browser support is the only big issue, we should take a view on that as and when.

mapkyca commented 6 years ago

Swinging back to this - now minify is handled by Grunt, we could stick a babel task on prior to that to backport.

Any thoughts?

johanbove commented 5 years ago

If we want to stick using Grunt (and not Webpack), then I could check out this Grunt plugin and see how to move the Known JS code into modules first, then we can also refactor/upgrade to ES6: https://github.com/babel/grunt-babel

mapkyca commented 5 years ago

I've already moved grunt over to using babel, and so we can write ES6.

So, I'd look at probably using classes and all that good stuff.