Add Next Gen Dynamic Media (NGDM) support to the StaticImage component - #1965
Add Next Gen Dynamic Media (NGDM) support to the StaticImage component#1965pankaj-parashar wants to merge 1 commit into
Conversation
Ports the NGDM asset support from adobe/aem-core-wcm-components#2481 to the Adaptive Forms StaticImage (form/image v1) component. When the fileReference is an NGDM asset (/urn:...) and the NextGenDynamicMediaConfig OSGi service is enabled, the delivery URL is built directly instead of resolving a DAM rendition. Bumps aem-sdk-api to 2023.9.13665 to pick up the NextGenDynamicMediaConfig API, and enables the NGDM asset picker on the component's file upload dialog field. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Accessibility Violations Found
|
1 similar comment
Accessibility Violations Found
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
|
||
| public static final String DAM_REPO_PATH = "fd:repoPath"; | ||
|
|
||
| private static final String NGDM_REFERENCE_PREFIX = "/urn:"; |
There was a problem hiding this comment.
Can you extract this out as utils, lets not pollute image with so much code
| private static final String PATH_PLACEHOLDER_SEO_NAME = "{seo-name}"; | ||
| private static final String PATH_PLACEHOLDER_FORMAT = "{format}"; | ||
| private static final String DEFAULT_NGDM_ASSET_EXTENSION = "jpg"; | ||
| private static final int DEFAULT_NGDM_ASSET_WIDTH = 640; |
There was a problem hiding this comment.
why is 640 default and jpg default ?
| .replace(PATH_PLACEHOLDER_SEO_NAME, assetName) | ||
| .replace(PATH_PLACEHOLDER_FORMAT, assetExtension); | ||
|
|
||
| return "https://" + nextGenDynamicMediaConfig.getRepositoryId() + imageDeliveryPath |
There was a problem hiding this comment.
Should this be exposed as a URL in crispr json for headless delivery as well ?
There was a problem hiding this comment.
getImageSrc() is @JsonIgnored on the StaticImage interface, but getValue() is not, and getValue() delegates to getImageSrc() — so the NGDM-resolved URL already flows into the "value" field of the exported JSON (crispr) model. The PR's own test (testNgdmImage) already asserts image.getValue() equals the NGDM URL too
| sling:orderBefore="visible" | ||
| sling:resourceType="cq/gui/components/authoring/dialog/fileupload" | ||
| class="cq-droptarget" | ||
| enableNextGenDynamicMedia="{Boolean}true" |
There was a problem hiding this comment.
Should this be namespaced as fd:
There was a problem hiding this comment.
this exact, non-namespaced property name is read directly by the platform's own cq/gui/components/authoring/dialog/fileupload widget (/libs/cq/gui/components/authoring/dialog/fileupload/render.jsp:
final String ENABLE_NEXT_GEN_DYNAMIC_MEDIA = "enableNextGenDynamicMedia";),
not something we invented — it's a fixed contract the shared widget expects.
Namespacing it fd:enableNextGenDynamicMedia would break it silently (the widget just wouldn't see the flag, silently falling back to non-NGDM behavior)
Ports the NGDM asset support from adobe/aem-core-wcm-components#2481 to the Adaptive Forms StaticImage (form/image v1) component. When the fileReference is an NGDM asset (/urn:...) and the NextGenDynamicMediaConfig OSGi service is enabled, the delivery URL is built directly instead of resolving a DAM rendition. Bumps aem-sdk-api to 2023.9.13665 to pick up the NextGenDynamicMediaConfig API, and enables the NGDM asset picker on the component's file upload dialog field.
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: