-
Notifications
You must be signed in to change notification settings - Fork 750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
solves #2625: Add feature to disable the featured page image inheritance #2626
base: main
Are you sure you want to change the base?
solves #2625: Add feature to disable the featured page image inheritance #2626
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2626 +/- ##
============================================
+ Coverage 87.21% 87.32% +0.10%
+ Complexity 2665 2653 -12
============================================
Files 233 232 -1
Lines 7123 7098 -25
Branches 1085 1080 -5
============================================
- Hits 6212 6198 -14
+ Misses 362 357 -5
+ Partials 549 543 -6 ☔ View full report in Codecov by Sentry. |
@michielspiritus , thank you for the contribution! I understand this is an important improvement for one of your customers. To better integrate it with the existing functionality, I believe the option of "Inherit featured image from page" should be removed (or at least disabled) when the inheritance is disabled in the policy. Also, to speed review and merge of this PR, may I kindly ask you to also provide an integration test? We already have tests covering the existing functionality at https://github.com/adobe/aem-core-wcm-components/blob/main/testing/it/e2e-selenium/src/test/java/com/adobe/cq/wcm/core/components/it/seljup/tests/image/v3/ImageIT.java |
@@ -344,9 +345,10 @@ public static Resource getWrappedImageResourceWithInheritance(Resource resource, | |||
String fileReference = properties.get(DownloadResource.PN_REFERENCE, String.class); | |||
Resource fileResource = resource.getChild(DownloadResource.NN_FILE); | |||
boolean imageFromPageImage = properties.get(PN_IMAGE_FROM_PAGE_IMAGE, StringUtils.isEmpty(fileReference) && fileResource == null); | |||
boolean disblePageImageInheritance = currentStyle != null ? currentStyle.get(Image.PN_PAGE_IMAGE_INHERITANCE_DISABLED, false) : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small typo in variable name, I believe it should be disablePageImageInheritance
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
I've wrote some IT tests but it's hard to test, always need to downgrade chrome. Can you review my latest changes? |
Quality Gate passedIssues Measures |
@vladbailescu any updates on this? |
Fixes #2625