cq-component-maven-plugin icon indicating copy to clipboard operation
cq-component-maven-plugin copied to clipboard

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

Open d-wells opened this issue 6 years ago • 1 comments

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

d-wells avatar Jun 02 '19 23:06 d-wells

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

mitcoding avatar Oct 31 '22 11:10 mitcoding