-
Notifications
You must be signed in to change notification settings - Fork 6
Filters on junior story model #16
Filters on junior story model #16
Conversation
…lter params like so: the_url.tld/junior_stories?gender=female
Thanks so much for this @TPei ! I'll try to take a look tomorrow. |
…seearch terms to make this more usable
@@ -33,4 +37,41 @@ def junior_story_params | |||
:remote, :tech_team_size, :company_size, :company_age, :person_of_colour, :other, | |||
:publishing_consent, :freelancer) | |||
end | |||
|
|||
def allowed_params |
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.
these values are exactly the same as the junior_story_params
above, aren't they - what's the benefit of doing it this way?
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.
Nope, the difference is in the require(:junior_story)
. Do we really need that above?
Hey @TPei thanks so much for this! There's some really awesome things happening here. I've made a few comments along the way on things I don't understand/could be moved. I'm also learning here & don't always know the best way to do things, so this is an adventure for the both of us. In general though I think this is really awesome & a definite improvement! Sorry I've been so slow on further developing this but you know... life :) Additionally, I have some further comments/thoughts:
So, those are my thoughts! I think though that some of these can be pulled out into separate issues. Where do you think we should go from here? If you want to address my in-line comments, I have no problem merging this & then we can go about changing the modal to a filter system. Thanks again! 🎉 |
Heyho, a lot to comment on here, hang on^^
UX improvements like positioning of the filter fields and visibility of filtered fields/values could be a follow-up ticket imo |
Updated TODO:
|
Sounds good!
Yes, we have a lot, but I was thinking we would only show a few, not all, otherwise it's quite overwhelming. Maybe only 5, max. It could be a side bar or a top bar. That being said, I'm fine with leaving the modal in for now & seeing how it develops. UX improvements in a separate ticket are fine w/ me. I'm happy to merge once the drop downs are in the modal :) |
I'm not really sure how to best go about this... |
hey @TPei sorry for being so slow about this! Pls feel free to ping me in slack next time! In your comment above, what part exactly are you talking about? Where are you wanting to replicate it? Do you mean in the filtering modal? I suppose you could always pull out the filterable attributes into a partial & include it in the form & then also in the modal? Does that make sense/answer your question? |
Would you say we could merge this and create follow-up issues @berlintam ? I don't see myself having much time anytime soon and this way someone could take over. Filtering works flawlessly (including exporting only the matched items), only the UI is a bit cluttered :) |
Hi @TPei thanks for all your hard work on this! I'm fine with merging but can you pls merge master into it to fix conflicts first? (let me know if you need any help with this) |
all done @berlintam 😄 |
Thanks for all your hard work on this @TPei! |
allows to filter for several fields on the junior_story model
TODO:
BONUS, but maybe later in separate branch:
income > 5000
income > 5000, income < 10000
or
and maybenot