Use bp_activity_set_action() to fill bp-templates activity dropdowns

[imath]

bp_activity_set_action() is used to register the activity actions.

This helps building the drop down types in Activity Administration and registering the callable functions that will dynamically generate the translatable action strings.

But in bp-templates :

• activity/index.php
• members/single/activity.php
• groups/single/activity.php

the filters are directly built in it. I've seen that in 2.0 , to avoid backcompat theme issues the 'Profile Updates' type was introduced using one of the templates hook to add a new activity type into the drop down ('bp_activity_filter_options').

I think we can improve this by using bp_activity_set_action() to also set the different template dropdowns options. This way we would (and plugins) only need to use this function to feed admin and front drop downs.

It would also avoid the different bp_is_active() checks in templates. Finally i think it won't have any impact on theme backcompat.

So i suggest the patch attached to this ticket.

[boonebgorges]

I like this idea. A couple thoughts:

1. You are excluding types from the group members screen with a blacklist. This is a bit messy. Maybe 'members' and 'group_members' could be different scopes.
2. The labels you're using in each component don't seem to match the values currently hardcoded into the templates. They probably should.
3. Strings should be escaped when building the <option> markup
4. What's the purpose of casting $label to an array and then doing a foreach() loop on it? Is there ever more than one label?
5. Looks like we never include a 'new_blog' filter. This is odd. (Probably a subject for a separate enhancement ticket.)

[imath]

Replying to boonebgorges:

Updated the patch to 5367.02.diff

1. You are excluding types from the group members screen with a blacklist. This is a bit messy. Maybe 'members' and 'group_members' could be different scopes.

That's really better, thanks a lot for advice, i've created the 'members_groups' scope because it's in a member's profile groups tab

2. The labels you're using in each component don't seem to match the values currently hardcoded into the templates. They probably should.

I've checked again, in previous version i forgot to add a domain, but i don't find the wrong labels :( There are 3 labels, i've created as they were not existing :

Activity comment => __( 'Update replies', 'buddypress' )
New Blog         => __( 'Site creations', 'buddypress' )
Changed Avatar   => __( 'Profile Picture Updates', 'buddypress' )

3. Strings should be escaped when building the <option> markup

Oops! Corrected this. Thanks.

4. What's the purpose of casting $label to an array and then doing a foreach() loop on it? Is there ever more than one label?

I think around midnight i like to make my life harder! You're right, i've changed this as anyway i need to loop in actions to get the different labels.

5. Looks like we never include a 'new_blog' filter. This is odd. (Probably a subject for a separate enhancement ticket.)

Yes i guess so, we would only have to set the label & scopes ;)

[boonebgorges]

In 5367.03.patch I make the following changes:

• Call it 'context' instead of 'scopes'. I think this is clearer.
• Change the wording of the newly introduced labels
• Change the context names. 'group' and 'member' seem to be more accurate to me, since they're for use on single group/member pages
• Instead of passing a context to bp_get_activity_show_filters() in the template, do some logic in that function itself to determine the proper context. This is more similar to what we do for other template functions (eg bp_has_activities()). It also avoids your ugly hack of adding _groups to the end of the context in some cases - all the logic is now centralized, but can be overridden by passing a value for $context.
• Change the "special case" for friends - your technique was maybe unnecessarily abstract, and would any kinds of duplicates. Maybe we want to do this, but we should talk about it first.
• Clean up code styling a bit

[imath]

It looks really great IMO.

I'm adding 5367.04.patch to use bp_get_groups_slug() instead of 'groups' in the switch cases, because if the groups slug is defined differently using for instance :
define( 'BP_GROUPS_SLUG', 'groupes' );
Then all 'member' context activity type are displayed instead of the 'member_groups' one.

I've also added a filter so that it's easier to disable an option. I know it's possible to filter bp_activity_set_action but i thought it would be simpler to restrict options directly from the template function bp_get_activity_show_filters().

[boonebgorges]

Thanks, imath! Looking good. Regarding the extra filter: I agree that the added filter will make it easier to remove items. This makes me think that maybe it should be the *only* filter. Two of them seems like overkill. (Why would you need to filter the markup?) I'm just going to do the filter on the array for now, and if someone requests the filter on the markup later, we can consider it then.

[imath]

Replying to boonebgorges:

I'm just going to do the filter on the array for now, and if someone requests the filter on the markup later, we can consider it then.

Totally agree :)

[boonebgorges]

In 8428:

Introduce bp_activity_show_filters(), and use it for generating dynamic activity filter dropdowns

Activity filter dropdowns were previously hardcoded into templates, with
template-specific hooks for plugins to add their own filter values. This
resulted in an inconsistent and awkward process of maintaining filter lists
between components. The new method introduces new parameters to bp_activity_set_action():
$context (an array of the contexts in which the filter should appear) and
$label (the text that should be used when generating the <option> markup). The
new function bp_activity_show_filter() is then used in the templates to
generate a dynamic list of <option> markup, based on the values registered by
the activity types.

Fixes #5637

Props imath

[imath]

Sorry boonebgorges, i just noticed the use of bp_get_groups_slug() when groups component is not active is causing an error :(
Suggesting 5637.05.patch to avoid it.

[boonebgorges]

In 8429:

Check that Groups component is active before using bp_get_groups_slug() in bp_activity_get_show_filters()

Fixes #5637

Props imath