ASSETS-33747 named smartcrop in image core v3 component #2698
Conversation
…stly (#2697) * ASSETS-33747 named smartcrop in image core v3 component powered by fastly Instead of fastly smartcrops, expect named smartcrops in delivery URLs. * ASSETS-33747 named smartcrop in image core v3 component powered by fastly Add editor code to render image through delivery URL and support smartcrop. * @trivial fix linting errors. * Show smartcrop renditions group even if DM is disabled but polaris is enabled. * Update code to also show dynamicmedia group when dialog is loaded with remote asset. * Make dependency on clientbuilderfactory optional and check for null. This is to avoid rewriting/fixing all Image test cases. * ASSETS-33747 named smartcrop in image core v3 component powered by fastly Bug fixes. * Minor bug fixes. * Fix test cases. * @trivial Fix existing test cases. * ASSETS-33747 named smartcrop in image core v3 component Review comments. Add a separate test case for smartcrop. Add a failing testcase for auto smartcrop. * ASSETS-33747 named smartcrop in image core v3 component Incorporate review feedback. * @trivial Hide the smartcrop option if remote asset does not have smartcrops. * ASSETS-33747 named smartcrop in image core v3 component Add test cases for srcset and responsehandler code. * @trivial Move the code to show smartcrop dropdown only after fetching the metadata. * @trivial Fix eslint error. --------- Co-authored-by: Levente Sántha <levente@adobe.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2698 +/- ##
============================================
- Coverage 87.20% 87.19% -0.02%
- Complexity 2673 2683 +10
============================================
Files 233 235 +2
Lines 7114 7170 +56
Branches 1081 1097 +16
============================================
+ Hits 6204 6252 +48
- Misses 361 363 +2
- Partials 549 555 +6 ☔ View full report in Codecov by Sentry. |
| @@ -51,9 +51,6 @@ | |||
| text="Enable DM features" | |||
| uncheckedValue="false" | |||
| value="{Boolean}true"> | |||
| <granite:rendercondition jcr:primaryType="nt:unstructured" | |||
There was a problem hiding this comment.
I'm not sure if this is the way to go. This means that DM features would be always available even if no scene7 or NGDM is available. Is that what you want?
Probably you would need and other render condition for NGDM so that this feature is rendered when either DM or NGDM is available.
There was a problem hiding this comment.
@LSantha Is there a rendercondition to check for OSGI configuration's value in core components? Or any other way apart from renderconditions?
FWIW, removal of this rendercondition will only make the enable DM features checkbox visible in the design dialog of image component. It will not enable it by default. An author will have to update the template to enable the feature manually.
There was a problem hiding this comment.
@mohiaror , I don't think there is such render condition but you can create one.
I know this thing is about the design dialog but as you can see it was still provided with a render condition to avoid any confusion for customer not using this feature.
There was a problem hiding this comment.
@LSantha I have added an OR condition for both DM and NGDM. Please see if it looks Ok to you.
Incorporate review comments.
* fixed unit test failures
…stly Auto smartcrop for remote assets should not depend on whether widths is configured or not.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
...m/adobe/cq/wcm/core/components/internal/models/v3/NextGenDMSrcsetBuilderResponseHandler.java
Fixed
Show fixed
Hide fixed
.../main/java/com/adobe/cq/wcm/core/components/internal/servlets/NGDMEnableRenderCondition.java
Fixed
Show fixed
Hide fixed
|
mohiaror
changed the title
Instead of fastly smartcrops, expect named smartcrops in delivery URLs.
Add editor code to render image through delivery URL and support smartcrop.
@trivial fix linting errors.
Show smartcrop renditions group even if DM is disabled but polaris is enabled.
Update code to also show dynamicmedia group when dialog is loaded with remote asset.
Make dependency on clientbuilderfactory optional and check for null. This is to avoid rewriting/fixing all Image test cases.
ASSETS-33747 named smartcrop in image core v3 component powered by fastly
Bug fixes.
Minor bug fixes.
Fix test cases.
@trivial Fix existing test cases.
ASSETS-33747 named smartcrop in image core v3 component
Review comments. Add a separate test case for smartcrop. Add a failing testcase for auto smartcrop.
Incorporate review feedback.
@trivial Hide the smartcrop option if remote asset does not have smartcrops.
ASSETS-33747 named smartcrop in image core v3 component
Add test cases for srcset and responsehandler code.
@trivial Move the code to show smartcrop dropdown only after fetching the metadata.
@trivial Fix eslint error.
Fixes #1, Fixes #2