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

Related Posts: add new filter to allow disabling nofollow #2299

Merged
merged 2 commits into from Aug 3, 2015

Conversation

@jeherve
Copy link
Member

commented Jun 23, 2015

Fixes #2233
cc @xyu

If this filter is merged, it would then become possible to disable nofollow on Related Posts' links with the following snippet:

function jeherve_custom_rp_rel( $post_id ) {
    return '';
}
add_filter( 'jetpack_relatedposts_filter_post_link_rel', 'jeherve_custom_rp_rel', 20, 2 );
*
* @param bool true Should the link rel attribute for Related Posts' links be dislpayed? Default is yes (true).
*/
'rel' => apply_filters( 'jetpack_relatedposts_filter_post_link_rel_enabled', true ),

This comment has been minimized.

Copy link
@xyu

xyu Jun 23, 2015

Member

Why not just have rel be the actual rel value on the anchor and allow developers to filter on nofollow directly rather then have a bool flag add it in JS?

This comment has been minimized.

Copy link
@jeherve

jeherve Jun 23, 2015

Author Member

That was actually my first approach, but I changed my mind because I couldn't think of any other rel attributes people would want to use apart from nofollow or nothing?

This comment has been minimized.

Copy link
@xyu

xyu Jun 23, 2015

Member

I would also consider passing the $post_id as a third arg; we do it in a lot of other filters in the related posts module and would allow more complicated filtering based on what post is returned.

This comment has been minimized.

Copy link
@xyu

xyu Jun 23, 2015

Member

Re rel attributes , I can see some may want to prefetch related posts if they think there's a high chance that someone would want to click on a link:
http://www.w3schools.com/tags/att_a_rel.asp

This also plays into passing a post id as an optional argument. If I know this one post of mine is always super popular I might want to remove nofollow and add a prefetch to it every time that post gets related to something else.

This comment has been minimized.

Copy link
@jeherve

jeherve Jun 23, 2015

Author Member

Makes sense. I'll update my PR.

This comment has been minimized.

Copy link
@jeherve

jeherve Jun 23, 2015

Author Member

Done!

@xyu

This comment has been minimized.

Copy link
Member

commented Jun 23, 2015

LGTM 👍

samhotchkiss added a commit that referenced this pull request Aug 3, 2015

Merge pull request #2299 from Automattic/add/related-posts-nofollow-f…
…ilter-2233

Related Posts: add new filter to allow disabling nofollow

@samhotchkiss samhotchkiss merged commit 662c89d into master Aug 3, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@kraftbj kraftbj deleted the add/related-posts-nofollow-filter-2233 branch Aug 4, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.