wintercms / wn-translate-plugin

Translate plugin for Winter CMS
MIT License
13 stars 18 forks source link

Allow properties of jsonable attributes of a model to be translatable #21

Open multiwebinc opened 2 years ago

multiwebinc commented 2 years ago

I think this would be a good idea to allow properties of jsonable attributes to be translatable. This would allow you to translate specific sub-fields of a repeater to be translated. For example:

Model:

class Gallery extends Model
{
    public $implement = ['@RainLab.Translate.Behaviors.TranslatableModel'];

    protected $jsonable = ['images'];

    public $translatable = [
        'images.alt_text',
        'images.description',
    ];
}

Then in the yaml:

tabs:
    fields:
        images:
            type: repeater
            form:
                fields:
                    image:
                        label: Image
                        type: mediafinder
                        mode: image
                        imageHeight: 150
                        imageWidth: 250
                        span: left
                        required: true
                    alt_text:
                        span: right
                    description:
                         type: textarea
                         span: full

If someone could give me some direction on how to even get started, I might be able to get a PR for this.

LukeTowers commented 2 years ago

@bennothommo any thoughts?

mjauvin commented 2 years ago

If I remember correctly, the current architecture does not really make this possible... but I'd LOVE if this was implemented, it would solve so many problems for my use case.

bennothommo commented 2 years ago

I'd say it wouldn't be possible simply because of the lack of IDs in the JSON-able data - you'd essentially have to copy the entire JSON data over for each translation and link it to the main record.

In this scenario, it would be structurally better to use a related model with translatable fields.

multiwebinc commented 2 years ago

@bennothommo Another option I thought of would be to store the translations right in the JSON. Like having fieldName, then fieldName[en], fieldName[es], fieldName[fr], etc.

bennothommo commented 2 years ago

@multiwebinc that could work. We'd have to establish (in the docs, perhaps?) that the developer will need to account for this extra data if they go down this route, and use a larger data format column (like say a BIGTEXT column in their database), but it's certainly within the realm of possibility.

mjauvin commented 2 years ago

yeah, but the beauty of the Translate plugin is that if you disable it, the data is unchanged (all translations are stored in external tables)

-- Marc

On Sun., Jan. 30, 2022, 20:56 Ben Thomson, @.***> wrote:

@multiwebinc https://github.com/multiwebinc that could work. We'd have to establish (in the docs, perhaps?) that the developer will need to account for this extra data if they go down this route, and use a larger data format column (like say a BIGTEXT column in their database), but it's certainly within the realm of possibility.

— Reply to this email directly, view it on GitHub https://github.com/wintercms/wn-translate-plugin/issues/21#issuecomment-1025308391, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPLTPURPM33XB6UIWIDYJ3UYXT47ANCNFSM5MZVWE7Q . You are receiving this because you commented.Message ID: @.***>

multiwebinc commented 2 years ago

@bennothommo Presumably for columns that contain JSON, people should be using the JSON type (i.e. $table->json() in their migrations). Assuming Laravel actually uses the JSON data type in the DB, this gives a limit of 1GB for MySQL and 255 MB for PostgreSQL. SQLite is around 1GB for TEXT or BLOB. So it's very unlikely these limits would ever be reached. Of course, it wouldn't hurt to add a note to the docs in case people use TEXT. What happens if the value exceeds the column limit? Is it truncated, or does the query fail?

@mjauvin Good point. I can't really think of a truly elegant way to get around that because of the lack of IDs, but perhaps when the translate plugin has been disabled and someone tries to write to a translatable JSON field, present a confirmation dialog saying something like "The field you are editing is translatable, but the Translate plugin has been disabled or removed. Making changes will remove any translated strings." Maybe also recommending creating a backup, re-enabling the plugin, removing the $translatable property of the model, etc. I would assume that most times people disable the plugin it's because they no longer want to use it, so the translation strings are generally not important anyway.

mjauvin commented 2 years ago

@multiwebinc if the plugin is disabled, you wouldn't be able to inform the user about this, unless you have some exception handling code in wintercms core

multiwebinc commented 2 years ago

@mjauvin Yeah, I was thinking of putting it in the core.

multiwebinc commented 2 years ago

