-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fixed inline translation function call with translate(key, {}, html) #9787
Conversation
Code Climate has analyzed commit 51ceaaa and detected 0 issues on this pull request. View more on Code Climate. |
Codecov Report
@@ Coverage Diff @@
## main #9787 +/- ##
=======================================
Coverage ? 82.07%
=======================================
Files ? 98
Lines ? 5940
Branches ? 0
=======================================
Hits ? 4875
Misses ? 1065
Partials ? 0 |
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.
Just a note on the parameter simplicity. I was also wondering -- can you add to the translation documentation in the README to clarify the usage of this new parameter? Again it's fine to do this in another PR -- but if you can add an item to your planning issue in #9686 that would be great. Thank you!!!
<div class="dropdown-menu"> | ||
<div class="dropdown-item"> | ||
<a style="margin-top:-8px;" rel="tooltip" title="<%= translation('tag.show.blog_updates') %>" href="/subscribe/tag/blog"> | ||
<a style="margin-top:-8px;" rel="tooltip" title="<%= translation('tag.show.blog_updates',{},false) %>" href="/subscribe/tag/blog"> |
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.
Hi @imajit this is looking really nice! I wonder if it's possible to simplify the parameters and omit the {}
by checking if options == true
OR by using translation(key, {html: true})
for some slightly shorter cleaner syntax? This could definitely be done in follow-up, as this PR looks good. Are there any other places where this is used and where it would need to be updated?
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.
Yeah, I'll change the syntax to make it more clear.
I think I have changed all the occurrences. If I find any new ones I'll make ftos to change them.
Sure, will add documentation about the updates in the function. |
…ubliclab#9787) * Adding some more corrected tags * Changed translation function * Changed all translation function inside html tag Co-authored-by: imajit <[email protected]>
…ubliclab#9787) * Adding some more corrected tags * Changed translation function * Changed all translation function inside html tag Co-authored-by: imajit <[email protected]>
Part of #9686
Translation function needs to be called with
false
parameter in cases where it is called inside HTML tags.