Smart crop Assets block powered by DMwOA #11
Conversation
|
I think we can directly use the |
|
@sdmcraft updated the PR. feel free to review again |
|
@anuraggupta228 , as we discussed offline, I think it may be worthwhile to make "smartcrop" a block/section/page level modifier instead of a block. And then for each entity (whether page / section / block) having that identifier, decorate its images to use smart crop |
…omain/page/section/block level
scripts/aem-assets.js
Outdated
| // check if smartcrop should be applied by checking page level | ||
| // meta tag or class attribute or section level data attribute | ||
| const renderImgSmartCrop = window.hlx?.aemassets?.smartCrops | ||
| && (hasDMImageSmartcropMeta() || ele.classList?.contains('dm-image-smartcrop') |
There was a problem hiding this comment.
| && (hasDMImageSmartcropMeta() || ele.classList?.contains('dm-image-smartcrop') | |
| && (hasDMImageSmartcropMeta() || ele.classList?.contains('smartcrop') |
scripts/aem-assets.js
Outdated
|
|
||
| function hasDMImageSmartcropMeta() { | ||
| const metaTags = document.getElementsByTagName('meta'); | ||
| const matchingMeta = Array.from(metaTags).find((meta) => meta.name === 'dm-image-smartcrop' && meta.content === 'true'); |
There was a problem hiding this comment.
| const matchingMeta = Array.from(metaTags).find((meta) => meta.name === 'dm-image-smartcrop' && meta.content === 'true'); | |
| const matchingMeta = Array.from(metaTags).find((meta) => meta.name === 'smartcrop' && meta.content === 'true'); |
scripts/aem-assets.js
Outdated
| // meta tag or class attribute or section level data attribute | ||
| const renderImgSmartCrop = window.hlx?.aemassets?.smartCrops | ||
| && (hasDMImageSmartcropMeta() || ele.classList?.contains('dm-image-smartcrop') | ||
| || ele.getAttribute('data-dm-image-smartcrop') === 'true'); |
There was a problem hiding this comment.
| || ele.getAttribute('data-dm-image-smartcrop') === 'true'); | |
| || ele.getAttribute('data-smartcrop') === 'true'); |
scripts/aem-assets.js
Outdated
| const extImages = ele.querySelectorAll('a'); | ||
| extImages.forEach((extImage) => { | ||
| if (isExternalImage(extImage, deliveryMarker)) { | ||
| const extImageSrc = extImage.getAttribute('href'); | ||
| const extPicture = createOptimizedPicture(extImageSrc); | ||
|
|
||
| // except the samrtcrop param in <img>, |
There was a problem hiding this comment.
| // except the samrtcrop param in <img>, | |
| // except the smartcrop param in <img>, |
scripts/aem-assets.js
Outdated
|
|
||
| // This would potentially repalce all exisiting picture tag with smartcrop loaded picture tag | ||
| // without this check, it would not account for block level smartcrop flag | ||
| if (renderImgSmartCrop) { |
There was a problem hiding this comment.
I think here we are doing it too late and call to createOptimizedPicture can be avoided if smart crop is to be done. Can we check renderImgSmartCrop in the beginning itself and invoke
if smartcrop
createOptimizedPictureWithSmartcrop
else
createOptimizedPicture
There was a problem hiding this comment.
the code for decorateExternalImages() is managed with 2 parts -
decorateExternalImages(ele, deliveryMarker) {
const renderImgSmartCrop = // checks for renderImgSmartCrop to be render or not;
// PART : 1 (checking with all <a> tag)
// convert all <a> tag to <picture> tag
// PART : 2 (checking with all <picture> tag)
// when any block/section explicitly call for decorateExternalImages() by setting at section/block level metadata then we'll need to check with all exisiting <picture> tag too in the HTML and recreate it with smartcrop
}
the beahvaiour to note here is that block/section code comes into picture after page load so when any block calls for decorateExternalImages() then by that time all <a> would have been already converted to <picture> tag leaving smartcrop unaccounted as on page load ele remains 'main'.
and I think it won't make much of a diff we all part1 or part2 first. wdyt?
Fixes : https://github.com/adobe-rnd/aem-assets-plugin/issues
Demo pages: