add ability to search developers by their bio or hero #341
Conversation
| <%= inline_svg_tag "icons/solid/chevron_down.svg", class: "flex-shrink-0 -mr-1 ml-1 h-5 w-5 text-gray-400 group-hover:text-gray-500" %> | ||
| </button> | ||
|
|
||
| <% if Feature.enabled?(:filter_by_search_query) %> |
There was a problem hiding this comment.
Is it possible to not have an else statement on the feature flag? Even if that means moving Sort all the way to the right even before this feature is enabled.
There was a problem hiding this comment.
Yeah the Sort button will be moved to the right side if we don't add the else here. So do you suggest to remove the else statement?
There was a problem hiding this comment.
Fine by me! The search will eventually be ready for everyone, eventually.
|
Hey @themudassarhassan, is this ready for review? |
|
@joemasilotti I left a question for you on the PR I think you have missed it. |
|
Weird, I'm not seeing it! Can you link me or point to the code here? |
|
Here is the link. Pardon me if I didn't get it right. |
|
Weird, I'm not seeing anything. Did you do a review and not submit it? Either way, want to copy-paste the question here?
|
|
Oh so sorry! You are right. I didn't submit the review. |
|
I have pushed the changes. Ready for review and merge |
There was a problem hiding this comment.
Looking good! I'm really excited for this one :)
I left a few questions/suggestions in the code. Along with those, 4 things:
-
Apologies, but I'm going back on an earlier decision :) Let's keep the Sort button on the left like you had before, even if that means an
elsestatement. It looks a little weird when bothrole_level_filtersandsearch_featureare disabled. -
It looks like the Sort dropdown on mobile is now rendering off the screen.
- On tablet (of certain sizes) the search bar butts up against the other filters. Let's add some right margin or space between to make sure there is a little room to breath.
- I know that I need to hit Enter to search, but I think we need a button. Let's add a Search button to the right of the search field that submits the form (along with Enter).
| <div class="mx-4 pb-4"> | ||
| <%= form.label :search_query, t(".search_query_label"), class: "block text-sm font-medium text-gray-700" %> | ||
| <div class="mt-1"> | ||
| <%= form.text_field :search_query, value: search_query, class: "shadow-sm focus:ring-indigo-500 focus:border-indigo-500 block w-full sm:text-sm border-gray-300 rounded-md" %> |
There was a problem hiding this comment.
| <%= form.text_field :search_query, value: search_query, class: "shadow-sm focus:ring-indigo-500 focus:border-indigo-500 block w-full sm:text-sm border-gray-300 rounded-md" %> | |
| <%= form.text_field :search_query, value: search_query, class: "shadow-sm focus:ring-gray-500 focus:border-gray-500 block w-full sm:text-sm border-gray-300 rounded-md" %> |
test/queries/developer_query_test.rb
Outdated
| @@ -126,7 +132,8 @@ class DeveloperQueryTest < ActiveSupport::TestCase | |||
| utc_offsets:, | |||
| role_types: [:part_time_contract], | |||
| role_levels: [:junior], | |||
| include_not_interested: true | |||
| include_not_interested: true, | |||
| search_query: "" | |||
There was a problem hiding this comment.
Let's add some query here so it tests the value is getting passed along correctly.
|
Hey, @joemasilotti I have pushed all feedback changes except one which is adding a search button next to the search field, regarding which I need some input. This is the UI on tablet layout, search input, button, and filters are quite close on this layout as shown in the image below: We cannot add space because there isn't any available, what do you suggest? |
|
Ha, wow that's getting crowded. Let's leave off the submit button for now. I'm hoping to redesign these soon anyways. Is everything ready for review then? |
|
Yes all other changes are done. |
|
This looks great – thanks again for knocking this out! |
This PR is aiming to add a search field on
/developerspage to allow users to filter developers by their bio or hero. I have used thepg_searchgem which implements the full text search feature of PostgresSQL usingto_tsvectordata type. Although this approach will work fine now but it will eventually get slow when we have hundreds of developer profiles so we also need to add index for that. I'll be creating a separate issue to add more details regarding its implementation.Pull request checklist
bin/check