Feat/link data type

[sidneyw]

Implements link data type proposal in #3099.

Adds the link dataType as a displayable field in both the NodeDetailsGenericTable and NodeDetailsInfo components via conditional render.

[squaremo]

Thanks for this! I'm not qualified to review the code change; but, to get it in shape for someone who is, I recommend rebasing against master, and squashing the redundant commits out. git rebase -i is your friend (though sometimes a difficult-to-like friend).

[sidneyw]

Sorry about that @squaremo. I've squashed and rebased. Thanks for taking a look!

[squaremo]

Sorry about that @squaremo. I've squashed and rebased

No worries, thanks for that.

I like the idea in #3099, but we shall have to wait for someone more familiar with Scope to check the code. One thing that did occur to me: do you perhaps want to have both a link href and a link text?

[sidneyw]

@squaremo How might we get both values from a plugin into the view? We had tested this idea earlier by sending the value as value: link-href:link-text and parsing the string in the view but decided not to move forward with that solution. Is there a better way?

[squaremo]

How might we get both values from a plugin into the view?

Good question -- my understanding is too superficial to have grasped that you couldn't just supply a structure. I can certainly see that encoding the values in a string seems like a step too far. :-S

[rndstr]

Appreciate the PR @RcrsvSquid! First feedback:

• extend dataType documentation in site/plugins.md
• links appear unstyled for me (should be same as others)
• I think there are some more places where these values are rendered such as
  

  scope/client/app/scripts/components/node-details/node-details-table-row.js

  Line 43 in da6995b

  function renderValues(node, columns = [], columnStyles = [], timestamp = null, topologyId = null) {

[sidneyw]

@rndstr I've responded to your comments with changes

[rndstr]

Thank you @RcrsvSquid, LGTM!

I agree with @squaremo that having text & link would be nice to have but that would require a bigger change

[sidneyw]

@foot @rndstr Is there anything else you need from me to be able to merge this pull request?

[rndstr]

@RcrsvSquid it just got brought to my attention by @foot that we should add a rel="noopener noreferrer" to all the links which prevents a phishing possibility. then we are good to go, thanks!

[foot]

Yeah! norel/noop would be one more little nice to have but as the plugin should be generating safe urls we can probably merge as is. 👌

[foot]

But @rndstr makes a good point against future proofing, if a future plugin proxies dodgy urls etc. So yes, if you could please add the noop/norel that would be grand! Ta!

[sidneyw]

@foot Done!

[rndstr]

@RcrsvSquid awesome ✨

(did run tests locally, they passed)