Open HitmanInWis opened 5 months ago
The fix for this one is quite a chore - basically the JS code for toggling the DM features on/off pays no heed to the page image checkbox, currently, so not only do we need to update the activity on initial load of the dialog, but we also need to handle the case of a user toggling between a selected image and the page image while the dialog is open.
The solution starts with exctracting the code for toggling the DM controls from retrieveDAMInfo
so that it's reusable:
/**
* Show or hide "DynamicMedia section" depending on whether the file is DM.
*/
function toggleDynamicMediaControls() {
var dmImagePath = imageFromPageImage && imageFromPageImage.checked ? $pageImageDmPath : $damAssetDmPath;
// show or hide "DynamicMedia section" depending on whether the file is DM
if (!dmImagePath || dmImagePath.trim() === "" || !areDMFeaturesEnabled) {
$dynamicMediaGroup.hide();
} else {
$dynamicMediaGroup.show();
getSmartCropRenditions(dmImagePath);
}
}
Note that this code choses between two different image DM paths, one for the page image, and one for the DAM image, since both can be present on the dialog. $damAssetDmPath
can be saved on retrieveDAMInfo
but since retrieveDAMInfo
is not called for the page image we must also update the callback in updateImageThumbnail
with the following code:
$pageImageFileReference = $pageImageThumbnail.find(".cq-FileUpload-edit").data("cqFileuploadFilereference");
if ($pageImageFileReference && $pageImageFileReference !== "") {
$.ajax({
url: $pageImageFileReference + "/_jcr_content/metadata.json"
}).done(function(data) {
if (data) {
$pageImageDmPath = data["dam:scene7File"];
if (imageFromPageImage && imageFromPageImage.checked) {
toggleDynamicMediaControls();
}
}
});
}
Once that is complete, the following locations need to call toggleDynamicMediaControls()
retrieveDAMInfo
where code was extracted from (call only if page image is NOT checked)updateImageThumbnail
(see code above, call only if page image IS checked)togglePageImageInherited
$cqFileUpload.on("click", "[coral-fileupload-clear]"
listenerAlso, since the variables in image.js
are not scoped to an object, they must be reset to null
each time a dialog is opened, to avoid stale values from a previous dialog:
$pageImageFileReference = null;
$pageImageDmPath = null;
$damAssetDmPath = null;
Last but not least, we should update the ImageImpl
sling model to be more defensive on how it applies Image Preset or Smart Crop Renditions. Currently it fetches both imagePreset
and smartCropRendition
from the JCR w/o any regard to the selected dmPresetType
value. If ever the image dialog isnt perfect at resetting field values on change of asset or toggling of the page image checkbox on/off, the generated URL will be wrong. The sling model should instead avoid injecting imagePreset
and smartCropRendition
, instead using the following code:
@ValueMapValue(injectionStrategy = InjectionStrategy.OPTIONAL)
@Nullable
private String dmPresetType;
@Nullable
private String imagePreset;
@Nullable
private String smartCropRendition;
...
if ("imagePreset".equals(dmPresetType)) {
imagePreset = properties.get(PN_IMAGE_PRESET, String.class);
} else if ("smartCrop".equals(dmPresetType)) {
smartCropRendition = properties.get(PN_SMART_CROP_RENDITION, String.class);
}
Can confirm I've run into this issue as well on a recent project. If I remember correctly, we found a "workaround" which unfortunately involved some manual authoring steps. I believe the workaround was to unselect the 'Inherit featured image from page' dialog option, then add your image directly into the dialog which makes the DM options appear, then select your DM options, then re-select the 'Inherit featured image from page' option, and the DM settings will still be applied. Can't say we were happy with that workaround 😃
Bug present as of version: 2.24.7-SNAPSHOT
When an Image component references a page image, and that page image is a Dynamic Media image, the dialog options for configuring Dynamic Media image modifiers/presets/smartcrop/etc. are not displayed to the user. The image does, however, render on the page with a DM URL.