Skip to:
Content

BuddyPress.org

Opened 8 years ago

Last modified 3 years ago

#3653 new defect (bug)

Clicking on "All Members" tab not working after a search.

Reported by: gingram815 Owned by:
Milestone: Awaiting Contributions Priority: normal
Severity: normal Version: 1.5
Component: Members Keywords: needs-patch 2nd-opinion
Cc: alexander.berthelsen@…

Description

WP version 3.2.1
BP 1.5

In the default BP theme, after searching the member directory using the search form at the top of the members directory page or using a query string, clicking on the "All Members" tab does not show all the members as it does not clear the previous search.

View my site with query string at Doctor.

Thanks,
Glenn

Change History (3)

#1 @boonebgorges
8 years ago

  • Keywords needs-patch 2nd-opinion added
  • Milestone changed from Awaiting Review to Future Release
  • Version set to 1.5

Confirmed. The issue is this.

  • Clicking 'All Members' launches an AJAX request handled by bp_dtheme_object_template_loader()
  • That function loads members/members-loop.php
  • That template does a bp_has_members() loop using bp_ajax_querystring()
  • In bp_has_members() we check the $_REQUEST global for search terms 's', and filter based on it

There seem to be at least two problems here.
(1) We (probably) shouldn't be looking at URL arguments out of $_GET when dealing with AJAX requests
(2) We (probably) shouldn't be filtering based on $_REQUESTs? in such a forced way in bp_has_members()

Problem (2) is a more general issue. IMO whatever else we do, we should move this logic http://buddypress.trac.wordpress.org/browser/tags/1.5/bp-members/bp-members-template.php#L309 *above* the parsing of default/function args. That way, it'll be possible to turn off the search_terms even when there's something hanging around in $_REQUEST.

That, by itself, probably won't fix the problem here, since the template will still default to $_GETs? in its query. Perhaps we want to make an exception in bp_dtheme_ajax_querystring() to the effect that, when DOING_AJAX, pass a null value for search_terms? Would like to hear feedback from others.

#2 @mercime
3 years ago

Confirming that this is still an issue as of BP 2.7 trunk.

#3 @lakrisgubben
3 years ago

  • Cc alexander.berthelsen@… added

This was a tricky one. ;)

I think that only half of this problem still exists, if you have a search in your querystring and then click on all members the search won't be passed through to that tab switching (since https://buddypress.trac.wordpress.org/browser/trunk/src/bp-templates/bp-legacy/js/buddypress.js#L897 will set search_term to an empty string because the searchbox input'll be empty) . But if you do the search by typing in the search box it'll be passed through to the tab switching.

It seems to me that the current behaviour for when you've manually typed a search, i.e. If you've searched for something and then click on all members the search is passed through to the ajax request for all members (so that the request is actually "all members/my friends that also match the search") is the intended behaviour. At least looking at the code in https://buddypress.trac.wordpress.org/browser/trunk/src/bp-templates/bp-legacy/js/buddypress.js#L897 which passes the value of the search field along to the ajax handler for the tab switching. I find this kind of intuitive since the search box lives above the tabs, the tabs are kind of a way to filter the search even more... But I agree that it's not totally obvious that this is the case.

Another problem is that if you've come to the members listing with a search in the querystring, i.e. /members/?members_search=search that'll at first load the page with that search, but if you switch tabs the search doesn't follow along since it's only added as a placeholder to the searchbox, not as the value (and to make things even more confusing, if you go to /members/?s=search it'll also do a search, but not add a placeholder) which makes for a kind of disjointed experience since it differs depending on if you've typed the search manually or made the search by following a link. This could easily be fixed by adding the value of the members_search querystring to the searchbox as a value instead of a placeholder. It wouldn't fix the use of ?s= as querystring but at least would make it a bit more coherent.

The big point here I think is deciding on if the search should be passed through to the tab switching, but in the meantime might be worth to just add the value of the query string to the searchbox input field (i.e. https://buddypress.trac.wordpress.org/browser/trunk/src/bp-templates/bp-legacy/buddypress/common/search/dir-search-form.php) so that they at least behave in a coherent way... If that'd be interesting I'd be happy to make a patch for it. :)

Note: See TracTickets for help on using tickets.