-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Remove non-indexable logged-out links #5713
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
<%= javascript_pack_tag "profileDropdown", defer: true %> | ||
<a href="/report-abuse?url=https://dev.to/<%= @user.username %>">Report Abuse</a> | ||
<% if user_signed_in? %> | ||
<%= javascript_pack_tag "profileDropdown", defer: true %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
less JS for non logged in users! yay!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-approving this.
+1 on doing one thing at a time :)
What type of PR is this? (check all applicable)
Description
I saw in our search console dashboard that Google is crawling some user-only pages because we link to them in non-logged-in views. There is still a layer of authentication, so it's not a problem other than likely waisting the crawler's time (which could hurt our SEO).
This feature adds a check for
user_signed_in?
where functionality is only needed for signed-in users and the link does not need to be crawled by search engines.Also addednofollow
on some links that should still exist for non-logged-in users but are not links to indexable content per se.I removed the "nofollow links" part of this PR because I had hesitations about the exact right choice. Everything else should be a no-brainer.