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

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

Open
hdodov opened this issue Aug 28, 2020 · 3 comments
Open

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

hdodov opened this issue Aug 28, 2020 · 3 comments
Labels
needs: delay ⏳️ Requires more time, on hold type: bug 🐛 Is a bug; fixes a bug

Comments

@hdodov
Copy link
Contributor

hdodov commented Aug 28, 2020

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

@afbora afbora added the type: bug 🐛 Is a bug; fixes a bug label Aug 28, 2020
@caplod
Copy link
Contributor

caplod commented Sep 3, 2020

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
Copy link

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
Copy link

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.

@stale stale bot added the type: stale 💤 Will be closed soon because there was no recent activity label Aug 10, 2022
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 2, 2022
@distantnative distantnative reopened this Feb 26, 2023
@distantnative distantnative removed the type: stale 💤 Will be closed soon because there was no recent activity label Feb 26, 2023
@getkirby getkirby deleted a comment from stale bot Jul 30, 2024
@distantnative distantnative added the needs: delay ⏳️ Requires more time, on hold label Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: delay ⏳️ Requires more time, on hold type: bug 🐛 Is a bug; fixes a bug
Projects
None yet
Development

No branches or pull requests

7 participants