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

dev/translation#67 - Allow translation of fields which lack an explicit HTML type #20606

Merged
merged 1 commit into from
Jun 15, 2021

Conversation

totten
Copy link
Member

@totten totten commented Jun 15, 2021

Overview

The Translation entity can be used to translate certain fields. Currently, the field settings (data_type, html_type) must satisfy some requirements before being translated. This change slightly relaxes those requirements.

Before

String fields can be flagged as translatable - but only if their HTML widget is textual.

After

String fields can be flagged as translatable - if their HTML widget is either textual or undefined.

Comments

  • This is motivated to allow civicrm_msg_template (msg_subject, msg_text, msg_html) to marked as translatable.
  • The data_type and html_type constraints were introduced during a much earlier draft. At that time, the list of fields was
    open-ended. Now, the list is much narrower (default=none) and requires an extra opt-in (hook_translateFields). Consequenty, there's less concern about someone trying to translate an inappropriate field.
  • This patch seemed simplest/lowest-change, though I wouldn't object if we landed on an alternative, eg:
    • Completely remove the HTML-type constraint.
    • Change the html_type data for MessageTemplate. (But I don't know if this will have other side-effects.)

…it HTML type

_Overview_: The `Translation` entity can be used to translate certain fields. Currently, the field settings (`data_type`, `html_type`)
must satisfy some requirements before being translated. This change slightly relaxes those requirements.

_Before_: String fields can be flagged as translatable - but only if they a text-style HTML widget.

_After_: String fields can be flagged as translatable - if they have text-style HTML widget or no clear widget.

_Comments_: This is motivated to allow `civicrm_msg_template` (`msg_subject`, `msg_text`, `msg_html`) to marked as translatable.

The `data_type` and `html_type` constraints were introduced during a much earlier draft. At that time, the list of fields was
open-ended. Now, the list is much narrower (default=none) and requires an extra opt-in (`hook_translateFields`). Consequenty,
there's less concern about someone trying to translate an inappropriate field.

This patch seemed simplest/lowest-change, though I am open/ambivalent about any of these approaches:

* Completely remove the HTML-type constraint.
* Change the `html_type` data for `MessageTemplate`. (But I don't know if this will have other side-effects.)
@civibot
Copy link

civibot bot commented Jun 15, 2021

(Standard links)

@eileenmcnaughton
Copy link
Contributor

@totten I'm ok with this but I can't help wonder if we should go with specific opt ins - ie

  • opt into the view based localisation

<localizable>true</localizable>

  • opt into the translation api based localisation
    <translatable>true</translatable>

@totten
Copy link
Member Author

totten commented Jun 15, 2021

@eileenmcnaughton Yeah, long term, we probably need to do something like that.

For the interim, it's kinda handy that it doesn't activate at all - unless there's a some kind of hook-listener. (It means that the system remains inert by default.) Of course, we could probably add an alternate toggle somehow.

@totten totten merged commit 01f7e22 into civicrm:master Jun 15, 2021
@totten totten deleted the master-html-types branch June 15, 2021 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants