getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 168 forks source link

Translations should not remove non-translatable fields in structures #2790

Open hdodov opened 4 years ago

hdodov commented 4 years ago

This issue is a continuation of #1571. @afbora asked me to open a new issue.

Describe the bug
When you save a translation, all non-translatable fields should be skipped. This was fixed in #1571. But now there's a problem in structures - there, non-translatable fields should not be skipped because it leads to unexpected behavior.

To Reproduce I have the following site.yml:

fields:
  text:
    type: text
  textConst:
    type: text
    translate: false
  test:
    type: structure
    fields:
      text:
        type: text
      textConst:
        type: text
        translate: false

...and the following site.en.txt:

Text: Foo

----

Textconst: Bar

----

Test:

- 
  text: foo
  textconst: bar

Then, I update the Bulgarian translation with the values Text = FooBG and Test/text = fooBG. That's the resulting site.bg.txt:

Text: FooBG

----

Test:

- 
  text: fooBG

As you can see, the Textconst field is missing, and it should be - this way Kirby will use the default translation value. However, the textconst field in the structure is also missing. This means that I get the value null for it in the template:

var_dump($site->test()->toStructure()->first()->text()->value()); // "fooBG"
var_dump($site->test()->toStructure()->first()->textConst()->value()); // null

At the same time, that value is grayed out in the panel and you can't fill it:

image

What's interesting is that even if I add textconst in site.bg.txt manually with my text editor, refresh the panel, and update the translation, it gets removed. (1)

Expected behavior
I see two solutions for that:

  1. In the toStructure() call, Kirby also loads the default translation values and does array_replace_recursive() with the current translation values
  2. Kirby should leave values in structures as they are, so that (1) doesn't happen and plugins can handle the synchronization of values across translations

I'm currently working on a plugin that traverses the page blueprints and synchronizes structures according to them. For the most part, it works great, but it fails when it tries to copy values from the default translation - they are lost in the $page->update() call. If you implement the second solution I proposed, my plugin would be able to solve the problem.

Kirby Version
3.4.2

caplod commented 4 years ago

There seems to be one more problem.

As you can see, the Textconst field is missing, and it should be - this way Kirby will use the default translation value.

Even if the field is removed by hand in the txt file, Kirby will not display the default translation for the field 'number' . Or am I missing something?

default.yml

teststructure:
  label: Test Structure Field
  type: structure
  fields:
    title:
      label: Titel
      type: text
    number:
      label: Nummer
      type: text
      translate: false <--

default.de.txt (default language)

----

Teststructure:

- 
  title: test
  number: "2"

default.en.yml (translation)

----

Teststructure:

- 
  title: test en
benjohnde commented 3 years ago

Is there any eta to get this fixed? @bastianallgeier I mentioned you because you closed #2819. I am not quite sure how hard it is to implement a consistent behaviour but we are also facing that issue on three different web pages.

Having a structure, two of three fields are translate: false. We cannot get this to work as the values just disappear instead of having the default ones used! Our work around is by retrieving / fetching the page once again with the default language and reading out the values by hand.

jonasfeige commented 3 years ago

Is there any update/ETA on this? Just ran into the issue myself with a structure field. It unfortunately makes them quite unusable in multi-language setups when you have fields that are not meant to be translated.