chipzoller / hugo-clarity

A theme for Hugo based on VMware Clarity
Other
582 stars 267 forks source link

Correcting makeExternalLinks() to allow 'rel' attribute to be overridden #205

Closed rootwork closed 3 years ago

rootwork commented 3 years ago

Short description

makeExternalLinks() has a small mistake that doesn't allow the rel attribute in an external link to be overridden.

Specifically, I wanted to do this in the follow.html layout partial to enable indieweb RelMeAuth.

If you look at the file changes, you'll probably understand it right away, without the need for the long description to follow...

Long description with screenshots

When including an external link in a layout file, you might want to override the default attributes set by the makeExternalLinks() function. For instance, in follow.html, this is used to construct the links:

  <a href="{{ $url }}">
    {{ partial "sprite" (dict "icon" $label) }}
  </a>

By default, that will result in a link like this:

current-default-and-overridden

The rel and target attributes come from the makeExternalLinks() function.

If you set these attributes in the layout file, it should override what makeExternalLinks() is doing. For instance, if you set the target with <a href="{{ $url }}" target="foo">, you'll get <a href="..." target="foo">.

But because of the mistake in makeExternalLinks(), you can't do that with rel; it will always be set to noopener.

With the fix, the default output is the same:

fixed-default

But if you override rel in a layout file, e.g. with <a href="{{ $url }}" rel="me">, you get the overridden value in the output as well:

fixed-overridden

Checklist

Ensure you have checked off the following before submitting your PR.

rootwork commented 3 years ago

FYI @onweru it looks like this also exists in your other themes, but I haven't used them so I can't confirm testing. For your convenience:

https://github.com/onweru/hugo-swift-theme/blob/3a49e560089cd3692fd4fd4e84c71d55687b20d4/assets/js/index.js#L57 https://github.com/onweru/compose/blob/3f2ccb9b9acb30834cb7759d14b57f884ea03981/assets/js/index.js#L168 https://github.com/onweru/newsroom/blob/59ae2f193880e25ad2610cd55185ea82fef92a34/assets/js/index.js#L188

onweru commented 3 years ago

FYI @onweru it looks like this also exists in your other themes, but I haven't used them so I can't confirm testing. For your convenience:

https://github.com/onweru/hugo-swift-theme/blob/3a49e560089cd3692fd4fd4e84c71d55687b20d4/assets/js/index.js#L57 https://github.com/onweru/compose/blob/3f2ccb9b9acb30834cb7759d14b57f884ea03981/assets/js/index.js#L168 https://github.com/onweru/newsroom/blob/59ae2f193880e25ad2610cd55185ea82fef92a34/assets/js/index.js#L188

@rootwork, yes this is true. I will fix them when I can