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

[IMP] mail: better activity info translatability by providing context #116802

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oomsveta
Copy link
Contributor

No description provided.

@robodoo
Copy link
Contributor

robodoo commented Mar 28, 2023

@C3POdoo C3POdoo requested a review from a team March 28, 2023 09:45
Comment on lines 57 to 62
_t(
`<span class="fw-bolder %(classes)s }}">%(delay)s:</span> <span class="fw-bolder">%(activity name)s</span> <span class="o-mail-Activity-user">for %(responsible employee)s</span>`
),
Copy link
Contributor

Choose a reason for hiding this comment

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

The html should be wrapped inside the getter and not exposed as a translation key.

@C3POdoo C3POdoo added the RD research & development, internal work label Mar 28, 2023
@oomsveta oomsveta force-pushed the master-fix_activity_info_translation-wil branch from 9a79154 to 4986048 Compare March 28, 2023 11:50
return markup(
sprintf(
_t(
`<span class="fw-bolder %(classes)s }}">%(delay)s:</span> %(activity name)s <span class="o-mail-Activity-user">for %(responsible employee)s</span>`
Copy link
Contributor

Choose a reason for hiding this comment

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

I would keep all classes out of translation if possible, it looks like a maintenance nightmare to have them inside.

Even html tags are annoying, but a bit less so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants