Skip to content

Toggle categories#138

Open
michaelpelletier wants to merge 9 commits intophoebejaffe:masterfrom
michaelpelletier:toggle-categories
Open

Toggle categories#138
michaelpelletier wants to merge 9 commits intophoebejaffe:masterfrom
michaelpelletier:toggle-categories

Conversation

@michaelpelletier
Copy link
Copy Markdown
Contributor

Based off of #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!

screen shot 2015-07-23 at 10 57 21 am

(Non-Smoker is greyed out because that's the hover state, the screencapture removed my cursor)

@EvanCarroll
Copy link
Copy Markdown
Contributor

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?

@EvanCarroll
Copy link
Copy Markdown
Contributor

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.

@michaelpelletier
Copy link
Copy Markdown
Contributor Author

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.

@phoebejaffe
Copy link
Copy Markdown
Owner

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. :)

@EvanCarroll
Copy link
Copy Markdown
Contributor

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

@phoebejaffe
Copy link
Copy Markdown
Owner

+1 to lodash

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.

3 participants