-
Notifications
You must be signed in to change notification settings - Fork 260
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
Changing tables #248
Changing tables #248
Conversation
$('.listItem.not_unisex').hide() | ||
|
||
$('#changing_table_filter').click -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible that we'll want to start pulling out all these boolean icons to enums rather than listing all of them here. Doesn't need to be part of this though, just a follow up. By no means is three icons unwieldy :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really think there is a rails convention that would help here. These technically are three different filters. There may be a gem that will help with this, but since we are doing this filtering in javascript -- I don't know if it would really help without a refactor here. I'm fine with this for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I too don't think it's necessary at this point but I think we'd want to use a bitfield, not an enum. Here's a popular one that's named after a silly looking dog https://github.com/pboling/flag_shih_tzu :)
A few minor comments. I don't have objections to the icon chosen, although maybe others have ideas |
Other than that, I'm fine with this. =) |
gtg, gratz on your contribution! @azelma thank you! |
Add field to indicate whether a restroom has a changing table, per: #233
I don't know what icon should be used for this. The best I could come up with was the fa-child one, but I think there's probably better out there, open to suggestions.
Screenshots:
@tkwidmer