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

Repeater fields are wrongly displayed as translatable #459

Closed
tobias-kuendig opened this issue Mar 30, 2019 · 25 comments · Fixed by #461
Closed

Repeater fields are wrongly displayed as translatable #459

tobias-kuendig opened this issue Mar 30, 2019 · 25 comments · Fixed by #461

Comments

@tobias-kuendig
Copy link
Contributor

I have the following model:

class Product extends Model
{
    public $implement = ['@RainLab.Translate.Behaviors.TranslatableModel'];
    public $translatable = [
        'name',
    ];
}

with the following form:

fields:
    name:
        label: Name
        type: text

additional_properties:
    label: Additional properties
    type: repeater
    form:
        fields:
            name:
                label: Name
                type: text
            value:
                label: Value
                type: text

Since the repeater field is called name and name is marked as translatable I end up with this:

image

This isn't what I was expecting, since the repeater field is actually "called" additional_properties.name.

What makes matters worse is that when I hit save, the model's name value gets updated to what was entered in the repeaters name field (in this case "Property name").

To me this is unexpected behaviour. The translatable fields should be aware of their context (being inside a repeater). What do you think?

@tobias-kuendig tobias-kuendig changed the title Repeater fields are wrognly displayed as translatable Repeater fields are wrongly displayed as translatable Mar 30, 2019
@w20k
Copy link

w20k commented Mar 30, 2019

Yep, definitely a bug.

@w20k
Copy link

w20k commented Mar 30, 2019

@tobias-kuendig could you test PR fix #461 @mjauvin ? Thanks!

@tobias-kuendig
Copy link
Contributor Author

This removes the language toggle from the input fields but it looks like it prevents the values from being persisted to the database completely.

After reloading the form the input fields of the repeater are empty.

@w20k
Copy link

w20k commented Apr 1, 2019

@tobias-kuendig thank you for your input, will ping @mjauvin on this one ;)

@LukeTowers
Copy link
Contributor

@tobias-kuendig are you sure that's caused by this? Could be the issues with build 450.

@tobias-kuendig
Copy link
Contributor Author

tobias-kuendig commented Apr 1, 2019

The PR contains only one additional condition for the if statement. If I add it the values are no longer saved.

Could it be that the frontend now uses the default repeater inputs (not the translate ones) but the logic to store the values still wants to take the data from the RLTranslate post value?

Edit: I'm on Build 450 btw

@mjauvin
Copy link
Contributor

mjauvin commented Apr 1, 2019

@tobias-kuendig I tried saving a repeater field with this PR applied and its values get saved as it should, even if the field is not marked as translatable

@w20k
Copy link

w20k commented Apr 1, 2019

Maybe that's the issue with the latest build and MLRepeater, still haven't tested it, yet.
octobercms/library@fd83904#commitcomment-32985893

@mjauvin
Copy link
Contributor

mjauvin commented Apr 1, 2019 via email

@tobias-kuendig
Copy link
Contributor Author

repeater

Demo plugin:

https://github.com/tobias-kuendig/translatable-repeater-test

$ composer info october/*   
october/backend v1.0.450 Backend module for October CMS
october/cms     v1.0.450 CMS module for October CMS
october/rain    v1.0.450 October Rain Library
october/system  v1.0.450 System module for October CMS

@LukeTowers
Copy link
Contributor

@tobias-kuendig @mjauvin if one of you can make a PR from the discussion in octobercms/library@fd83904#commitcomment-32985893 we can get that fix tested and merged in.

@mjauvin
Copy link
Contributor

mjauvin commented Apr 1, 2019

@LukeTowers are you suggesting changing references to $_POST in MLRepeater for the equivalent using Laravel's Request object ?

@mjauvin
Copy link
Contributor

mjauvin commented Apr 1, 2019

Even when disabling the Rainlab.Translate, I get problem saving repeater fields now...

@LukeTowers
Copy link
Contributor

@mjauvin fix has been merged in

@tobias-kuendig
Copy link
Contributor Author

repeater

$ composer info october/*

october/backend v1.0.451 Backend module for October CMS
october/cms     v1.0.451 CMS module for October CMS
october/rain    v1.0.451 October Rain Library
october/system  v1.0.451 System module for October CMS

RainLab.Translate is 1.5.0

Should this problem be fixed in these releases?

@mjauvin
Copy link
Contributor

mjauvin commented Apr 3, 2019

it's working fine with PR #461 applied

@tobias-kuendig
Copy link
Contributor Author

🤦‍♂️ Once #461 is merged this issue is resolved 👍

LukeTowers pushed a commit that referenced this issue Apr 3, 2019
…#461)

Fixes #459. Credit to @mjauvin.

Repeater and NestedForm formwidgets' field names are marked as translatable when they should when their name matches another field name from the model.
@bennothommo bennothommo added this to the 1.6.0 milestone Apr 23, 2019
@vladimiryakimenko
Copy link

repeater

$ composer info october/*

october/backend v1.0.451 Backend module for October CMS
october/cms     v1.0.451 CMS module for October CMS
october/rain    v1.0.451 October Rain Library
october/system  v1.0.451 System module for October CMS

RainLab.Translate is 1.5.0

Should this problem be fixed in these releases?

Translate for fields in repeater not work for me =(
Снимок экрана 2019-06-11 в 9 35 40

public $implement = ['@RainLab.Translate.Behaviors.TranslatableModel'];
public $translatable = ['name','name_full', 'text_blocks_title', 'text_blocks_text'];

@LukeTowers
Copy link
Contributor

@TuxCod no, it's in 1.6.0, you have 1.5.0. Enable edgeUpdates if you want to get the latest changes from the marketplace.

@mohamed-aitlqadi
Copy link

@LukeTowers still same problem :( current version = 1.6.7

@mjauvin
Copy link
Contributor

mjauvin commented Apr 21, 2020

Repeater fields are NOT translatable. Currently, you need to add the repeater name to the translatable array and add a repeater item PER LANGUAGE.

@mohamed-aitlqadi
Copy link

Screenshot (72)_LI

@mjauvin thanks

@muhammad-ihsan
Copy link

muhammad-ihsan commented Jan 29, 2021

@mohamed-aitlqadi how did you fix this? mine doesn't not showing the trans language on October CMS build 470.

Models:

public $implement      = ['System.Behaviors.SettingsModel', 'RainLab.Translate.Behaviors.TranslatableModel'];
public $translatable   = ['services'. 'issues'];

Fields:

# ===================================
#  Form Field Definitions
# ===================================
tabs:
  fields:
    services:
      tab: Service
      type: repeater
      titleFrom: title_when_collapsed
      form:
        fields:
            icon:
              label: Icon
              type: dropdown
            title:
              label: Title
            description:
              label: Description
              type: textarea
              size: small

    issues:
      tab: Issue
      type: repeater
      titleFrom: title_when_collapsed
      form:
        fields:
            title:
              label: Title
            description:
              label: Description
              type: textarea
              size: small


@MedaQ
Copy link

MedaQ commented Jan 29, 2021

@muhammad-ihsan
you need to add this line protected $jsonable = ['services'];
ps : declare your field as text if possible

@mjauvin
Copy link
Contributor

mjauvin commented Jan 29, 2021

@muhammad-ihsan ou cannot translate sub-fields within a repeater, you need to add seperate repeater entries per language.

#461 fixed this bad behavior (which didn't event do the right thing in the first place)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants