gethinode / hinode

A clean documentation and blog theme for your Hugo site based on Bootstrap 5
https://gethinode.com
MIT License
146 stars 51 forks source link

Add option for adding a target to a button #326

Closed myrthos closed 1 year ago

myrthos commented 1 year ago

Is your feature request related to a problem? Please describe. A button can have a link, but my preference for links that make people leave the site, is to have them open in a separate tab. I know that other people think differently about that, which is why I think it should be optional.

Describe the solution you'd like Add to the button component the optional argument target, which is only used when there is also a href. This allows calling the button component with target = ”_blank”, or anything else as a target and the button component adds that target to the link tag.

Additional context I implemented this myself, when I noticed that the social links at the top of a page opened on the same page. I just hacked the extra target call into the call to the button component as it works for me. I don't need it, but perhaps it would give some flexibility to other users if they could parametrise in the TOML file, if they want to have these social links open in a separate page or not (although that could be considered a separate feature request I suppose).

As I mentioned, I already have this working for me, but I think it might be a nice addition for some other users. Although if you do implement it, I will undo my changes and use what you made, to make updating future changes to the template easier.

markdumay commented 1 year ago

Yes, it makes sense to make this configurable. I sporadically open links in new tabs on gethinode.com too, mostly in the footer links. I think we should have a global setting and then have the option to override this setting on individual buttons. The default setting would be false, meaning all links (internal and external) stay in the same tab.

myrthos commented 1 year ago

That would work very well. There are other options than "_blank" for the target, but I don't think they are really useful anymore, as the effects these options have can also be achieved, and probably better, with CSS styling.

markdumay commented 1 year ago

I found a more generic approach applicable to all links: https://discourse.gohugo.io/t/handle-external-links-with-render-hook-templates/22452

Downside with this approach is that it's all or nothing. Then again, from a UI perspective it makes sense to do this consistently.

The code could also optionally inject an icon for all regular external links (could be another option independent of opening links in new tabs).

myrthos commented 1 year ago

That is indeed a much better solution. I also like Bep's simple solution. And having an optional icon identifying it as an external link, is a very nice addition.

markdumay commented 1 year ago

As it turns out, the render-link hook contains a bug unfortunately: https://github.com/gohugoio/hugo/issues/7416. The short explanation: Hugo does not pre-render .Destination when using the {{< delimiter. It does work with the {{% delimiter.

I often use the pattern [External link]({{< param "links.named_link" >}}) in the Hinode documentation. I imagine it makes more sense to introduce a new shortcode with underlying partial, instead of forcing the use of {{%. I'm thinking to simply name it link with several (partially optional) arguments.

myrthos commented 1 year ago

I haven't used that up to now, I just hardcode the link. I can see the advantage of your approach when the external link is used multiple times though and having a link short code with parameters would be a good idea.

My current solution is that I am only doing this for the sharing buttons and I added the target to the sharing parameters in params.toml. I had to change the sharing and button partials anyway as I have added sharing to Mastodon and that required a few more changes, including JavaScript code, which also needs to be parameterized in params.toml. So, I could imagine that the new link partial would be used for the link of the sharing buttons. However, as sharing to Mastodon requires dynamic interaction with the user to acquire the user's Mastodon instance, to know where to send the sharing information to, I am not using the href, but need an id in the link to have a piece of JavaScript trigger on that when it is clicked. Other parameters I am using are the text to show to the user (just in case I ever decide to go multilingual) and the sharing link. A link for the Mastodon button would look like <a href="#" id="mastodon" data-m-prompt="please enter your mastodon instance to share to:" data-m-src ="share?text=TheTitle&url=LinkToThePage"..... This required changing the sharing and button partials.

For clarity, I am not saying that the new link shortcode and partial need to accommodate for the above. I can make the necessary changes again if it doesn't. I just mentioned it to explain that for me it has become a bit more than just the target. Maybe the option to add a set of key-value pairs that are added to the 'a' tag would make it really flexible :)

markdumay commented 1 year ago

I've added a new shortcode called link to generate regular and managed links. It supports two arguments: cue and tab. I've updated the button and social sharing config too.

I'll give your suggestion to support key-value pairs some more thought. The current code is not fully DRY yet and could use some optimization.

myrthos commented 1 year ago

That looks really nice and is a great addition.

As to the key-value pairs, having thought about it, it is a very special case, where clicking on a link is to trigger something in JavaScript and transfer some piece of information. Although it would be nice to have the option, I think it also increases complexity, where there aren't that many realistic use cases.

markdumay commented 1 year ago

Glad you like it! I have added basic key-value support to the button partial. I added web sharing to the sharing partial, which needed additional data attributes as input to a custom JavaScript. If you're interested, perhaps the same approach would work for the custom Mastodon login too? Below is the relevant code snippet.

{{ if .Site.Params.sharing.webshare }}
    {{ $attr := dict "data-sharing-title" .Title "data-sharing-description" .Description "data-sharing-url" .Permalink }}
    {{ partial "assets/button.html" (dict "href" "#!" "icon" "fas share-nodes fa-fw" "id" "btn-webshare" "class" "btn-social p-0" "attributes" $attr )}}
{{- end -}}

I'm considering to use the same approach (custom key-value pairs) to increase reuse throughout the codebase. I duplicated the link behavior at several places and it would be beneficial to have in one place only (the button partial).

myrthos commented 1 year ago

I have modified the Mastodon changes to use the new button partial and the key-value pairs, so that I no longer have to change the button partial anymore. I still have to change the sharing partial to facilitate the Mastodon changes, which makes those changes very Mastodon specific.
Still, it is one partial less that I need to change, so that is good :smile:

myrthos commented 1 year ago

I also tried the link shortcode and it works very well. it looks even better than your examples, where the icon indicating it is an external ink is much closer to the text than in reality. Reality looks better.
A very good addition :+1:

markdumay commented 1 year ago

Nice! I made a last-minute change that is not fully incorporated into the docs site yet. I noticed the issue with spacing and added an ‘nbsp;’ to fix it. I suspect the go template “eats” too much whitespace due to the ‘{{-‘ symbols. It’s a small nuisance when using go templates.