Skip to:
Content

bbPress.org

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#2090 closed defect (bug) (fixed)

New Filters for Topic/Reply Admin Links

Reported by: mordauk's profile mordauk Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 2.4 Priority: normal
Severity: normal Version: trunk
Component: API - Actions/Filters Keywords: has-patch
Cc:

Description

While working on a new bbPress [extension](https://twitter.com/pippinsplugins/status/277092946069831681) I found that I wanted to add new links to the Topic / Reply admin links.

The links are setup with the bbp_get_reply_admin_links() and bbp_get_topic_admin_links() functions, both of which send the final HTML through a filter before it is returned.

The problem with these two filters, is that anyone who wants to tie into them will have to do some hacky things to properly add a new admin link.

Let's assume I want to add a new "Add Note" link at the start or end of the admin links list. I would do something like this:

function sample_add_bbp_admin_link( $links, $args ) {
	
	$links .= '<a href="#">some link</a>';

	return $links;
}
add_filter( 'bbp_get_topic_admin_links', 'sample_add_bbp_admin_link', 10, 2 );

But this will result in this:

<span class="bbp-admin-links">
	<a href="forums/topic/i-found-a-bug/edit/">Edit</a> | <a href="/wpms/sub/forums/topic/i-found-a-bug/?action=bbp_toggle_topic_close&#038;topic_id=90&#038;_wpnonce=adbde4f67e">Close</a> | 
	<a href="/wpms/sub/forums/topic/i-found-a-bug/?action=bbp_toggle_topic_stick&#038;topic_id=90&#038;_wpnonce=40ae9c144b">Stick</a> (<a href="/wpms/sub/forums/topic/i-found-a-bug/?action=bbp_toggle_topic_stick&#038;topic_id=90&#038;super=1&#038;_wpnonce=40ae9c144b">to front</a>) | <a href="forums/topic/i-found-a-bug/edit/?action=merge">Merge</a> | 
	<a title="Move this item to the Trash" href="/wpms/sub/forums/topic/i-found-a-bug/?action=bbp_toggle_topic_trash&#038;sub_action=trash&#038;topic_id=90&#038;_wpnonce=62ad135be0">Trash</a> | 
	<a href="/wpms/sub/forums/topic/i-found-a-bug/?action=bbp_toggle_topic_spam&#038;topic_id=90&#038;_wpnonce=49d9a0e0c0">Spam</a>
</span>
<a href="#">some link</a>

Note that the new HTML is outside of the SPAN tag.

In order to properly place my link inside of the before/after elements, I would have to do some hacky stuff with str_replace().

I propose that a new filter gets added for each admin links function where the links array is setup, like so:

if ( empty( $r['links'] ) ) {
	$r['links'] = apply_filters( 'bbp_topic_admin_links', array(
		'edit'  => bbp_get_topic_edit_link ( $r ),
		'close' => bbp_get_topic_close_link( $r ),
		'stick' => bbp_get_topic_stick_link( $r ),
		'merge' => bbp_get_topic_merge_link( $r ),
		'trash' => bbp_get_topic_trash_link( $r ),
		'spam'  => bbp_get_topic_spam_link ( $r ),
	), $r['id'] );
}

This would make it much easier and practical to add new links. Instead of using str_replace (or some other hacky method), we could just do this:

function sample_add_bbp_admin_link( $links, $topic_id ) {
	
	$links['add_note'] = '<a href="' . some_function( $topic_id ) . '">some link</a>';

	return $links;
}
add_filter( 'bbp_topic_admin_links', 'sample_add_bbp_admin_link', 10, 2 );

Attachments (1)

bp_admin_links.patch (1.4 KB) - added by mordauk 12 years ago.
Adds filters to the admin links array

Download all attachments as: .zip

Change History (13)

@mordauk
12 years ago

Adds filters to the admin links array

#1 @mordauk
12 years ago

  • Component changed from General to Actions/Filters
  • Keywords has-patch added
  • Version changed from 2.1 to trunk

#2 @johnjamesjacoby
12 years ago

  • Milestone changed from Awaiting Review to 2.3

#3 @johnjamesjacoby
12 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [4562]) Topic/Reply admin links:

  • Add filters to links arrays.
  • Props mordauk.
  • Fixes #2090.

#4 @aaclayton
11 years ago

  • Milestone changed from 2.3 to 2.3.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm loving the new filters on reply and topic admin links, with one exception. All of the admin buttons allow you to pass the button creation function an argument which allows the customization of the inner HTML. This is great because I can do cool things like put Font Awesome (or other icon fonts) inside the button link. For example:

bbp_get_topic_edit_link  ( array( 'edit_text' => '<i class="icon-edit"></i>Edit' ) );

Unfortunately, bbp_get_topic_trash_link() runs esc_html() on this argument which escapes my icon. I suggest that esc_html() be removed from the trash link function as a developer supplied argument should hardly be treated as "untrusted" by its own theme.

The same problem affects a few other admin links as well: bbp_get_reply_trash_link(), bbp_get_topic_split_link(), and bbp_get_reply_move_link(). It would really be great to get these escapes removed. Considering it would be a super quick fix, I hope this could maybe be squeezed in for 2.4.

Last edited 11 years ago by aaclayton (previous) (diff)

#5 @aaclayton
11 years ago

On a very related note... it would be really nice to get rid of the mandatory parenthesis that wrap the super sticky button in bbp_get_topic_stick_link(). They aren't even inside an HTML element, so there's not even a easy way to hide them using CSS. At the moment I make the font color of the parent container transparent, but it would be nice if I had the option not to output them in the first place.

Also, an arg in the parent bbp_reply_admin_links() and bbp_reply_topic_links() call to apply a css class to the buttons themselves would be very valuable. I know a lot of theme designers (myself included) use classes like a.button, and being able to pass those styles onto admin links would offer nice flexibility.

Last edited 11 years ago by aaclayton (previous) (diff)

#6 @johnjamesjacoby
11 years ago

  • Milestone changed from 2.3.3 to 2.4

Moving to 2.4 to shift around some of the escaping for you.

#7 @johnjamesjacoby
11 years ago

  • Owner set to johnjamesjacoby
  • Resolution set to fixed
  • Status changed from reopened to closed

In 5069:

Shift output escaping around in admin links to allow for filtering and passing additional HTML into them. Hat tip mourdak. Fixes #2090.

#8 @johnjamesjacoby
11 years ago

In 5070:

Add missing output escaping in admin edit links, and add unique classes to all admin links. See #2090.

#9 @johnjamesjacoby
11 years ago

In 5071:

Rename link class to match function and output, and remove some duplicate escapings. See #2090.

#10 @aaclayton
11 years ago

Awesome guys, thank you for implementing this!

Minor typo in changeset 5070 - /includes/replies/template.php line 1819 calls "est_html"

I appreciate the enhancements!

#11 @mordauk
11 years ago

Thanks for updating it, JJJ.

#12 @johnjamesjacoby
11 years ago

In 5072:

Fix typo. See #2090.

Note: See TracTickets for help on using tickets.