search

andreasbm / lit-translate

A blazing-fast and lightweight internationalization (i18n) library for your next web-based project
https://codepen.io/andreasbm/pen/MWWXPNO?editors=1010
MIT License
137 stars 14 forks source link

added translateUnsafeHTML directive #14

Closed stefanholzapfel closed 4 years ago

stefanholzapfel commented 4 years ago

Hi!

I have written a translateUnsafeHTML directive as needed in issue #13 and in the way proposed by @davidsteinberger in issue #9 .

Please review and merge if it's viable.

stefanholzapfel commented 4 years ago

Hi Andreas!

Thanks for your feedback. Sure, using the original unsafeHTML directive is much smarter :)

I have made another change and added a parameter to the original translate instead of writing it's own directive. What do you say?

Of course we could also move the renderHTML bool into the ITranslateConfig interface if you prefer.

stefanholzapfel commented 4 years ago

@tree shaking: Good point, reverted to 2 functions.

I'm not happy with the naming though. Since the translations stem from a json file that is under the developers control, it's not "unsafe" to use in the original sense of unsafeHTML where "unsafe" should raise awareness that sanitization is necessary for user input. The term "HTML" in contrast adds information about what differentiates both directives.

But I now know what it does, so you can merge it that way if you want :)

andreasbm commented 4 years ago

I'm not happy with the naming though. Since the translations stem from a json file that is under the developers control, it's not "unsafe" to use in the original sense of unsafeHTML where "unsafe" should raise awareness that sanitization is necessary for user input. The term "HTML" in contrast adds information about what differentiates both directives.

I think it's a very valid point - I might revert the name back to translateUnsafeHTML before release 👍 Thanks a lot for your PR - it's a very nice addition to lit-translate!

I will merge it now, have a great weekend.