Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced post and reply menu item text with icons and tooltips. Removed Spam menu item. #79

Closed
wants to merge 2 commits into from

Conversation

DavidScottBernstein
Copy link
Collaborator

Fixed #53 replaced post and reply item menu text with icons, removed Spam item, added tooltips for items. (Branch David20)

…ed Spam menu item.

Fixed #53 replaced post and reply item menu text with icons, removed Spam item, added tooltips for items.
@DavidScottBernstein
Copy link
Collaborator Author

@rgilman, I changed the post action menu items from text to icons. I thought this was going to take me weeks but your approach of using the background image for the icon works great. Also, thank you for the huge help with hooks, they work great. I removed the Spam menu item and added tooltips for all menu items using the hooks approach. Of course, the icons themselves are just placeholders for when we can get custom icons made, which will look much nicer.

@DavidScottBernstein
Copy link
Collaborator Author

@iangilman, @rgilman's idea of using the background image of menu items to display icons works great, I changed the Actions and Reply menus in posts, also added tooltips and removed Spam link using hooks. I just rebuilt everything from a new master to avoid merge conflicts.

@rgilman
Copy link
Member

rgilman commented Oct 30, 2018

@DavidScottBernstein, I've discovered another way to remove the Spam item. Not necessarily better but I wanted to share it. I got the clues I needed from the second post in customizing bbpress admin links. Here's the code that works for me locally (in bfc-functions.php):

//change topic admin links displayed
function bfc_change_topic_admin_links ($r) {
	$r['links'] = apply_filters( 'bfc_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 ),  
		'reply' => bbp_get_topic_reply_link( $r )
		), $r['id'] );
	return $r['links'] ;
	}
add_filter ('bbp_topic_admin_links', 'bfc_change_topic_admin_links' ) ;

//change admin links displayed
function bfc_change_admin_links ($r) {
	$r['links'] = apply_filters( 'bfc_reply_admin_links', array(
		'edit'  => bbp_get_reply_edit_link ( $r ),
		'move'  => bbp_get_reply_move_link ( $r ),
		'split' => bbp_get_topic_split_link( $r ),
		'trash' => bbp_get_reply_trash_link( $r ),
		'reply' => bbp_get_reply_to_link   ( $r )
		), $r['id'] );
	return $r['links'] ;
	}
add_filter ('bbp_reply_admin_links', 'bfc_change_admin_links' ) ;

These look just like the code starting at lines 1841 and 2418 in plugins/bbpress/includes/topics/template.php except they don't include 'spam'. The advantage of this approach is that it removes everything associated with the spam link.

BTW, I gather that bfc_reply_admin_linksis a kind of dummy hook that needs to be different from bbp_reply_admin_links likely to avoid some kind of circular reference.

@rgilman
Copy link
Member

rgilman commented Oct 30, 2018

Another thought: Now that we have the filter functions going, I'm wondering if it might be better form to replace the words with the icons via <img> tags within the <a> tags – rather than doing background images and setting the text color to transparent. This might enable spacings to be based on the icon size rather than the word size.

@iangilman
Copy link
Member

The changes look good to me, but @rgilman's thought about using the icons in the tags seems interesting. What do you think, @DavidScottBernstein? I guess I could go either way on it, but it's worth considering.

@rgilman The alternate way to remove the spam link is good to know about. Any thoughts on how you would add the title while in there? I still don't know enough about WordPress to evaluate which approach would be better.

@rgilman
Copy link
Member

rgilman commented Oct 31, 2018

@iangilman. I'm not sure what you mean by "add the title while in there." (Edit: duh, now I see. "title" is the correct name for the metadata that becomes the tooltip.) If you mean how to add the tooltip, I don't think it can be done directly via what I have with bfc_change_topic_admin_links and bfc_change_admin_links. It could be done with rewriting the various functions like bbp_get_topic_edit_link but it's likely better to do that task via the filter function that David has already created.

We will need to use something like bfc_change_topic_admin_links and bfc_change_admin_links to insert like buttons. This feels like the cleaner WP approach for removing Spam as well since it removes the whole Spam object, not just it's text.

@rgilman
Copy link
Member

rgilman commented Oct 31, 2018

To be specific, here's the pattern for how I see replacing the words with icons:

$array = str_replace('>Edit<', 'title="Edit this item"><img src="/wp-content/themes/bfcom/assets/images/edit.svg"><', $array);

DavidScottBernstein added a commit that referenced this pull request Oct 31, 2018
…moved Spam link

Re-Fixed #79 using @rgilman's second approach. This branch, David21, replaces branch David20.
@iangilman iangilman closed this in #81 Nov 1, 2018
@iangilman iangilman deleted the David20 branch November 1, 2018 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants