Opened 15 years ago
Closed 7 years ago
#1269 closed enhancement (maybelater)
Create a BP_Loop_Template base class that all template classes can inherit from
Reported by: | rvenable | Owned by: | johnjamesjacoby |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Templates | Keywords: | needs-refresh, trac-tidy-2018 |
Cc: | mercijavier@… |
Description
Most of the classes in the templatetags.php files (for instance, BP_Blogs_User_Blogs_Template or BP_Groups_User_Groups_Template) all use common functionality for looping through an array of items. This functionality could be put into a base class. New template classes could be made simply by extending the base class and creating their own custom constructor. This would make it easier for plugin developers to quickly create new template classes.
I've attached a class I made that does what I am talking about (made from the example class in the BP Skeleton Component).
Attachments (5)
Change History (27)
#1
@
15 years ago
If this was accepted by the powers-that-be, it'd have to be written to work with PHP4.
#2
@
15 years ago
I agree with rvenable. Some kind of template class would really make developing much easier, without boring repetitive steps.
#4
@
14 years ago
- Component set to Core
- Owner set to johnjamesjacoby
- Status changed from new to assigned
Assigning to myself to go along with component API for 1.3.
#5
@
10 years ago
- Milestone changed from Future Release to 2.0.1
- Priority changed from minor to normal
- Severity set to normal
I am sad that this ticket got lost for so many years. It is a great idea.
Doing this correctly will mean writing lots of unit tests for the existing template loops, as they are all based on each other in some way, but have all been modified in ways that'll make them ideosyncratic. That said, I think it's worth doing, both because it'll make development easier for devs, but because it'll result in the removal of a fairly large amount of redundant code in BP.
Let's see if we can make some progress here for 2.1.
#6
@
10 years ago
- Milestone changed from 2.0.1 to 2.1
This is great! While working on Skeleton, i thought it would be nice to have a "generic" folder for custom components directory page with an index / generic-loop / generic-entry but this piece of the puzzle was missing. I think having this class would be awesome!
#11
@
10 years ago
Gave it a look today, there's a lot of loops, and some can use very specific logic.
For instance :
- getting the groups is using 3 different functions in case of invite / single groups / or groups
- most of the queries are building an array lookings like this
array( 'items' => $items, 'total' => $total )
, most of the time the first argument is a plural form relating to the component eg: activities, groups, blogs, notifications, but members is not used in favor of users - while most of the loops are offering a consistent action when loop start and ends, notifications is specific eg:
do_action( 'group_loop_start' )
,do_action( 'member_loop_start' )
,do_action( 'activity_loop_start' )
> not plural forms, butdo_action( 'notifications_loop_start' )
> plural form
I think we should progressively add core loops but offer plugin devs an abstract class they can use in their plugins. Then we will be able to iterate and improve it thanks to plugin devs feedbacks.
1269.01.patch is a first example with the blogs component.
I've first built unit tests before editing the BP_Blogs_Template
class
then tested it extending the BP_Template_Loop
class
#12
@
10 years ago
imath - Thanks for your research and work on this!
It's a bit annoying (although not surprising) that BP's loops are inconsistent in the ways you've described. But, as you note, we shouldn't let that hold us up. Core subclasses like BP_Groups_Template
can just override additional parent methods to maintain backward compatibility, and where necessary, we can break up some of the big methods so that the bits that get overridden are smaller.
A couple principles going forward:
- In cases where you're overriding a non-abstract class - that is, a specific component's template loop has to override a
BP_Template_Loop
method for backward compatibility or because it's got to do something special - let's make sure that the docblock explains the reasoning. - As noted above, if we find that we have to override something in
BP_Template_Loop
, let's try to make it as narrow as possible. If that means breaking up a big method so that it calls some smaller methods, then let's do it. - I think we'll need to maintain methods like
rewind_blogs()
, just in case anyone is calling them directly. We can turn them into wrappers forrewind_items()
etc, and if you want, we can throw deprecated function notices (though I don't care one way or another about that). - Let's make sure that the docblocks for the
BP_Template_Loop
hooks are really good. These will be dynamic hooks, so we want to make sure that they're documented thoroughly.
#13
@
10 years ago
you're welcome :)
In 1269.02.patch, i'm trying to include 3 of your points. Adding the groups component didn't require to break up the set_loop()
function into smaller parts so far. So i've left it the way it was. I'll see for the other components if we need to.
Seems to also work fine for the groups component, added unit tests for each get_items()
possibilities. Though it made me wonder why are we setting a sortby argument in the pagination 'add_args' as it's not previously set ?
#14
@
10 years ago
- Keywords has-patch added
I've been working on adapting every component's template loops (except for the legacy forums one) so that they extend the BP_Template_Loop
.
Unlike the previous patches, this time i'm separating unit tests from the patch, because i've used them as a base to check i wasn't breaking anything. So you'll find the unit tests in 1269.unittests.patch.
You'll see in 1269.03.patch that some components use a very specific logic for their loops (the group invite one for instance), but the more complicated was the xProfile one. Actually BP_XProfile_Data_Template
is somehow doing 2 loops in one (groups and fields). So i've chosen to split them :
BP_XProfile_Data_Template
is mainly taking care of profile field groups- a new class
BP_XProfile_Fields_Template
is looping fields.
Everything went fine doing the split, but a template issue forced me to keep a bunch of "fields" method in BP_XProfile_Data_Template
to avoid breaking profile loops for BP-Default or standalone BuddyPress themes.
The problem is located in 3 templates :
src/bp-templates/bp-legacy/buddypress/members/register.php
src/bp-templates/bp-legacy/buddypress/members/single/profile/edit.php
src/bp-templates/bp-legacy/buddypress/members/single/settings/profile.php
In these templates, the profile loop is not checking if the group has fields using bp_profile_group_has_fields()
, they directly do a while ( bp_profile_fields() ) : bp_the_profile_field();
. So i've edited each of these templates adding the 'has_fields' check as i actually need it to init the BP_XProfile_Fields_Template
class.
I think it's ready for testing, and i'd love to have your feedbacks.
#15
follow-up:
↓ 16
@
10 years ago
This looks really really great, imath!
I'm going to try and look through this in the next few days.
I'm kind of scared about plugins that might have extended a BP_X_Template
class. I know BP Groups Hierarchy extends BP_Groups_Template
, but there might be others.
#16
in reply to:
↑ 15
@
10 years ago
Thanks a lot r-a-y for your feedback, and i agree about:
I'm kind of scared about plugins that might have extended a
BP_X_Template
class. I know BP Groups Hierarchy extendsBP_Groups_Template
, but there might be others.
I'll run some tests on this plugin to see how we can be more confident about it :)
#17
@
9 years ago
- Keywords needs-refresh added
Just applyed the patch, it's outdated and really needs to be refreshed.
#18
@
9 years ago
- Milestone changed from 2.3 to Future Release
This would be super cool, but needs a champion.
#21
@
7 years ago
- Keywords trac-tidy-2018 added
We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.
Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.
If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.
For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/
Base class BP_Loop_Template