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
723 stars 737 forks source link

Next Gen DM Dialog unexpectedly using InheritedField model #2800

Open HitmanInWis opened 5 days ago

HitmanInWis commented 5 days ago

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;
    }
mohiaror commented 5 days ago

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

HitmanInWis commented 5 days ago

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

mohiaror commented 23 hours ago

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

HitmanInWis commented 8 hours ago

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

mohiaror commented 7 hours ago

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