[Tabs][Carousel] The Json export doesn't work properly for nested Tabs and Carousels

[andreeadracea]

The items elements are not exported properly in Json when having nested tabs.
(CQ-4253492)

[jckautzmann]

We should first define how the JSON should look like. I see the following options:

Option 1

We introduce a tabs object with the tab item specific properties (for now only the tab title).
The :items object consists of all the nested models JSON.

The JSON would look as follows:

{
    "tabs": [
        {
            "title": "Tab 1"
        },
        {
            "title": "Tab 2"
        }
    ],
    ":itemsOrder": [
        "item_1",
        "item_2"
    ],
    ":type": "weretail/components/content/tabs",
    ":items": {
        "item_1": {
            "columnCount": 12,
            "columnClassNames": {},
            "gridClassNames": "aem-Grid aem-Grid--12 aem-Grid--default--12",
            ":items": {},
            ":itemsOrder": [],
            ":type": "wcm/foundation/components/responsivegrid"
        },
        "item_2": {
            "columnCount": 12,
            "columnClassNames": {},
            "gridClassNames": "aem-Grid aem-Grid--12 aem-Grid--default--12",
            ":items": {},
            ":itemsOrder": [],
            ":type": "wcm/foundation/components/responsivegrid"
        }
    }
}

Advantages:

• minimal implementation effort
• tab item specific properties are nicely isolated from the other nested models JSON.

Disadvantages:

• it might be confusing to read: which title belongs to which item?

Option 2

We introduce a specific node for the tab item which:

• stores the tab item specific properties (today only the title).
• has a specific resource type (e.g. core/wcm/components/tabs/v1/tabs/item) and a dedicated model

The JCR structure would look as follows:

tabs
    item1
        responsivegrid
            text
            image
…

The JSON would look as follows:

{
    ":itemsOrder": [
        "item_1",
        "item_2"
    ],
    ":type": "weretail/components/content/tabs",
    ":items": {
        "item_1": {
            ":type": "weretail/components/content/tabs/item",
            "title": "Tab 1",
            "container": {
                "columnCount": 12,
                "columnClassNames": {},
                "gridClassNames": "aem-Grid aem-Grid--12 aem-Grid--default--12",
                ":items": {},
                ":itemsOrder": [],
                ":type": "wcm/foundation/components/responsivegrid"
            }
        },
        "item_2": {
            ":type": "weretail/components/content/tabs/item",
            "title": "Tab 2",
            "container": {
                "columnCount": 12,
                "columnClassNames": {},
                "gridClassNames": "aem-Grid aem-Grid--12 aem-Grid--default--12",
                ":items": {},
                ":itemsOrder": [],
                ":type": "wcm/foundation/components/responsivegrid"
            }
        }
    }
}

Advantages:

• tab item specific properties are nicely isolated from the other models
• easy to read and to find out which title belongs to which item
• nicely maps the JCR structure

Disadvantages:

• big implementation effort

WDYT?

[vladbailescu]

Another option would be to wrap the exported items in a class that adds the extra properties we need and inline/unwrap the original exported model.

The JSON would look like

{
    [..],
    ":itemsOrder": [
        "item_1",
        "item_2"
    ],
    ":type": "weretail/components/content/tabs",
    ":items": {
        "item_1": {
            // These are coming from the wrapper
            "title": "Tab 1",
            // These are coming from the wrapped model
            "columnCount": 12,
            "columnClassNames": {},
            "gridClassNames": "aem-Grid aem-Grid--12 aem-Grid--default--12",
            ":items": {},
            ":itemsOrder": [],
            ":type": "wcm/foundation/components/responsivegrid"
        },
        "item_2": {
            "title": "Tab 2",
            "columnCount": 12,
            "columnClassNames": {},
            "gridClassNames": "aem-Grid aem-Grid--12 aem-Grid--default--12",
            ":items": {},
            ":itemsOrder": [],
            ":type": "wcm/foundation/components/responsivegrid"
        }
    }
}

We could use @JsonUnwrapped to inline wrapped model properties.

[gabrielwalt]

TLDR: I'm for Option 3.

Option 1
I like the simplicity of this option, but the way the tabs and the items work is not very consistent: one is an array and the other an object. Would it make sense that we use the same indexes for the tabs?

{
    ":itemsOrder": [
        "item_1",
        "item_2"
    ],
    ":items": {
        "item_1": {...},
        "item_2": {...}
    },
    "tabs": {
        "item_1": {
            "title": "Tab 1"
        },
        "item_2": {
            "title": "Tab 2"
        }
    ],
    ":type": "weretail/components/content/tabs"
}

Option 2
I like the clarity of this option, but it's adding one more resource type. It would require customers to create a relatively useless proxy component just for the items. Would it make sense to skip that resource type property?

{
    ":itemsOrder": [
        "item_1",
        "item_2"
    ],
    ":items": {
        "item_1": {
            "title": "Tab 1",
            "container": {...}
        },
        "item_2": {
            "title": "Tab 2",
            "container": {...}
        }
    },
    ":type": "weretail/components/content/tabs"
}

Option 3
I like the compromise between simplicity and clarity, and it's consistent with the fact that the Tabs are adding the title property to the Layout Container component. Theoretically, it should be the model of the Layout Container that should provide the title in its JSON, but for the components that don't provide a title, it's good if the model of the Tabs can inject that. However, that should only happen when the component's model doesn't output any title, in case the component's model implemented some business logic for the title.

Finally, we should also apply the same architectural concept to the Carousel component.

[jckautzmann]

The following commit implements option 3 with item specific properties (itemTitleand itemPath):
adb1ecb