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

Fix 607 - Format http(s) links and emails as <a> in query results #4257

Merged
merged 2 commits into from Sep 21, 2018

Conversation

adube
Copy link
Contributor

@adube adube commented Sep 20, 2018

This patch introduces a new filter in ngeo that allows to return a value as trusted HTML in addition to automatically converting http(s) or email strings into anchor html strings.

StringToHtmlReplacements is an angular constant that defines expressions that can be converted and a template to use when a string matches.

ngeoTrustHtmlAuto is the new filter that does like ngeoTrustHtml, but will replace strings with html if a match is found.

Note: Instead of the regex provided in 607 for the email, I used the one from: https://www.regular-expressions.info/email.html, which is lighter when used in a case-insensitive regex and more robust.

@fredj
Copy link
Member

fredj commented Sep 21, 2018

We have a mix between StringToHtmlReplacement and StringToHtmlReplacements; can you change to have always StringToHtmlReplacements?

* @param {angular.$sce} $sce Angular sce service.
* @param {!Array.<!ngeox.StringToHtmlReplacement>}
* ngeoStringToHtmlReplacements List of replacements for string to html.
* html.
Copy link
Member

Choose a reason for hiding this comment

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

minor: extra html. comment

@fredj
Copy link
Member

fredj commented Sep 21, 2018

And is the addition in ngeox really needed? (cc @sbrunner )

EDIT: @sbrunner told me that this structure can be provided in dynamic.js so the addition is correct

Copy link
Member

@fredj fredj left a comment

Choose a reason for hiding this comment

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

Looks very good.

@adube
Copy link
Contributor Author

adube commented Sep 21, 2018

Thanks for the reply.

We have a mix between StringToHtmlReplacement and StringToHtmlReplacements; can you change to have always StringToHtmlReplacements?

StringToHtmlReplacements is the global value that is an array, i.e. ngeoStringToHtmlReplacements.

StringToHtmlReplacement is the ngeox definition of what a single entity of a "replacement" is, that is: the expression and the template when we have a match. i.e.: ngeox.StringToHtmlReplacement.

It think this is okay as it is. Please, let me know if you agree.

@fredj
Copy link
Member

fredj commented Sep 21, 2018

@adube ok, understood. Please merge

@adube adube merged commit c3aff97 into camptocamp:master Sep 21, 2018
@adube adube deleted the v2_4-607-auto-links branch September 21, 2018 12:05
@sbrunner sbrunner added this to the 2.3 milestone Oct 30, 2018
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.

None yet

3 participants