#7210 closed enhancement (fixed)
Group type frontend integration
Reported by: | r-a-y | Owned by: | |
---|---|---|---|
Milestone: | 2.7 | Priority: | normal |
Severity: | normal | Version: | 2.6.0 |
Component: | Groups | Keywords: | has-patch |
Cc: | mercime.one@…, espellcaste@… |
Description
We introduced the Group Types API in #6784.
This ticket is to flesh out the API for frontend integration.
Attached patch does the following:
- Integration into the group "Create" and "Manage > Settings" pages:
- Group type link integration into group header:
- Group type directory integration:
Dev notes:
Group type directory - I've set a marker (buddypress()->groups->current_directory_type
) in groups_directory_groups_setup()
to determine if we're on a valid group directory type. This differs from the marker set for member type directories, which is set in our URI parser (bp_core_set_uri_globals()
).
bp_groups_set_group_type() - $group_type
parameter now accepts an array, so you can pass multiple group types for saving instead of just one group type.
To-do:
- Group template hierarchy integration.
- Admin metabox currently uses a
<select>
dropdown box, but if a group has multiple types, a super admin cannot select more than one type. Probably refactor this to use a multiple<select>
dropdown box.
Attachments (5)
Change History (46)
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
8 years ago
#5
@
8 years ago
02.patch
includes some new parameters to bp_groups_register_group_type()
:
'has_directory'
- see comment:1.'show_in_create_screen'
- if set totrue
, the group type can be selected on the frontend group creation screen and in the group's"Manage > Settings"
page. Default:false
.'show_in_list'
- if set totrue
, the group type will appear in the group type list as rendered bybp_group_type_list()
(Screenshot ). This defaults totrue
if'show_in_create_screen'
is set totrue
. Default:null
.
This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.
8 years ago
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
8 years ago
#8
@
8 years ago
FYI, I'm going to commit this in the next day or so.
Then, I'll do further iterations to the admin metabox and template hierarchy afterwards.
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
8 years ago
This ticket was mentioned in Slack in #buddypress by mercime. View the logs.
8 years ago
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
8 years ago
#19
@
8 years ago
@hnla made a good point in Slack today about adding in a new parameter so a group type can be automatically checked during the group creation process.
This is just a note for me to return to this on the weekend.
#20
@
8 years ago
Just moving my general thoughts from slack to ticket
Allowing for a value to be set to represent a default checked=checked on register is a good idea, let the dev or admin decide which 'type' represents a 'normal' or 'default' group for their install.
Of a lesser note are we concerned that in an existing install of groups if one then sets a new group 'type' then creates or associates a group to that type we now have a group list with a column that shows a group type string(label) for perhaps one group but the rest of the type columns for groups are empty, likewise create steps shows one group type, the possibility of creating a non group type exists but isn't clear?
My feeling is that we do:
Types are registered with bp_groups_register_group_type
this action prompts BP to assume a type 'Normal' (terminology is wrong but...)
BP sets a type 'normal' for all groups unless create steps or admin backend has overridden with a new or specific type from the choices (yes this step is problematic as it means running through all groups adding values to the type field so perhaps not a great idea).
A new param is provided to allow for a preset checked=checked to exist for a chosen type.
If this param wasn't set and if BP did set a type 'Normal' the default selection would be this value 'normal' set by BP automagically.
A new param is provided that allows for a descriptive string to be set on create steps for the type checkbox items:
'create_step_types_descrip' => 'Please select the group type from the available options or leave as the default selection set',
That last option is really not massively important but could be nice but how that is generated to screen needs thought so that appropriate markup would be generated so likely a step too much at this stage.
#21
@
8 years ago
Another brief point (although not sure if this is simply not having found the better or proper approach to follow) is:
It feels as though the group type selection should be on the initial screen along with the name and descrip and thus part of that initial group save, currently I'm needing try and remove plugin creation steps that might be easier if definitively I could check for group type from the very first stage or at the earliest possible moment in the steps.
#22
follow-up:
↓ 23
@
8 years ago
admin.patch
handles selecting multiple group types in the Groups admin dashboard, as well as showing multiple types in the Groups list table:
"Not available on the frontend" is a label that is shown when a group type is registered with 'show_in_create_screen' => false
.
At the moment, the patch doesn't cover the "Change group type to" bulk actions select box that @dcavins put together. Could use some feedback from him on how to proceed.
I'm going to put together another patch to handle the default checked types that @hnla and I are discussing since comment:19.
#23
in reply to:
↑ 22
@
8 years ago
Replying to r-a-y:
At the moment, the patch doesn't cover the "Change group type to" bulk actions select box that @dcavins put together. Could use some feedback from him on how to proceed.
We can improve this in the next release (similar changes would need to be made to the users list table bulk change tool, too). WP itself has a similar inconsistency between the user list table "Change role" select and the use of checkboxes to set multiple roles on the single user manage screen.
This ticket was mentioned in Slack in #buddypress by hnla. View the logs.
8 years ago
#27
follow-ups:
↓ 28
↓ 37
@
8 years ago
@hnla - I've added checked_by_default.patch
regarding comment:20. The new parameter is called 'create_screen_checked_by_default'
and defaults to false
if not set.
The new parameter name sounds a little wordy to me. Any suggestions for a better name?
Let me know what you think and whether or not this solves your problem.
@dcavins - I agree about moving the bulk change action stuff to the next release.
FWIW, WP 4.7 is going to make things easier to set custom bulk actions:
https://make.wordpress.org/core/2016/10/04/custom-bulk-actions/
Perhaps we can make this a 4.7-only feature?
#28
in reply to:
↑ 27
@
8 years ago
Replying to r-a-y:
@hnla - I've added
checked_by_default.patch
regarding comment:20. The new parameter is called'create_screen_checked_by_default'
and defaults tofalse
if not set.
The new parameter name sounds a little wordy to me. Any suggestions for a better name?
Let me know what you think and whether or not this solves your problem.
Think it's going to be wordy regardless and I don't see a problem with your choice as for alternative suggestions?
'create_screen_is_type_default'
'create_steps_type_selected'
'create_steps_type_checked'
'create_steps_type_default'
Could ring the changes ad nausea, think it's just a necessarily wordy key.
#29
@
8 years ago
This maybe not right time to discuss this or simply missed some salient point in terms of a use cases but:
you can now set multiple group types for that group.
I get that 'types' is simply a taxonomy' in same way those are used in WP e.g. 'category'; but what is the use case for being able or needing to set multiple group types to one single group instance? If we say that a group type has a series of distinct properties as set or registered are we then not going to run into issues where we 'use' these types or do we view this simply as the developers problem to deal with and manage add or don't add multiple taxonomies as you would with a post :)
#30
@
8 years ago
Let me know what you think and whether or not this solves your problem.
It tests fine, as to solving problem I think it is a smart option to be able to have set in advance and certainly prevents accidental type selection, so all round a 'good thing'.
The concerns with pre-existing groups not having a 'type' and columns in admin remaining blank are maybe unfounded, it feels wrong but then nothing will break as a result, there may be use cases where it's best that all groups do have a type though but I can't see that for sure at this moment. and we would have to run some form of update over all groups setting to a 'default'.
My pressing concern is that I do think the creation step initial screen must house the Name, Descrip, and type as the three fundamental pieces of info before we create a group_id then checks on the other steps can be made to bail out of custom group steps more easily or adjust subsequent step screens.
#31
@
8 years ago
@r-a-y updating templates I noticed that we're wrapping the bp_get_current_group_directory_type_message()
markup group type slug in a pair of <strong>
elements, if this is a typographical convention i.e we want it emboldened type then I would rather <b>
is used and actually with html5 the notion of the older but valid elements i,b are repurposed with new semantic meaning, although strong is too it still carries a notion of emphasis and importance when this is just a sentence read with flat emphasis. b is allowed to be styled or styled other than as 'bold type' so adding a class allows us to establish how we treat an element with a class as given ( for the moment it would and does remain 'bold' )
Patch gives example.
Of course conversely we simply wrap in span with or without a class and leave to pure styling with no explicit semantics.
#32
@
8 years ago
@r-a-y same goes for bp_group_type_list The <strong>
element might be better served and the cdata as something like a dl list as we're possibly running a few items in a list.
<dl> <dt>Group Types:</dt> <dd>Group1</dd> <dd>group2</dd> </dl>
They can then be run inline as they render now or run as default dl list styling
#33
@
8 years ago
@hnla - Thanks for the feedback.
My pressing concern is that I do think the creation step initial screen must house the Name, Descrip, and type as the three fundamental pieces of info before we create a group_id then checks on the other steps can be made to bail out of custom group steps more easily or adjust subsequent step screens.
I would consider the group privacy option more pressing than the group type. Group privacy is also listed on the second step as well.
Would welcome other feedback here, but I think we need another cycle to re-evaluate all the other group options during the group creation phase.
I noticed that we're wrapping the bp_get_current_group_directory_type_message() markup group type slug in a pair of <strong> elements
For bp_get_current_group_directory_type_message()
, current markup was lifted from the member type directory's message:
https://buddypress.trac.wordpress.org/browser/tags/2.6.2/src/bp-members/bp-members-template.php?marks=1969#L1959
So if we change this here, we'd need to change it for member types as well.
bp_group_type_list The <strong> element might be better served and the cdata as something like a dl list as we're possibly running a few items in a list.
The W3C says the following about lists and groups of links here:
The objective of this technique is to create lists of related items using list elements appropriate for their purposes. The ol element is used when the list is ordered and the ul element is used when the list is unordered. Definition lists (dl) are used to group terms with their definitions. Although the use of this markup can make lists more readable, not all lists need markup. For instance, sentences that contain comma-separated lists may not need list markup.
I think the last block applies to us here.
#35
in reply to:
↑ 34
@
8 years ago
Replying to hnla:
ok :)
Actually not ok :) I don't really care what legacy templates do any longer they are dead to me however I do want and expect to be able to make a choice on the newer templates and despite the W3C( who haven't always got things spot on ) ) I probably would favour listing but more to the point to get what I may judge to be right I don't want to be doing silly hoop jumping exercises like having to filter functions just to change something so trivial.
Part of what I wanted to progress albeit slowly was this idea of decoupling core from markup so either a function accepts params to set what markup used or core simply passes over the data and the templates generate the markup; I have to see what works best for the nuoveau templates in this and probably like minded template tags in core so not suggesting anything for the moment but will try and patch a solution for 2.8.
#36
follow-up:
↓ 39
@
8 years ago
I don't really care what legacy templates do any longer they are dead to me
Are you referring to the group type directory message or the group type list function? I don't mind if we make the change to the directory type message, although the change is quite trivial.
FWIW, <strong>
vs. <b>
- https://developer.mozilla.org/en-US/docs/Web/HTML/Element/strong#Bold_vs._Strong
strong is a logical state, and bold is a physical state. Logical states separate presentation from the content, and by doing so allow for it to be expressed in many different ways. Perhaps instead of rendering some text as bold you want to render it red, or a different size, or underlined, or whatever. It makes more sense to change the presentational properties of strong than it does bold. This is because bold is a physical state; there is no separation of presentation and content, and making bold do anything other than bold text would be confusing and illogical.
<strong>
sounds like the correct use here, but I could be wrong.
On the other hand, bp_group_type_list()
is new, so we can make whatever changes that we need to. Right now, the function was created to output a comma-delimited list (I guess the function name isn't named appropriately :) ).
List items are not really appropriate for comma-delimited lists unless you want to use CSS :before
and :after
declarations, which complicates things.
I guess it all depends on what you want to do with the function. If you plan on using this function for list items, then the function needs to be tweaked to accommodate it.
#37
in reply to:
↑ 27
@
8 years ago
Replying to r-a-y:
@hnla - I've added
checked_by_default.patch
regarding comment:20. The new parameter is called'create_screen_checked_by_default'
and defaults tofalse
if not set.
The new parameter name sounds a little wordy to me. Any suggestions for a better name?
I think the parameter name is OK as create_screen_checked_by_default
. You could shorten it to create_screen_checked
and the meaning would still be pretty clear.
Re bulk changes, I'm hoping to add a multiple edit interface to the groups list like WP uses on the posts list, where you choose several items then click edit and get the quick attribute editor. Would be good for types, status, etc. Could also be used to add members to multiple groups.
#39
in reply to:
↑ 36
@
8 years ago
Replying to r-a-y:
I don't really care what legacy templates do any longer they are dead to me
Are you referring to the group type directory message or the group type list function? I don't mind if we make the change to the directory type message, although the change is quite trivial.
FWIW,
<strong>
vs.<b>
- https://developer.mozilla.org/en-US/docs/Web/HTML/Element/strong#Bold_vs._Strong
strong is a logical state, and bold is a physical state.
Logical states separate presentation from the content, and by doing so allow for it to be
expressed in many different ways. Perhaps instead of rendering some text as bold you want
to render it red, or a different size, or underlined, or whatever.
It makes more sense to change the presentational properties of strong than it does bold.
This is because bold is a physical state; there is no separation of presentation and content,
and making bold do anything other than bold text would be confusing and illogical.
}}}
<strong>
sounds like the correct use here, but I could be wrong.
No matter @r-a-y I'll work out what we need by way of ease of access to these elements so we can change as see fit, I'll try and make a note on Nouveau tickets about it.
fwiw I think that Mozilla ref sounds like gibberish tbh I would not say strong is correct nor do I understand what on earth they are on about but in html5 they attempted to redefine semantics for all these elements so to describe b
as physical is not strictly correct, I do love Mozilla & their MDN site but on this I think they need to review their advice :)
Semantics and markup discussions finished please do what you think best I'm just confusing the issue get things in and we'll worry about this sort of detail another time.
#40
@
8 years ago
- Resolution set to fixed
- Status changed from new to closed
Thanks for hashing this out with me, @hnla. I'm all for semantic markup as well :)
I think all the main tidbits are in. Going to close this one!
Group Types codex article has been updated as well:
https://codex.buddypress.org/developer/group-types/
Registering directory support for Group Types is similar to Member Types:
function my_bp_custom_group_types() { bp_groups_register_group_type( 'team', array( 'labels' => array( 'name' => 'Teams', 'singular_name' => 'Team' ), // New for 2.7 - Change 'teams' to whatever slug you want your group type directory to have. 'has_directory' => 'teams' ) ); } add_action( 'bp_groups_register_group_types', 'my_bp_custom_group_types' );