#6513 closed defect (bug) (fixed)
New Filter for Groups Widgets and Members Widgets
Reported by: | dunhakdis | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 2.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Core | Keywords: | good-first-bug has-patch |
Cc: |
Description
Hello!
I often find it difficult to hide the '|' horizontal line separator every time I develop a new theme or design the buddypress widget area for members and groups. It might be a good idea to add filters to those encoded separators.
Instead of directly encoding '|', why don't we provide filters like
<?php echo apply_filters('bp_members_widget_separator', '|'); ?>
for members and
<?php echo apply_filters('bp_groups_widget_separator', '|'); ?>
for groups.
So that I could add some function to the theme's functions.php to easily change those horizontal separators.
<?php // This will change the default widgets separator from '|' to '>>' (raquo) add_filter('bp_groups_widget_separator', 'theme_bp_widgets_separator'); add_filter('bp_members_widget_separator', 'theme_bp_widgets_separator'); function theme_bp_widgets_separator(){ return '»'; } ?>
Thanks!
-- Joseph
Attachments (2)
Change History (16)
#5
@
9 years ago
Yeah, let's definitely do something like this.
I think we'd probably fix this in another way (probably put the output lines into an array, then use array_join, sort of like how the page <title> is generated, but agree with you that this would be a good improvement.
I don't see the benefit of getting fancy when building the markup. Something like widgets.patch seems OK to me. (Though we should (a) run it through the filter just once and store in a variable $separator
, and (b) escape before echoing.)
This ticket was mentioned in Slack in #buddypress by espellcaste. View the logs.
9 years ago
#7
@
9 years ago
A few things to consider here.
While in principle the notion of providing access to modify these characters is undeniably a good thing we are leaping over some far more fundamental issues that should be addressed.
- The use of a series of inline anchors for what amounts to nav links.
- Outputting bare naked character data to an aggregating non semantic element, not a good thing, it's hard to target those characters and technically upsets formatting blocks.
Ideally we ought to update this and the other widgets that have these 'sort' links to use a construct that can better manage styling and provide some semantic context, classically a ul construct. To ensure we don't upset existing layouts we would add rulesets to manage this new markup to replicate the look we currently default to i.e. horizontal, with a separator that is now largely a decorative visual aid rather than something we need to add in order to vaguely comply with guidelines when links are used for navigation without any semantic expression of their purpose.
If not updating the links in this fashion then we need - at the least - to wrap the characters in span tags to provide formatting context, ease of styling, and ability to provide attr then we can/could provide aria scope 'role="separator"'
The solution proposed uses a filter to access the output, I'm not clear why we need to use this approach as it's somewhat technical (I don't propose it's not provided but it's additional to...) for general users and where we're talking about a widget which by definition passes args back so why are we not updating the widget control to include a text input or maybe safer a set select dropdown of available options for separator characters to use and which we would output to span markup.
We need now to be tackling aspects such as this as fully and finally as possible and I see this as part of the larger task I would like us to begin to tackle with BP templates in the next release/s.
#8
@
9 years ago
Thanks, hnla. This all sounds sensible. The markup for the widgets is some of the oldest in BuddyPress, and should be addressed in a holistic way at some point.
But let's not lose sight of the fact that all the reporter is asking for is a way to change the separator from a hard-coded |
to something else. It seems obvious to me that, whatever the long term goals for the widgets may be, we can definitely do something in the short term. If we think that a filter on a piece of hardcoded text (as we have now) will be maintainable, then let's do it. If you think it'd be better to move the character to a CSS :before { content }
rule, then let's do that. But let's please do something. Can you make a suggestion about what to do in the short term?
#9
@
9 years ago
But let's not lose sight of the fact that all the reporter is asking for is a way to change the separator
Absolutely, and believe me my remarks above were made with that in mind, but it's important to note we need to address deeper issues and to try and improve as much as logically possible in each iteration just in case it has to last for longer than we cared for it to.
So, and sorry if this is all longwided, we should for the moment add the filter, based on the fact that we can live with the filter regardless of what other changes may be effected.
As suggested the filter would be stored to a variable lets then output that variable to markup adding a class and aria role, so:
<span class="bp-separator" role="separator"><?php echo esc_html( $separator ); ?></span>
Thus we at least have our characters wrapped in a suitable element and not floating around haplessly, we have a styling hook and an aria control for good measure.
For clarity inlne anchors used in this manner essentially read as a sentence with hyperlinks and thus is why the instruction is made that a separator of some nature is provided in order that the links are broken, really the 'pipe' character is the correct option here for this. This instruction is not a decorative or visual cue though it's an assistive one.
If or when we markup these links in a ul construct the hard character is redundant as the ul construct conveys the semantic information naturally via it's 'li' elements then it's perfectly acceptable to use a separator but it now becomes purely a decorative item, then we should use CSS for it's presentational purpose to provide these either as border-left or as said using pseudo elements
#10
@
9 years ago
This all sounds good to me. Let's get a patch using the span
+ filter method described by hnla.
I think we'd probably fix this in another way (probably put the output lines into an array, then use
array_join
, sort of like how the page<title>
is generated, but agree with you that this would be a good improvement.