-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Changed the creation of the update notification link #309
Conversation
…js object from concated string.
@Faldon, thanks for your PR! By analyzing the annotation information on this pull request, we identified @jancborchardt, @MorrisJobke and @LukasReschke to be potential reviewers |
@@ -18,7 +18,7 @@ $(document).ready(function(){ | |||
version = oc_updateState.updateVersion, | |||
docLink = oc_updateState.updateLink, | |||
text = t('core', '{version} is available. Get more information on how to update.', {version: version}), | |||
element = $('<a>').attr('href', docLink).attr('target','_blank').text(text); | |||
element = $('<a href="'+docLink+'" target="_blank">'+text+'</a>'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure to have text and docLink escaped in this cases then using escapeHTML.
Tested and works 👍 Code looks good |
looks good 👍 |
Great stuff @Faldon, will add you to the organization and the Javascript team. :) |
@jancborchardt With pleasure. I finally thought, it's time to contribute back. That's the point in OSS. |
@Faldon welcome to Nextcloud then! :) Very much looking forward to future contributions. |
🚀 🎉 |
Instead of chaining the DOM manipulation commands, create an object from a concatenated string.
Solves #273