Remove usage of cq:htmlTag for the image component

[kwin]

Currently the image components uses a cq:htmlTag node (https://docs.adobe.com/docs/en/aem/6-3/develop/components/components-basics.html#Properties%20and%20Child%20Nodes%20of%20a%20Component) in addition to the explicit markup being rendered in the HTL script. What is the reason for that and why is the additional surrounding div necessary? The image markup would be easier to overwrite in case only the HTL would actually render the markup, can't you just add the necessary classes to the explicit divs given in the HTL script (https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/master/content/src/content/jcr_root/apps/core/wcm/components/image/v1/image/image.html)?

[kwin]

The div with class cmp-image is used as selector in the javascript to place the actual image element at the right position. There is a complicated logic in https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/master/content/src/content/jcr_root/apps/core/wcm/components/image/v1/image/clientlibs/site/js/image.js#L184 which should IMHO be described, as this is important to know when

• you extend your own component from image, as the placement of the cmp-image class by default is on an additional surrounding div (which means, the image is always rendered as first component). This is relevant when the HTL script is overwritten, as there the assumption that the image should always be placed as first child of the cmp-image container does not always hold.
• using composition to render an image as part of your own component. In this case you don't have any container div with a "cmp-image" class which makes the javascript logic no longer work.

Either the need for the surrounding div should be removed at all (by always placing the image tag next to the noscript tag) or to render that div explicitly through the image's HTL script.

[raducotescu]

@kwin, for composed components you can use data-sly-resource with the AEM specific decorationTagName option. This should be used in combination with https://helpx.adobe.com/experience-manager/release-notes--aem-6-3-cumulative-fix-pack.html, where the rendering of the wapping div has been unified for all parsys components.

Regarding the purpose of the surrounding div - that's the way the editor knows what part of the markup represents a component. If you check the markup during editing you'll see more classes appended to the div than what the Core Components provide, but I'm sure you already know this.

The current form of the component only renders the div you mentioned while the wcmmode is enabled, which is why we pushed the cmp-image class into the cq:htmlTag node, since that surrounding div will always be rendered.

[kwin]

The current form of the component only renders the div you mentioned while the wcmmode is enabled, which is why we pushed the cmp-image class into the cq:htmlTag node, since that surrounding div will always be rendered.

I am not sure I understand. cq:htmlTag will be considered in any case, no matter what the wcmmode is. I would rather propose to move the class as explicit markup inside the htl script (may probably end up as an additional div which you may want to prevent) or get rid of it at all (by always placing the image above/below the noscript element). I know that you can override the decorationTagName in HTL or in your derived component but that can be easily forgotten. So it would be easier if the cq:htmlTag would not be used here. IMHO the only good reason for using cq:htmlTag is reducing the nesting level by being able to extend the parent component's div instead of writing it to the image component's markup itself.

[raducotescu]

"the div you mentioned" = https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/master/content/src/content/jcr_root/apps/core/wcm/components/image/v1/image/image.html#L16 - this one is removed if https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/master/content/src/content/jcr_root/apps/core/wcm/components/image/v1/image/image.html#L19

[kwin]

That is a misunderstanding. I am proposing to either

1. render an additional (not yet existing) div within the image's HTL which carries that class or
2. get rid of that class altogether and instead rely on the noscript element for positioning the image

IMHO classes which are added to the surrounding div should only be used for authoring capabilities but not for actual front-end logic, because then it is harder to reuse the component in different occasions.

[raducotescu]

There's a second reason for which those classes are appended to the cq:htmlTag node. Assuming you'd have a proxy component that needs also its own custom classes, but very little change regarding its behaviour, the cq:htmlTag node allows you to merge these classes, rather than to change HTL markup in the proxy.

All Core Components use this pattern, by the way. It's just that the Image Component also provides some JavaScript code that binds to the DOM.

[gabrielwalt]

As you said, the objective was to avoid nesting multiple DIVs with a wrapper element that is meaningless on publish as it would then be needed only for authoring, and instead to reuse that wrapper for the component's main class.

However, I don't understand however why this makes it harder to reuse the component in different occasions. As @raducotescu said, the wrapper element can be controlled quite flexibly from data-sly-resource, and, in the contrary, this allows another proxy component to append other classes to the wrapper element without duplicating the HTL markup.

[davidjgonzalez]

Im not sure where this issue landed, but it seems like for v2, you've landed on removing the cq:htmlTag and adding the to the actual HTML. Would this close https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/issues/85 ?

From what I see in the v2 sandbox this is a breaking change.

[kwin]

Indeed in v2 the image no longer relies on cq:html tag (https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/development/content/src/content/jcr_root/apps/core/wcm/sandbox/components/image/v2/image/image.html) and instead generates the full markup explicitly in the HTL script.

[davidjgonzalez]

This will be good to clearly articulate with the v2 release as a “sometimes breaking change” depending on how the css targeting core components HTML is constructed. Ironically the inconsistency of the breaking makes it more confusing than an always breaking change (IME).