Skip to content
This repository has been archived by the owner on Jul 22, 2020. It is now read-only.

Linkify URLs in annotations #156

Merged
merged 2 commits into from
Aug 18, 2017
Merged

Linkify URLs in annotations #156

merged 2 commits into from
Aug 18, 2017

Conversation

prymitive
Copy link
Contributor

Right now we only convert annotations into links if the entire value is a link, this turns links inside annotation text into clickable hyperlinks.

Fixes #152

@terinjokes
Copy link

Not to nitpick too much, but linkify-it seems to have better support for autolinking text.

Thinking about this post-React, this gets significantly easier (and safer) to do if the URLs can be provided as separate pieces of metadata. Is this something we could convert to?

@prymitive
Copy link
Contributor Author

If annotation text is entirely an URL (key:http://foo) it will be explicitly marked as such and moved to alert.links.
This change is for annotation with link inside the text (key:go to http://foo). I can switch to linkify-it, not sure about splitting, can you give an example of the metadata you have in mind?

@filippog
Copy link
Contributor

filippog commented Aug 8, 2017

FWIW I gave linkifyjs a try with a sample url I had while testing #152 with more than one closing parenthesis and it didn't work as expected. I've reported the issue as Hypercontext/linkifyjs#209 (see also markdown-it/linkify-it#23) though arguably it isn't something easily fixable or maybe in scope at all since the url I gave isn't url encoded anyway

@prymitive
Copy link
Contributor Author

@filippog can you share an example that had this issue? Would be nice to add a test for it.

@filippog
Copy link
Contributor

@prymitive for sure! The link is sth like this:

https://kibana/app/kibana#/dashboard/dashboard_name?_g=(time:(from:now-1h,mode:quick,to:now))&_a=(filters:!((query:(match:(host:(query:hostname,type:phrase))),meta:(alias:!n,disabled:!f,index:'logstash-*',key:host,negate:!f,value:hostname)),(meta:(alias:!n,disabled:!f,index:'logstash-*',key:program,negate:!f,value:puppet-agent),query:(match:(program:(query:puppet-agent,type:phrase)))),(meta:(alias:!n,disabled:!f,index:'logstash-*',key:level,negate:!f,value:ERROR),query:(match:(level:(query:ERROR,type:phrase))))))

@prymitive prymitive force-pushed the linkify branch 2 times, most recently from fb81522 to d3a838c Compare August 18, 2017 04:51
@prymitive
Copy link
Contributor Author

javascript-linkify seems to be the only module that handles kibana type links correctly, need to cleanup package-lock.json diff

use linkifyjs to make all URLs in the annotation clickable, but since it requires us to stop escaping html when rendering annotation object let's first manually escape it to prevent rogue alerts with malicious annotations from executing <scripts> and other ugly things in user browsers
Help icon doesn't make sense for annotation key, info icon is a bit more related to the usage of annotations
@prymitive
Copy link
Contributor Author

Cleaned package-lock.json, diff is happy now

@prymitive prymitive merged commit 577c15e into master Aug 18, 2017
@prymitive prymitive deleted the linkify branch August 18, 2017 19:20
@filippog
Copy link
Contributor

Nice, thanks a lot!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants