-
Notifications
You must be signed in to change notification settings - Fork 156
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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' );` |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.
Stop using
BP_Component
in favor ofBP_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.