Improve group management members screen

[dcavins]

The group management members screen is a bit odd.

The structure of the members lists is unlike any list anywhere else in BP. There are three members loops, one for admins, one for mods, and the members, and the admins and mods loops use bp_has_members() with the include argument while the members loop uses bp_group_has_members().

I'm attaching patches that make the structure more like other lists and employs bp_group_has_members() for all three loops.

See #7093, #7094, #7095 for related enhancements.

[dcavins]

List structure changes

[dcavins]

Use bp_groups_has_members() for all three loops.

[dcavins]

Manage members list before patch

[dcavins]

Manage members screen after patches.

[r-a-y]

Markup changes look fine to me. Would it make sense to mirror the markup of the /members/members-loop.php file here?

The switch to bp_group_has_members(), while it makes logical sense to do so, is actually more query-intensive as that utilizes BP_Group_Members_Query, which has to re-query for the group administrator / group moderator IDs and populate some group member data. Would like @boonebgorges to chime in here.

Also, just to keep in mind that offereins has a patch to split up the admin.php template into separate template parts - #7079. That patch would probably need to be refreshed as well.

[dcavins]

I’ve refreshed this patch as though #7079 has been committed. I’ve also made the following changes:

• Change structure of list items to more closely match members/members-loop.php as suggested by @r-a-y. (Should we be also updating groups/single/members.php to be more like that?)
• Added the action bp_group_manage_members_admin_item to the admin and moderator list items. (Already was applied to the member list.)
• Added “none found” messages for each loop to support the addition of a search box in #6385.

I thought more about using bp_groups_has_members() instead of bp_has_members() for the admin and mod loops, and, while I'm not assuming we'll go that direction, there are some advantages. If we used bp_groups_has_members() for the loops, we could also then use bp_group_member_joined_since() (which seems useful) and easily add pagination when necessary to the admin and mod loops via bp_group_member_needs_pagination().

I've run some performance comparisons, and with the loops as is, the page in my test environment (a small group) required 73 queries and .0327s of query time. Using bp_groups_has_members() for all three loops required 77 queries and .0266s of query time. I’ll attach both versions of the patch, so others can check it out if they’d like.

[dcavins]

Requires #7079 patch. Structural/markup changes. Does not change the loop function.

[dcavins]

Requires #7079 patch. Structural/markup changes. Uses bp_group_has_members() for all three loops.

[boonebgorges]

While @r-a-y is correct that there's a performance hit to using bp_group_has_members(), it's definitely the more semantic and consistent thing to do. My vote is to use bp_group_has_members(), and to work separately to eliminate redundant queries from BP_Group_Member_Query. See ​https://buddypress.trac.wordpress.org/ticket/5407#comment:7 for some prior discussion.

[dcavins]

Cleaned-up version that uses bp_groups_has_members() for all three loops and adds pagination.

[dcavins]

Example view in Twenty Thirteen

[dcavins]

Example of search results and messages.

[dcavins]

I've uploaded a new version of the patch above, 7105.04.patch and a couple of screenshots showing how the changes look. Everyone, but especially @mercime and @hnla: if you have a minute for a glance at the patch or a test drive, I'd appreciate your feedback.

I'm planning to commit this change before 2.7-beta because this is the perfect opportunity to change this template file, and the old screen isn't great for large groups.

One change I'm still considering making is adding an action hook in the "content" area of each member item so that devs could add other useful info to the member list, like extended profile data and such, to help group admins make good decisions.

Thanks for your feedback!

[mercime]

@dcavins Tested and worksforme. Checking a11y, new markup structure passes :)

One thing I'd like to see changed are the old demote/promote/etc/ links into buttons per issue I posted in ​bp-nouveau repo. Users of assistive technologies will be confused that the "promote link" and "demote link" do not go anywhere but do something else instead. But if it's not within this ticket's roadmap, let me know and I'll add it in new ticket re Buttons and Links :)

EDIT: using #members-list ID causes duplicate IDs when the Members widget is active on the same page as this screen. It's the old markup, duplicate IDs also happens on the Members Directory and Group > Members screens.

[hnla]

Nothing much to add really mercime has it covered so will step back, just be careful with changes on this scale it's one thing to test in our environs and passing as worksforme another to try and extrapolate how those changes might stack up in whatever custom scenarios that could exist - but indeed changes are technically proper the nesting of so much code in a heading element just jaw dropping - commit it!

[mercime]

@dcavins heads up that I added a <div> surrounding the three <div class="widgets"> for ARIA live region in r11126.

[dcavins]

In 11138:

Improve group manage members screen.

• Change structure of list items to more closely match

members/members-loop.php.

• Use bp_group_has_members() for each loop.
• Added the action bp_group_manage_members_admin_item to the admin

and moderator list items. (Already was applied to the member list.)

• Added “none found” messages for each loop to support the addition of

a search box in #6385.

• Added bp_group_member_joined_since() meta to member list items.
• Added pagination controls to each list when needed.

Props dcavins, hnla, mercime, r-a-y.

Fixes #7105.