@mjauvin Another idea that just occurred to me is to add something like a backend.plugin.afterRemove hook that will allow plugin developers to display a popup (or plain text message in the CLI) one single time after the plugin is deleted. This could be used to explain what happens to any residual data (such as translations) that are not deleted, any further steps that might be necessary, to display a discount message for people who might want to purchase a pro version of the plugin at a discounted rate, or to display a generic "We're sorry to see you go" type message.

kubamarkiewicz commented 6 months ago

I'd love to see this feature implemented, that would make using jsonable fields on multilingual websites possible.

mjauvin commented 6 months ago

@multiwebinc @kubamarkiewicz I would appreciate it if you guys could test PR #76

kubamarkiewicz commented 6 months ago

That's great news, I'll test it!

mjauvin commented 6 months ago

@kubamarkiewicz I added a comment in the PR's description to clone the branch directly in order to test this.

kubamarkiewicz commented 6 months ago

@mjauvin I made some tests with repeater and nestedform fields and works almost perfectly!

I have found only one problem. When I have repeater field configured as in the example that you have included in Readme.md then when I save translations and reload the form, then when I switch the language in 'Job Title' field, the value does not change. I works correctly however, with the following configuration:

    public $translatable = [
        'data',
        'data[title]',
    ];
fields:
  data:
    type: repeater
    translationMode: fields
    form:
      fields:
        name:
          label: Name
        title:
          label: Job Title
        phone:
          label: Phone number
kubamarkiewicz commented 6 months ago

It would be also great if translations for 'blocks' field type could be implemented in a similar way. There is a difference however, because in that case there is no possibility to configure 'translatable' property in the controller and I have suggested adding 'translatable' property in fields definition in yaml: https://github.com/wintercms/wn-blocks-plugin/discussions/25

Do you think that would be possible?

mjauvin commented 6 months ago

@kubamarkiewicz I was missing data in the translatable array in my example.

This is needed in order to fetch the right version of the data for the selected locale.

mjauvin commented 6 months ago

It would be also great if translations for 'blocks' field type could be implemented in a similar way. There is a difference however, because in that case there is no possibility to configure 'translatable' property in the controller and I have suggested adding 'translatable' property in fields definition in yaml: wintercms/wn-blocks-plugin#25

Do you think that would be possible?

Once this gets released, we'll look into adding support to the blocks plugin.

kubamarkiewicz commented 6 months ago

@mjauvin ok, now the example in Readme.md works correctly.

kubamarkiewicz commented 6 months ago

@mjauvin I made some more in depth testing of repeater field and I have found a problem with reordering items: when there are two items saved in a repeater field, when I reorder them and refresh the page, the fields that are translatable (in this case 'Job title') appear in wrong items:

image
mjauvin commented 6 months ago

Interesting find.

If you can continue those within the PR itself, that would be appreciated.

kubamarkiewicz commented 5 months ago

Would it be possible to allow alternative syntax to specify translatable fields inside .yaml file?

fields:
  data[contacts]:
    type: repeater (or nestedform)
    translationMode: fields
    form:
      fields:
        name:
          label: Name
          translatable: true
        title:
          label: Job Title
          translatable: true
        phone:
          label: Phone number

The goal is to be able to define jsonable fields entirely in fields.yaml file without necessity to modify the model file. I have a plugin where .yaml file containing nested form definition is located in theme folder and I would like to allow people to define fields there without touching the model file.

The syntax could be similar to the proposed syntax for translations in blocks plugin: https://github.com/wintercms/wn-blocks-plugin/discussions/25

What do you think about it?

mjauvin commented 5 months ago

Would it be possible to allow alternative syntax to specify translatable fields inside .yaml file?

fields:
  data[contacts]:
    type: repeater (or nestedform)
    translationMode: fields
    form:
      fields:
        name:
          label: Name
          translatable: true
        title:
          label: Job Title
          translatable: true
        phone:
          label: Phone number

The goal is to be able to define jsonable fields entirely in fields.yaml file without necessity to modify the model file. I have a plugin where .yaml file containing nested form definition is located in theme folder and I would like to allow people to define fields there without touching the model file.

The syntax could be similar to the proposed syntax for translations in blocks plugin: wintercms/wn-blocks-plugin#25

What do you think about it?

I wanted to do that as well... thanks for confirming this not only makes sense to me.

kubamarkiewicz commented 5 months ago

@mjauvin definitely it makes sense!