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
Empty fields with nested repeaters #307
Comments
I have same issue |
Me too in a slightly different context. I was doing tests with https://github.com/daftspunk/oc-test-plugin states:
tab: States
type: repeater
form:
fields:
title:
label: State
item:
type: repeater
prompt: Add new suburb
form:
fields:
subtitle:
label: Suburb
span: left I saved these values (same in my other language except that If I try to switch to my other langauge, I get 2 empty text fields inside my Switching back to the default locale also adds the empty fields. |
Discover the same issue with OctoberCMS 434 & RainLab Translate 1.3.8
First behavior : empty repeaters are added when switching localeFor reproduce :
Demo : http://take.ms/ZSSfE if you create the following structure:
and click switch button and choose the same lang (default language) you will get something like that
Switching to the same language is obviously not a normal action but this reflect the problem we have when switching to an not empty language. Second behavior : removing one of these empty fields in one language will also remove it in main languageFor reproduce :
Demo : http://take.ms/qpp5p |
A bounty have been added on this issue : https://www.bountysource.com/issues/50188520-empty-fields-with-nested-repeaters |
I did some experiments yesterday and here's what I found: At this line, I tried to modify the http request that is sent when changing language, remove these I write it here hoping in may help to find a solution, but I am realy unsure about what these parameters are used for and what could break when they are removed. |
UPDATE: I was wrong, the problem is more complicated and same as described by @ChVuagniaux Changing type from textarea to richeditor doesn't influence at all, it was just circumstance. Translate 1.4.0, October 435.
|
I found a quick "fix" to this problem. It's a temp solution because it introduces a new bug (see below). But in some use cases the new bug may be less problematic than the actual behavior. Here's what I did: at this line https://github.com/octobercms/october/blob/master/modules/backend/formwidgets/Repeater.php#L199 $itemIndexes = post($this->indexInputName, $loadedIndexes); by this: $itemIndexes = $loadedIndexes; Note that it's an October hack, not a translate plugin hack. This solves the problem of empty fields being created when switching the repeater language. If I understood correctly the repeaters logic, the actual implementation doesn't support correctly nested repeaters because when you ask to create a new repeater item, along with the POST data is sent a This array is then used in the back-end to rebuild a representation of the repeater as it is in the user browser. If it's not done, like in my fix, the back-end doesn't know the existance of non-saved fields and cannot create sub-elements. What happens actually is that in the backend representation, all repeater nodes at a given level will have the ammount of children of the node with the highest children count at the same level. The problem isn't noticeable as long as the broken representation stays in the back-end. But when switching locale with the translate plugin, it is sent to the browser to refresh the form. |
I tried to reproduce this behavior and actually realized, that the data looks correct in the database, but the repeater renders these empty fields, so it is a markup/javascript issue in my eyes. Can anyone of you ( @ChVuagniaux @Fl0Cri @pvullioud ) confirm that? |
@alxy Unfortunetly is not that simple. I was debuging a little and there are bugs in repeater in october core. I found a fix in context of using nested repeaters in pages plugin. But unfortunetly it does not seems to work when used on normal backend pages. |
@pszczegielniak Sure, if you safe the data again with the empty fields added, then these are in the database as well. But for me it appears like this: That leads to the conclusion, there must be something wrong with the processing and rendering of the existing items, when triggered via the |
@alxy in fact, the process of saving to the database and switching locale is very different.
If you do something, either saving or trigering an ajax handler (here I click on the button to add a new "state" to the repeater), a POST request with the following data is issued: Now if we are in the context of saving the data to the DB, it's easy: just take data pointed by The problem with ajax handlers (particulary
Depending on what ajax handler we are running, this may be harmless. For example
The JS will receive this html and paste it at the right place. The fact that the structure that October thinks to be right is broken doesn't matter. But for
And here the JS will overwrite the "correct" data in the browser. Probably, the best thing to do would be to use I cannot work much on this these times, so may someone else find a fix quickly :) |
@Fl0Cri I've sent a message to @daftspunk to get his input on this since he built the repeater in the first place :) We should be able to come up with a fix once he sets his mind to it. Thanks to everyone for the great work on digging into the cause of the issue so far! |
Friendly reminder about this issue and the bounty |
Hi guys, i'm looking for a solution to this bug, Actually i've found that when you're switching locale, some datas change: Before i get this changes, i need to create in my static page a "title" repeater and a "subtitle" repeater for FR locale, then i switch to EN locale and this time, i create just "subtitle" repeater and "content" repeater (which has richeditor, image field and radiobutton in it), until this everything is ok. But when i switch again to FR locale, it starts messing up as you can see below .
I will continue investigate this, but i hope it could help someone to find a fix too :) |
Hi everyone, protected static $onSwitchLangCalled = false; // translable repeater support line 238) processExistingItems() $itemIndexes = post($this->indexInputName, $loadedIndexes);
$itemGroups = post($this->groupInputName, $loadedGroups);
if(self::$onSwitchLangCalled){
$itemIndexes = $loadedIndexes;
$itemGroups = $loadedGroups;
}else{
$itemIndexes = post($this->indexInputName, $loadedIndexes);
$itemGroups = post($this->groupInputName, $loadedGroups);
} /plugins/rainlab/translate/formwidgets/MLRepeater.php $this->actAsParent();
self::$onAddItemCalled = true; // translable repeater support
$parentContent = parent::render();
self::$onAddItemCalled = false; // translable repeater support
$this->actAsParent(false); 174) reprocessExistingLocaleItems($data) self::$onSwitchLangCalled = true; // translable repeater support
$this->processExistingItems();
self::$onSwitchLangCalled = false; // translable repeater support Any suggestion on help will be appreciated :) |
@massimomegistus , i've just tested it on static page with repeaters and it does not fix the issue :( i'll search again if i could find something... |
Thank you @Ladylain, let me know, please |
Hi @Ladylain, /**
* Splices in some meta data (group and index values) to the dataset.
* @param array $value
* @return array
*/
protected function processSaveValue($value)
{
if (!is_array($value) || !$value) {
return $value;
}
return array_values($value);
} i've tested on a static page too right now and it seems to work. mediacarousel:
name: Carousel
description: Thumbs Carousel
icon: icon-leaf
fields:
carousel_slides:
label: Thumbs carousel
prompt: Add a new slide
type: repeater
form:
fields:
slide_image:
label: Image
type: mediafinder
mode: image
span: left
slide_caption:
label: Caption
span: right Switching form a language to another the slides are mantained as expected. Without my fixes all the slides are repeated in each carousel. This are the screenshots: as you can se, in english and italian the are different groups and the carousel/slide hierarchy is mantained Let me know :) |
O_O You are my fucking hero at the moment :) All seems to work good on my side, i have a complex repeater which allows me to do many things that i want, but until now, translation was creating issues, and here you come with a great solution. I think we need to test a little more to make sure everything is ok for everybody, but it's a hell of a lot of progress. Thanks so much, my content redactor will be happy on monday :p and of course, have a nice week end |
I should have checked into this the last couple of days, but I've been working on this too in combination with octobercms/october#3729. I didn't know it would be so popular all of a sudden, but I just saw all this activity when I was readying my own PR for submission. Anyway, I reached similar conclusions as @massimomegistus and named things differently, but I'm right in line with this fix. What I believe the above version doesn't do, however, is handle nested group repeaters or any other widget that overrides |
Well, nested group repeaters should be okay due to octobercms/october#3523 avoiding the |
Hi Heros, |
@pvullioud so does @jimcottrell's PRs solve all the issues? |
@LukeTowers, i've just tested it again, and without @massimomegistus's code, it doesn't fixed the problem for my issue rainlab/pages-plugin#353 but with the @massimomegistus's, it works perfectly |
+1 |
@jimcottrell any idea what's up with @Ladylain and @buccia? |
@Ladylain when trying @jimcottrell's fix, have you applied both #409 and octobercms/october#3729 ? In my case it solved the main problem. I only found 2 problems that can occur when trying to manipulate nested repeaters. Can someone confirm these? |
@LukeTowers I haven't had a chance to get to the problems @Fl0Cri mentioned with the fix in #409 yet, but I should be able to tackle that soon. There's nothing wrong with the code from @massimomegistus, so there's no reason people wouldn't have success with it for the empty field issue. It's just that it doesn't address the underlying problem of repeaters not calling That said, if I can't get to fixing the deeper issue in the next couple of days, I'm happy to either submit a simpler PR or let @massimomegistus do it to address just the empty field issue described here. |
@jimcottrell I'd rather wait for the deeper fix, if people need an immediate fix they can apply the patch from @massimomegistus. Thanks for your work on this so far! |
@massimomegistus you're a genius ! |
Can everyone please test with a fresh install and octobercms/october#4234 and #466 applied? |
Bring MLRepeater inline with core changes to Repeater. Related: octobercms/october#4234, #307, #290.
Closing as the above PRs have been merged, will reopen if anyone still has a problem after applying those fixes. |
could someone help me? I got a problem using the plugin with nested repeater. I have my fields like:
[MLRepeater]
[Repeater]
-- Text Field CHILD
-- Rich Editor CHILD
...
[/Repeater]
[Repeater]
-- Text Field CHILD
-- Rich Editor CHILD
...
[/Repeater]
...
[/MLRepeater]
When I change language, "Textfield ROOT" changes correctly, however "Text Field CHILD" appears blank, empty, even if data in DB are correct and when I print a dump($this) for MLRepeater "data" is correct. But, repeaters inside are empty. When I tried to get saved data in Frontend, Everything is right.
The text was updated successfully, but these errors were encountered: