Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linkify annotations #946

Merged
merged 3 commits into from
Aug 13, 2017
Merged

Linkify annotations #946

merged 3 commits into from
Aug 13, 2017

Conversation

w0rm
Copy link
Member

@w0rm w0rm commented Aug 13, 2017

This pr partially addresses #884 by making the links in annotations clickable. I'm not sure that the links in labels should be clickable, I think it would be too much and might affect the rendering performance of the alert list.

The implementation is rather simple: I'm splitting the text by words, then checking if a word starts with http(s)://. This means each link should be surrounded by a whitespace.

screen shot 2017-08-13 at 01 56 14

@w0rm w0rm requested review from mxinden and stuartnelson3 August 13, 2017 00:01
(\result ->
case result of
Ok link ->
a [ href link ] [ text link ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add target "blank" here?

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change. Really enjoyed reading this code! This looks good to me.

@mxinden mxinden merged commit 250bd35 into master Aug 13, 2017
@w0rm w0rm deleted the linkify-annotations branch August 13, 2017 18:16
iksaif pushed a commit to iksaif/alertmanager that referenced this pull request Sep 15, 2017
* Make links in annotations clickable

* Update test script and bindata.go

* Add target = _blank
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants