aem-core-wcm-components icon indicating copy to clipboard operation
aem-core-wcm-components copied to clipboard

Bugs with Image component referencing Page featured image

Open HitmanInWis opened this issue 9 months ago • 4 comments

Bugs current as of: 2.24.7-SNAPSHOT

  • When image component references a page image, "alternative text for accessibility" on dialog loads as empty unless you uncheck/check the "alternative text for accessibilty" box
  • When image compnoent references a page image, "alternative text for accessibility" can be saved as empty
  • When you add a Link to an image component when inheriting featured image from the page, a JS error prevents from saving "Alternative text for acessibility" even if a value is in the field
  • On the Image policy dialog, a JS error causes "Get alternative text from DAM" selection to be hidden until a different checkbox is clicked

HitmanInWis avatar Apr 30 '24 17:04 HitmanInWis

For the bug: "On the Image policy dialog, a JS error causes "Get alternative text from DAM" selection to be hidden until a different checkbox is clicked"

The fix for this one is simple. https://github.com/adobe/aem-core-wcm-components/pull/2249 was the commit that added the Image editor JS lib to the design dialog (which causes the error), but https://github.com/adobe/aem-core-wcm-components/pull/2288 fully reverted the JS changes that triggered the need to add the lib to the design dialog in the first place.

As such, I believe we can simply remove extraClientlibs="[core.wcm.components.image.v3.editor]" from the Image design dialog, resolving this bug.

HitmanInWis avatar May 31 '24 18:05 HitmanInWis

NOTE: This one may now be resolved by https://github.com/adobe/aem-core-wcm-components/pull/2790/files

For the bug: "When you add a Link to an image component when inheriting featured image from the page, a JS error prevents from saving "Alternative text for acessibility" even if a value is in the field"

The issue is in apps/core/wcm/components/image/v3/image/clientlibs/editor/js/image.js. What's happening is the logic that is added to display a nice error message for missing alt text ("Error: Please provide an asset which has a description that can be used as alt text.") is checking the wrong checkbox when using a page image. When using a page image, if the (hidden) checkbox for "altValueFromDAM" is checked, the validation think you're still trying to default from DAM value and throws an error for that value being empty, regardless of you having entered your own alt text.

The following code:

        validate: function() {
            var seededValue = $(altInputSelector).attr("data-seeded-value");
            var isAltCheckboxChecked = document.querySelector('coral-checkbox[name="./altValueFromDAM"]').checked;
            var isDecorativeChecked = document.querySelector("coral-checkbox[name='./isDecorative']").checked;
            var assetWithoutDescriptionErrorMessage = "Error: Please provide an asset which has a description that can be used as alt text.";
            if (isAltCheckboxChecked && !seededValue && !isDecorativeChecked) {
                return Granite.I18n.get(assetWithoutDescriptionErrorMessage);
            }
        }

Needs to be updated to have isAltCheckboxChecked choose appropriately between altValueFromDAM and altValueFromPageImage based on whether the component is using a page image or not.

HitmanInWis avatar May 31 '24 19:05 HitmanInWis

For the bug: "When image component references a page image, "alternative text for accessibility" on dialog loads as empty unless you uncheck/check the "alternative text for accessibilty" box"

The issue is in apps/core/wcm/components/image/v3/image/clientlibs/editor/js/image.js. After the dialog loads and the thumbnail is updated to show the referenced page image, there is logic that hides the checkbox for "get alt text from DAM" (altTuple checkbox in code). When that checkbox is hidden, the ./alt field value is reset to the original value of of the field (i.e. emtpy). Since this text field is the same one used by the altFromPageTuple checkbox, it unintentionally clears the value that should be present for when we are getting the alt text from the page image.

To remedy this, the following code snippet should be updated to re-update the ./alt field after the altTuple checkbox is hidden:

            updateImageThumbnail().then(function() {
                $cqFileUploadEdit = $dialog.find(".cq-FileUpload-edit[trackingelement='edit']");
                if ($cqFileUploadEdit) {
                    fileReference = $cqFileUploadEdit.data("cqFileuploadFilereference");
                    if (fileReference === "") {
                        fileReference = undefined;
                    }
                    if (fileReference) {
                        retrieveDAMInfo(fileReference);
                    } else {
                        altTuple.hideCheckbox(true);
                        altFromPageTuple.update(); // ADD THIS LINE
                        captionTuple.hideCheckbox(true);
                    }
                }
            });

HitmanInWis avatar May 31 '24 21:05 HitmanInWis

For the bug: "When image component references a page image, "alternative text for accessibility" can be saved as empty"

There's a couple issues, which I think stem from https://github.com/adobe/aem-core-wcm-components/pull/1995/files which added a check to verify the fileupload field is visible. The "required" value for the alt field is incorrect in two scenarios:

  • On initial dialog load with "image from page" checked
  • On checking the "image from page" checkbox after opening the dialog with no image specified

To fix the issue on initial dialog load, we can add a call in updateImageThumbnail to toggle the alt fields in the promise

        // Get the updated page image thumbnail HTML from the server
        return $.ajax({
            url: thumbnailConfigPath + ".html" + thumbnailComponentPath,
            data: {
                "pageLink": linkValue
            }
        }).done(function(data) {
            toggleAlternativeFieldsAndLink(imageFromPageImage, isDecorative); // ADD THIS
            if (data) {

To fix the issue on toggle of the "image from page" checkbox, update togglePageImageInherited

    function togglePageImageInherited(checkbox, isDecorative) {
        if (checkbox) {
            // toggleAlternativeFields(checkbox, isDecorative); REMOVE THIS LINE
            if (checkbox.checked) {
                $cqFileUpload.hide();
                $pageImageThumbnail.show();
            } else {
                $cqFileUpload.show();
                $pageImageThumbnail.hide();
            }
            toggleAlternativeFieldsAndLink(checkbox, isDecorative); // ADD THIS, *AFTER* UPLOAD FIELD IS SHOWING
        }
    }

There may be more ideal/elegant fixes, but this seems to work for me.

HitmanInWis avatar Jun 03 '24 13:06 HitmanInWis