tech-radar icon indicating copy to clipboard operation
tech-radar copied to clipboard

Cleanup/enhancenments to links

Open cmreigrut opened this issue 3 years ago • 2 comments

This changes a few things about links:

  1. Links are now always enabled for blips Previously, it was only enabled for blips if there is no legend (config.print_layout = true)
  2. hrefs are only added if there is actually a link Previously, there were always href created--if there was no link, the href was #. This caused a page refresh and a blink of the tech radar.
  3. Use xlink:href everywhere Previously, blips used xlink:href, but text still used href. xlink:href has actually been deprecated, but currently still has better cross-platform support.
  4. Links can now open in new tabs Previously, links always opened in the existing tab. This adds a new config: links_in_new_tabs. Setting that to true will add a target="_blank" for all hrefs, causing the link to be opened in a new tab. Setting config.links_in_new_tabs to false or not including it at all retains the old behavior.

cmreigrut avatar Nov 17 '21 19:11 cmreigrut

Thanks, @cmreigrut , for the very helpful changes.

rishiraj88 avatar Nov 17 '21 22:11 rishiraj88

Why not merged?

r3code avatar Feb 11 '22 13:02 r3code

@cmreigrut thanks for your contribution and patience. I accepted changes 1,3,4 and incorporated links_in_new_tabs: true as default behavior (incl. README).

I reverted change 2 (dropping the href="#" target), because as implemented currently empty <a> tags negatively affect the SEO score of the resulting page. The code would need to be adjusted to skip the <a> tag in case links are missing. Feel free to send another PR if you're up for it. I can't reproduce the page refresh/blink behavior that you mentioned in Chrome/Safari.

I'm closing this PR in favor of #103.

bocytko avatar Dec 21 '22 18:12 bocytko