adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
747 stars 753 forks source link

[Container] Create a Container v2 with the correct JSON output #1407

Open stefanseifert opened 3 years ago

stefanseifert commented 3 years ago

The JSON output of the Container v1 looks as follows (as it inherits from the Responsive Grid component JSON):

{
    "columnClassNames": {
        "responsiveimage_933266850": "aem-GridColumn aem-GridColumn--default--12",
        "text_1649938976": "aem-GridColumn aem-GridColumn--default--12",
        "text": "aem-GridColumn aem-GridColumn--default--12",
        "title": "aem-GridColumn aem-GridColumn--default--12"
    },
    "columnCount": 12,
    "gridClassNames": "aem-Grid aem-Grid--12 aem-Grid--default--12",
    "allowedComponents": {
        "applicable": false,
        "components": < ... ]
    },
    ":items": { ... },
    ":itemsOrder": [ ... ],
    ":type": "myapp/components/container"
}
{code}

As some applications might rely on this JSON, we decided to:
* keep the JSON output as it is now for container v1 
* create a new version of the Container component (v2) and adapt the JSON output as follows:

**Simple Container:**
{code}
{
    "id": "container-09eae90b3e",
    "layout": "SIMPLE",
    ":items": { ... },
    ":itemsOrder": [ ... ],
    ":type": "myapp/components/container"
}
{code}

**Responsive Container:**
{code}
{
    "id": "container-09eae90b3e",
    "layout": "RESPONSIVE_GRID",
    "columnClassNames": {
        "responsiveimage_933266850": "aem-GridColumn aem-GridColumn--default--12",
        "text_1649938976": "aem-GridColumn aem-GridColumn--default--12",
        "text": "aem-GridColumn aem-GridColumn--default--12",
        "title": "aem-GridColumn aem-GridColumn--default--12"
    },
    "columnCount": 12,
    "gridClassNames": "aem-Grid aem-Grid--12 aem-Grid--default--12",
    ":items": { ... },
    ":itemsOrder": [ ... >,
    ":type": "myapp/components/container"
}
antoantonyk commented 3 years ago

@stefanseifert I really wanted to have the exact JSON model like you mentioned here. I tried to implement the changes based on your PR. But now it is breaking the responsive layout, because the responsive layout component properties(columnClassNames, columnCount, gridClassNames) are missing. Is there any way to include these as well in addition to container properties?

cshawaus commented 3 years ago

Hi,

I also come across this problem on a work project and until last night couldn't see a solid path forward without reimplementing lots of private code. However, I stumbled upon ResponsiveGridExporter which is what ResponsiveGrid uses for its own implementation.

Last night I worked on this and found a way to maintain the responsive grid properties while allowing for custom properties to be available in the model output. The link below shows how the model needs to be structured.

https://github.com/cshawaus/sling-delegation-demo/blob/main/core/src/main/java/com/slingdelegation/core/models/CustomContainerModel.java#L29-L43

There are 3 important things happening in my approach:

  1. Explicitly declare @Exporter to ensure Jackson always generates JSON for our model
  2. Have our model extend the built-in ResponsiveGrid class which has all the private goodies we want
  3. Uses a custom exporter (CustomContainerExporter) which extends ResponsiveGridExporter

Essentially, what I have achieved with this approach is backwards compatibility with AEM's responsive grid while ensuring Sling Delegation remains functional with LayoutContainer. And like magic, the below image shows myCustomProperty available in the default output provided by AEM.

image

~Now, with anything there are caveats and so far I have only found one which is SPA root models fail to resolve child :items and always returns empty. This appears to be an issue with Sling Model Factories in HierarchyPageImpl and I currently have a bug ticket open but need to add an SPA example to my demo repo.~

~As long as you aren't expecting to use the SPA, this solution will work perfectly fine and based on my testing doesn't introduce any degradation to performance. Hopefully this helps you move forward as it certainly solved 50% of my issues after weeks of no progress.~

Update for SPA: I have found a solution that ensures both normal and AEM SPA use remain functional. https://github.com/cshawaus/sling-delegation-demo/commit/c721125e176fc6d4ba2fa0166477ec1275a74221

jckautzmann commented 3 years ago

@adobe export issue to Jira project SITES as Story

github-jira-sync-bot commented 3 years ago

:white_check_mark: Jira issue SITES-3332 is successfully created for this GitHub issue.

jckautzmann commented 3 years ago

The JSON should be as follows for a simple container:

{
    "id": "container-09eae90b3e",
    "layout": "SIMPLE",
    ":items": { ... },
    ":itemsOrder": [ ... ],
    ":type": "myapp/components/container"
}

I see following options how the JSON should look like for a responsive grid container?

Option#1:

{
    "id": "container-09eae90b3e",
    "layout": "RESPONSIVE_GRID",
    "columnClassNames": {
        "responsiveimage_933266850": "aem-GridColumn aem-GridColumn--default--12",
        "text_1649938976": "aem-GridColumn aem-GridColumn--default--12",
        "text": "aem-GridColumn aem-GridColumn--default--12",
        "title": "aem-GridColumn aem-GridColumn--default--12"
    },
    "columnCount": 12,
    "gridClassNames": "aem-Grid aem-Grid--12 aem-Grid--default--12",
    "allowedComponents": {
        "applicable": false,
        "components": [ ... ]
    },
    ":items": { ... },
    ":itemsOrder": [ ... ],
    ":type": "myapp/components/container"
}

Option#2:

{
    "id": "container-09eae90b3e",
    "layout": "RESPONSIVE_GRID",
    ":items": { ... },
    ":itemsOrder": [ ... ],
    ":type": "myapp/components/container"
}

IMHO option#1 makes the most sense as it has the basic properties of a simple container and adds the responsive grid specific properties. @stefanseifert @gabrielwalt @vladbailescu WDYT?

jckautzmann commented 3 years ago

After discussing with our team, we decided to:

Simple Container:

{
    "id": "container-09eae90b3e",
    "layout": "SIMPLE",
    ":items": { ... },
    ":itemsOrder": [ ... ],
    ":type": "myapp/components/container"
}

Responsive Container:

{
    "id": "container-09eae90b3e",
    "layout": "RESPONSIVE_GRID",
    "columnClassNames": {
        "responsiveimage_933266850": "aem-GridColumn aem-GridColumn--default--12",
        "text_1649938976": "aem-GridColumn aem-GridColumn--default--12",
        "text": "aem-GridColumn aem-GridColumn--default--12",
        "title": "aem-GridColumn aem-GridColumn--default--12"
    },
    "columnCount": 12,
    "gridClassNames": "aem-Grid aem-Grid--12 aem-Grid--default--12",
    ":items": { ... },
    ":itemsOrder": [ ... ],
    ":type": "myapp/components/container"
}
jckautzmann commented 3 years ago

Following code can be used as a starting point: https://github.com/adobe/aem-core-wcm-components/compare/development...issue/1407