icfnext / cq-component-maven-plugin

Other
22 stars 35 forks source link

useCoral3Dialogs=true not compatible with @Tab(touchUIPath=...) #84

Open d-wells opened 5 years ago

d-wells commented 5 years ago

When setting useCoral3Dialogs=true configuration for the cq-component-maven-plugin, existing Tab inclusions, via @Tab(touchUIPath=...) stop working.

<plugin>
    <groupId>com.citytechinc.cq.cq-component-plugin</groupId>
    <artifactId>cq-component-maven-plugin</artifactId>
    <version>${component.plugin.version}</version>
    <extensions>true</extensions>
    <executions>
        <execution>
            <goals>
                <goal>component</goal>
            </goals>
        </execution>
    </executions>
    <configuration>
        <componentPathBase>jcr_root/apps/company/components</componentPathBase>
        <defaultComponentGroup>Company Name</defaultComponentGroup>
        <transformerName>lower-case</transformerName>
        <useCoral3Dialogs>true</useCoral3Dialogs>
        <additionalFeatures>
            <additionalFeature>rte-touchui</additionalFeature>
        </additionalFeatures>
    </configuration>
</plugin>

The issue is that the path property does not get written to the xml node corresponding to the item node with sling:resourceType granite/ui/components/foundation/include, e.g. (with 6.4.1-SNAPSHOT)

<items jcr:primaryType="nt:unstructured">
      <tabs jcr:primaryType="nt:unstructured" maximized="{Boolean}true" sling:resourceType="granite/ui/components/coral/foundation/tabs">
        <items jcr:primaryType="nt:unstructured">
          <basic cq:hideOnEdit="{Boolean}false" cq:showOnCreate="{Boolean}true" jcr:primaryType="nt:unstructured" margin="{Boolean}true" maximized="{Boolean}false" sling:resourceType="granite/ui/components/foundation/include"/>
        </items>
      </tabs>
</items>

I've confirmed this behavior in 6.0.0, 6.1.0, 6.4.0, and develop (6.4.1-SNAPSHOT).

Issue appears to have been introduced in com.citytechinc.cq.component.touchuidialog.layout.tabs.TabsLayoutCoral3Maker, commit # e4989ad10f6febc3f461a7c2ccc23359f945b7b1, with the change from

// Create all Tabs
List<FixedColumnsLayoutElement> tabs = new ArrayList<FixedColumnsLayoutElement>();
for (FixedColumnsLayoutElementParameters currentLayoutElementParams : tabParametersList) {
    if (currentLayoutElementParams != null) {
        tabs.add(new FixedColumnsLayoutElement(currentLayoutElementParams));
    }
}

to

// Create all Tabs
for (ContainerParameters tabContainerParameters : tabContainerParametersList) {
    if (tabContainerParameters != null) {
        tabs.add(new Container(tabContainerParameters));
    }
}

FixedColumnsLayoutElement class includes a getPath() method. Container class does not have a getPath() method.

That method is critical for the XMLWriter when determining what xml attributes / node properties are applicable for the current element.

Based on the last several months of commits, it looks like there's an effort to move completely to containers for 6.3+, due to the increased coral3 dialog usage, so instead of suggesting a reversion of e4989ad10f6febc3f461a7c2ccc23359f945b7b1, I'm suggesting an update to com.citytechinc.cq.component.touchuidialog.container.Container and com.citytechinc.cq.component.touchuidialog.container.Section.

See https://github.com/d-wells/cq-component-maven-plugin/pull/1/commits

mitcoding commented 1 year ago

I'm submitting a new pull request as D-Wells original suggestion has vanished from the internet. I don't know what d-wells specifically changed but I'm assuming the changes I've put in is pretty close to what D-Wells suggested, based on what I could clean from d-wells' comment above without ever seeing the specific changes.

https://github.com/icfnext/cq-component-maven-plugin/pull/94/commits