Extend the built-in shortcodes

[digitalcraftsman]

See #1576.

[CLAassistant]

All committers have accepted the CLA.

[digitalcraftsman]

Should I squash everything into a single commit?

[bep]

No. These are fine as separate commits.

[moorereason]

I've looked at the youtube shortcode, and I'm not a big fan of it. style tags? Shame.

I'm working on an enhancement.

[moorereason]

I've submitted PR #1597 to add a shortcode .ParamsType function. With that commit, I would propose updating these defaults to be more concise and flexible. Here's how I would define the Youtube shortcode (line breaks for readability):

{{ if eq .ParamsType "position" }}
<div class="{{ if len .Params | eq 2 }}{{ .Get 1 }}{{ else }}youtube-container{{ end }}">
  <iframe src="https://www.youtube.com/embed/{{ .Get 0 }}" allowfullscreen frameborder="0"></iframe>
 </div>
{{ else }}
<div class="{{ if .Get "class" }}{{ .Get "class" }}{{ else }}youtube-container{{ end }}">
  <iframe src="https://www.youtube.com/embed/{{ .Get "id" }}" allowfullscreen frameborder="0"></iframe>
 </div>
{{ end }}

Example references:

{{< youtube f8TkUMJtK5k >}}
{{< youtube f8TkUMJtK5k flex-video >}}
{{< youtube id="f8TkUMJtK5k" class="flex-video" }}

I like this approach because the user can control everything via SASS/CSS:

.flex-video {
  width: 560px;
  height: 315px;

  iframe {
    width: 100%;
    height: 100%;
  }
}

[bep]

I had a look at this, too -- and having only inline style tags is not ideal. But it is the only way for a shortcode like this to be truly standalone.

But they should be fallback options, somehow.

[digitalcraftsman]

@moorereason I know that inline style tags are an ugly way to add CSS code. But this is the only (as far as I know) for a shortcode to stay independent and to work out of the box. It was my intention that they take the full width of the layout and are responsive. With defaults, the user doesn't need to add any styling to achieve this.

I like your approach to give users more flexibility to style the elements of the shortcode.

[moorereason]

I've updated PR #1597 to use a simple boolean, as @bep suggested. Since it doesn't sound like I'm going to win the style argument, here's an updated example:

{{ if .IsNamedParams }}
<div {{ if .Get "class" }}class="{{ .Get "class" }}"{{ else }}style="position: relative; padding-bottom: 56.25%; padding-top: 30px; height: 0; overflow: hidden;"{{ end }}>
  <iframe src="https://www.youtube.com/embed/{{ .Get "id" }}" {{ if not (.Get "class") }}style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;" {{ end }}allowfullscreen frameborder="0"></iframe>
 </div>
{{ else }}
<div {{ if len .Params | eq 2 }}class="{{ .Get 1 }}"{{ else }}style="position: relative; padding-bottom: 56.25%; padding-top: 30px; height: 0; overflow: hidden;"{{ end }}>
  <iframe src="https://www.youtube.com/embed/{{ .Get 0 }}" {{ if len .Params | eq 1 }}style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;" {{ end }}allowfullscreen frameborder="0"></iframe>
 </div>
{{ end }}

If you pass in a class name, we use that instead of the built-in style tags. The logic on the iframe is that if you pass in a class, it's only for the out div container, and I assume that if you're using CSS classes, then you know how to select the iframe inside your container div.

[bep]

That I like.

[digitalcraftsman]

I've added the IsNamedParams property from @moorereason (see #1597) in the shortcodes.

[bep]

Looks good to me. Any comments @moorereason ?

[moorereason]

Does it matter we use src="//hostname/..." on some links and src="https://hostname/..." on others. Why not remove the protocol from all links? Isn't that the proper way to include remote resources?

On the twitter shortcode, I'd just send the tweet ID. The username is unnecessary:

http://api.twitter.com/1/statuses/oembed.json?id={{ index .Params 0 }}

[digitalcraftsman]

I've made every shortcode protocol independent by just leaving the //, except in the Twitter shortcode. This shortcode make use of getJSON which needs a specific protocol. Without naming one the function isn't able to request the data and an error message is returned: unsupported protocol scheme "".

Now, the Twitter shortcode just requires the tweet's id.

[moorereason]

LGTM 👍

PS - @digitalcraftsman, I think you need to sign the new CLA. Hugo is now Apache 2.0 licensed! Yea!

[digitalcraftsman]

Done!

Nice to see that @spf13 was finally able to make the transition.

[bep]

Good work. Merged in 311307c

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.