elementary / blog-template

Template for the Jekyll-powered elementary blog
https://blog.elementary.io
GNU General Public License v3.0
32 stars 20 forks source link

Move external link handling from JS to build time #3

Closed cassidyjames closed 5 years ago

cassidyjames commented 5 years ago

From https://github.com/elementary/blog/issues/48. Context:

@cassidyjames:

I don't think there's anything that can be done in Jekyll to enforce this for Markdown links (I'll double-check), but at the very least we can do that super light JS that adds target="_blank" to links that link offsite.

So with the Markdown parser you can do [Example](https://example.com){:target="_blank"}, but that's annoying to do for every link. Instead, I'm adding some lightweight JS that adds target="_blank" to any external links that don't already have a target attribute. This means if you want to override the behavior, you can just add [Example](https://example.com){:target="_self"}, but [Example](https://example.com) will automatically open in a new tab.

@btkostner:

Please don't use JavaScript for this. Try doing some build time plugin like https://keith-mifsud.me/projects/jekyll-target-blank

@cassidyjames:

@btkostner I tried exactly that first, and it didn't work. I believe due to the --safe environment used by GitHub Pages. But if you can get it working, be my guest. :wink:

Gonna merge this for now since we have a lot of external links in blog posts. I would prefer doing it as build time as well, and will open a new issue to fix that.

I do agree we should move it to build-time ideally if we can get that working.

dar5hak commented 5 years ago

This technique might be useful. It's a workaround, but it's compatible with GitHub Pages.

The prerequisite is that all internal links should be relative, that is, not start with the string http. Looking at the rendered HTML of a few blog posts tells me that is indeed the case, but I also found this:

As I’ve <a href="{ site.baseurl }/the-need-for-a-freedesktop-dark-style-preference/">written before</a>, practically all major platforms and browsers…</p>

That shouldn't be a problem in theory, but I want to test it out.

cassidyjames commented 5 years ago

@dar5hak ah that could work! Here’s a sample post that has links to other blog posts if you want to test with real data: https://raw.githubusercontent.com/elementary/blog/master/_posts/2019-09-19-updates-for-august-2019.md?token=AAEVGYBNZBGXREBZMD2RAV25TYBUE

dar5hak commented 5 years ago

Impeccable timing, dude. I was testing things on my fork and this is exactly what I needed. :beers:

dar5hak commented 5 years ago

Another prerequisite was that href must be the first attribute in an <a> tag, or it won't be replaced. However, this only needed a change in license.html. Everything else was already handled beforehand.

Check out my changes and the published version.

I've tested everything I could think of (including the "Newer"/"Older" links which aren't published).

Update: On a second thought, license.html probably doesn't need to be changed either. Reverted.

dar5hak commented 5 years ago

I also didn't publish the sample post you shared with me because I didn't want to pollute the PR.

If everything looks good, I'll make a PR to upstream.