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

External links should always open in new tab/window, internal links should not #202

Closed
raamdev opened this issue Jan 11, 2016 · 10 comments
Closed

Comments

@raamdev
Copy link
Contributor

@raamdev raamdev commented Jan 11, 2016

@RealDavidoff writes in #199 (comment)...

  1. The "Subscribe without commenting" and "Manage my subscriptions" links open in a new window/tab. This doesn't make sense, and is needless: If user doesn't comment then he doesn't click the send-comment button at the end of the article that he has just finished reading. So keep him in the same window/tab.
    Most sensible is: Always open a NEW window/tab for EXTERNAL links, and whenever the user wants/needs to continue reading (or filling in data) on the current page but shall be able to see additional information on another internal page. Links before the end of an article likely meet this criterion (if the article is good enough!), but if the linked NEW article is standalone (not just helping to understand the current page) then keeping the reader in the SAME window/tab is least annoying. - And obviously, links that open a new window/tab should be marked as such. This is brilliantly done in s2m, but not in cm.
@raamdev raamdev added this to the Next Release milestone Jan 11, 2016
@raamdev raamdev changed the title External links should always open in new tab/window External links should always open in new tab/window, internal links should not Jan 11, 2016
@jaswrks
Copy link

@jaswrks jaswrks commented Jan 25, 2016

@kristineds This looks like an issue that you could outline.

Search the template files for target="_blank" and try to locate those links which currently open in a new window. We should remove target="_blank". I agree with that.

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Jan 25, 2016

try to locate those links which currently open in a new window.

And vice versa, i.e., all links should be reviewed and any links that are external links should have target="_blank" added to them, so that they do open in a new tab/window.

@kristineds
Copy link
Contributor

@kristineds kristineds commented Jan 28, 2016

@jaswsinc @raamdev Waiting for feedback on this outline. :) cc @renzms


Next Actions


  • Repeat in the Lite version of Comment Mail.
@jaswrks
Copy link

@jaswrks jaswrks commented Jan 28, 2016

@kristineds Looks good to me. All except for https://github.com/websharks/comment-mail-pro/blob/b5a67f413afabf0ce9ee91cd45288a7e81813047/comment-mail-pro/plugin.inc.php#L1738

That is an external link, because it moves you from the installation site to comment-mail.com.

@kristineds
Copy link
Contributor

@kristineds kristineds commented Jan 31, 2016

@renzms
Copy link
Contributor

@renzms renzms commented Feb 3, 2016

@raamdev @jaswsinc Ready for review. Thanks! cc @kristineds

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Feb 3, 2016

@kristineds @renzms Nice work collaborating on this one! Reviewed and merged.

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Feb 3, 2016

Next Lite Release Changelog:

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Feb 3, 2016

Next Pro Release Changelog:

@raamdev raamdev closed this Feb 3, 2016
@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Feb 13, 2016

Comment Mail v160213 has been released and includes changes from this GitHub Issue. See the release announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#202).

@wpsharks wpsharks locked and limited conversation to collaborators Feb 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants