AamAadmiParty / cleansweep

Volunteer & Campaign Management System
Other
40 stars 21 forks source link

Added search to volunteers page #162

Closed anandology closed 8 years ago

anandology commented 8 years ago

We already have volunteer search accessible from edit committee pages. Make it available on the volunteers page.

raghavio commented 8 years ago

Ajax or form?

anandology commented 8 years ago

Thanks for the PR, @RaghavPro. Couple of issues I want you to address.

  1. search should be GET request, not POST.
  2. The search string should be displayed in the search box when displaying the results.
  3. Hide the action buttons when showing search results.
  4. clearly indicate that these are search results and not entire list of volunteers.

My suggestion for 4 is to display a message like this:

Displaying volunteers matching "Anand". [show all]

raghavio commented 8 years ago

Sure.

Date: Thu, 4 Feb 2016 04:17:56 -0800 From: notifications@github.com To: cleansweep@noreply.github.com CC: Raghav_ftw@hotmail.com Subject: Re: [cleansweep] Added search to volunteers page (#162)

Thanks for the PR, @RaghavPro. Couple of issues I want you to address.

search should be GET request, not POST. The search string should be displayed in the search box when displaying the results. Hide the action buttons when showing search results. clearly indicate that these are search results and not entire list of volunteers.

My suggestion for 4 is to display a message like this:

Displaying volunteers matching "Anand". [show all]

— Reply to this email directly or view it on GitHub.

anandology commented 8 years ago

Thanks @RaghavPro for doing the improvements in 9e8b4f94ab364fd722ff06919e8ad2849c22c607.

anandology commented 8 years ago

Found a bug.

It is showing only first 10 entries.

Make the limit to 100 and change the message to "showing first 100 volunteers matching xxx" if the count is 100.

Also I suggest using the query param q instead of search-query.

raghavio commented 8 years ago

This is done I think.