Fixes to MLRepeater for nested repeater usage

[jimcottrell]

Not ready to be merged yet, but I'm submitting so it's easy for others to test and comment on. This applies to #307, which I've apparently duplicated effort on.

Note that this is in combination with octobercms/october#3729, so any testing should have that applied to core.

[LukeTowers]

Just to tie this all together was there a PR to the test plugin to demonstrate the issue and prove a fix for the issue across both MLRepeater and the base Repeater?

[jimcottrell]

That's definitely the next step.

[Fl0Cri]

Nice work !

I have a site with many nested, grouped, and ML repeaters using StaticPages on which to test it.
I applied both this PR and octobercms/october#3729
I tested everything I could think about and it looks nice (it fixes #307 ).

There's only 2 issues I could see but these are not breaking bugs IMO. See video demonstrating the problems: http://take.ms/WU53rb

1. • From a base page (in default language), switch to a non translated lang.
   • Create a repeater item, notice how it appears empty, save and close the page. (if you added a second repeater item before saving it also would have been empty)
   • Now reopen the same page, switch back to the same language as before and add a new item to the repeater. This time it is pre-filled with values from the default language.

2. If you remove every item in one language of a repeater and save, the last item you removed will stay.

I already encountered 1. (or at least something close) before this PR so I think it's not a regression of this fix. That said, I don't realy know how to qualify this (bug or feature ?) because it's sometimes useful to copy the original content when translating.

For 2. it seems only related to repeater groups: I also tested with a simple repeater made of one text field and there's no problem with deleting the last item.
Here's how the DOM look like after deleting an item on a grouped repeater: http://take.ms/awz63
There's a long time since I last looked into the repeaters code but maybe it's a JS issue? (so not related to this fix)

[jimcottrell]

@Fl0Cri Thanks for giving this another set of eyes. I also ran into (2) when doing some further testing. That'll be applicable to the core change to getting the repeater save value through the child form widgets. I don't have a solution quite yet though. I'll look into (1) as well and see what's going on there. Thanks!

[jimcottrell]

@Fl0Cri I think I've worked out the issues that were occurring if you wouldn't mind taking another look at the latest versions of this and the core PR.

[Fl0Cri]

Ok, (1) seems to be gone with your fix. But (2) is still here for languages that are not the main language.

I've also found a variant of (1), let's call it (1.2). It occurs with nested repeaters and switching language from an other form control (holding ctrl key when switching the lang of the title for example). It seems to be fine when using directly the repeater's button. (After testing it again it's wrong. If you switch the language using the repeater lang switch and holding ctrl the problem will not occur. But if you don't hold ctrl with this switch it will)

Here's a video to show this bug and the repeaters deletion problem: http://take.ms/bDKyv

To reproduce 1.2:

• Create a static page that contains a group repeater and with one of the available groups containing a repeater (layout code below)
• Fill content in the main language so that the second entry is an instance of the group with the repeater (grid in my example)
• Save and switch to an other language from the title, holding the ctrl key. If you switch from the repeater itself the problem won't occur
• Add the first item of the repeater in the new language
• Save, close the page, reopen it and switch again to the secondary language (using the same way as above)
• Add an instance of the group containing the repeater and try to add subitems. You'll notice that the content from the main language is loaded. Strange fact is that the items seems to be offset (the first sub item in the secondary language is the second one from the main language)
• If you make the same operation again, don't forget to delete manually the file of the secondary language in /themes/.../content/static-pages-xx as it's not done automatically when deleting the main file.

Comments about 2:

• The main language is not affected by this bug (not sure if it was already the case before your fix)
• Removing sub items from a nested repeater in the secondary language is also ok
• Trying to remove the last item of the repeater discards all other removals (remove 1, 2 and save, both will come back. Remove 2, save, remove 1, and only 1 will stay)

My simplified layout I used for the video:

/themes/.../layouts/testRepeater.htm:

description = "Test repeater"

[localePicker]
forceUrl = 1

[staticPage]
useContent = 0
default = 0
==
{repeater name="sections" groups="themes/.../meta/sections-repeater.yaml" tab="Sections"}
    {# parser fail if there is noting between open and close tag #}
{/repeater}
<!DOCTYPE html>
<html>
    <head></head>
    <body>
        {% for section in sections %}
            <section>

                {% if section._group == 'simple' %}

                    <h2>{{ section.title }}</h2>
                    {{ section.content | raw }}

                {% elseif section._group == 'grid' %}

                    <h2>{{ section.title }}</h2>
                    <div class="row">
                        {% for block in section.blocks %}
                            <div class="col-md-6">
                                {{ block.content | raw }}
                            </div>
                        {% endfor %}
                    </div>

                {% endif %}

            </section>
        {% endfor %}
    </body>
</html>

/themes/.../meta/sections-repeater.yaml:

simple:
    name: Simple
    description: 'Full width text'
    icon: icon-paragraph
    fields:
        title:
            type: text
            label: Title
            span: left
        content:
            type: richeditor
            label: Content
            span: full

grid:
    name: Grid
    description: 'A grid of text blocks'
    icon: icon-paragraph
    fields:
        title:
            type: text
            label: Title
            span: left
        blocks:
            type: repeater
            label: Grid
            span: full
            form:
                fields:
                    content:
                        type: richeditor
                        label: Content

[Fl0Cri]

Small update: because I made a clean install to test #416 I've also made my tests again with these PR and I get the same results (tested with Chrome and FireFox).

Can someone reproduce it?

[LukeTowers]

This needs to be re-evaluated in lite of octobercms/october#4234 and #466