Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start using BP_Component_Feature to organize the Members Invitations feature #403

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

imath
Copy link
Contributor

@imath imath commented Nov 1, 2024

Stop using BP_Component in favor of BP_Component_Feature to avoid messing with BuddyPress component globals and "faking" a component to register the feature navigation.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/9254


This Pull Request is for code review only. Please keep all other discussion in the BuddyPress Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the WordPress Core Handbook for more details.

@imath imath requested a review from dcavins November 1, 2024 11:28
Copy link
Contributor

@dcavins dcavins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this is a great improvement for making sense of how these pieces fit together. Having an API-style way to register features of components will really help us in the long run. Thanks!

* The BP Members Invitations feature is optional.
*
* If you want to disable it, you can use:
* `add_filter( 'bp_is_members_invitations_active', '__return_false' );`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This note seems confusing because we have a toggle in WP Admin > BP Settings to disable invitations. I understand that this filter will remove the option from the settings screen as well, so maybe:

If you wish to hide all references to member invitations in the BP Settings screens, you can use:
add_filter( 'bp_is_members_invitations_active', '__return_false' );

If you simply wish to disable the invitations feature, you can uncheck the checkbox for the following option:
"Allow registered members to invite people to join this network"

* @since 15.0.0
*/
public function globals() {
buddypress()->members->invitations = new stdClass();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class seems to be only used to store the query_loop as used in bp-members-template.php. Would it make more sense to fold it into the members->active_features->invitations object in the future?

public $is_feature = true;

/**
* Disable the `BP_Component()` starting method.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand that you're doing this to help differentiate features from components and essentially have two distinct setup routines. Maybe the note here should say explicitly that classes that extend BP_Component_Feature should use the init() method and classes that extend BP_Component should use start(). I'm not sold on this wording, but this section of the code confused me at first look, so I wonder if we can help code readers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants