Open michaelpelletier opened 8 years ago
I'm not a fan of this patch, the system currently sucks and it needs a total rewrite. It's a disgusting mess. That said, this is a step in the right direction and it's better than what we've got. @benjaffe let's nuke the current system and do just do a rewrite?
Removing JQuery UI is awesome though. Fuck that system with a load of bricks. @michaelpelletier++
What do you guys think of relying on Bootstrap, and migrating everything to use Bootstrap and bootstrap widgets? It'd make maintaining this a lot easier.
I agree that it could use a rewrite, but I also don't know that it makes sense to rewrite it until we have a better idea of how the UX should work, which is what I was playing with here. I think we can all agree that gigantic lists in an overflowing modal is probably not the best approach.
As far as adding new libraries, I don't love Bootstrap either. It might make maintaining easier, but it adds bloat, and I'm not sure that the benefits outweigh that. Although looking at the Manifest.json, it looks like we're already pulling in Bootstrap, so that may be a fine solution.
Something that I think we could benefit from bringing in is Underscore, since there are an actual billion cases where we're looping through arrays.
I'm a big fan of bootstrap in general, but in this case, since we're overriding CSS that OKC has written, many many of the cases will require custom CSS. That said, there are probably a lot of places that Bootstrap could help out (I'm thinking menus and dialogs).
We don't need Underscore for looping through arrays. We can use Array.map
for that. But a lot of this code is old and crusty, and not particularly functional or well-separated. Underscore could definitely help with a lot of it, so I'm open to bringing it in if it's needed. :)
I vote for lodash if we're going that direction. It'll run on the server-side/dev code if we write it. That said man, beggers can't be choosers. The big obstacle is yanking jquery-ui
+1 to lodash
Based off of https://github.com/benjaffe/chrome-okc-plugin/pull/133 so contains all of those changes as well, but I had another thought on how to handle category selection.
This may or may not remove the need for jQuery UI depending on where else it is used, but adopts a simpler "toggle" functionality instead of clicking and dragging between groupings. To my knowledge, the order that you put categories in didn't matter before (I may be wrong on this?), so it may be fine to drop the idea of dragging altogether.
Just an option to consider, at least!
(Non-Smoker is greyed out because that's the hover state, the screencapture removed my cursor)