aem-core-wcm-components icon indicating copy to clipboard operation
aem-core-wcm-components copied to clipboard

Next Gen DM Dialog unexpectedly using InheritedField model

Open HitmanInWis opened this issue 1 year ago • 6 comments

Version: 2.25.5-SNAPSHOT

Perhaps not a "bug" if nothing is broken, but nextgendmthumbnail.html is adapting the request to the completely unrelated com.adobe.cq.wcm.core.components.commons.editor.dialog.inherited.InheritedField sling model just to be able to get a attrClass attribute. This sling model has a very specific use for a field that inherits its value from a parent and allows for overrides (the "Brand Slug" field on page properties). Using it here is very unexpected, and could easily cause a problem in the future if InheritedField is refactored.

To resolve this, all we need to do is add getAttrClass to the NextGenDMThumbnail interface and impl...very simple update

    @ValueMapValue(name = "granite:class", injectionStrategy = InjectionStrategy.OPTIONAL)
    private String attrClass;

    @Override
    public @Nullable String getAttrClass() {
        return attrClass;
    }

HitmanInWis avatar Jun 26 '24 21:06 HitmanInWis

This is now stale code and will be removed from core-components soon.

mohiaror avatar Jun 27 '24 05:06 mohiaror

Next Gen DM support being removed? or updated? Can you tell me more?

HitmanInWis avatar Jun 27 '24 14:06 HitmanInWis

No just this specific smartcrop functionality has now been replaced with a new one. The same functionality you get along with classic DM.

mohiaror avatar Jul 01 '24 21:07 mohiaror

ok, so then does that make all of the NGDM stuff in WCM Core unnecessary (i.e. can be removed)?

HitmanInWis avatar Jul 02 '24 12:07 HitmanInWis

Not all NGDM. Just the smartcrop dialog which was powered by fastly. It has been replaced with the smartcrop which is already present in dynamic media enabled AEM environment. I have raised a PR to remove the dead code - https://github.com/adobe/aem-core-wcm-components/pull/2781

mohiaror avatar Jul 02 '24 13:07 mohiaror

Thank you! We can close this issue out as soon as #2781 is merged!

HitmanInWis avatar Jul 02 '24 23:07 HitmanInWis