adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
726 stars 735 forks source link

[Tabs] [Accordion] [Carousel] Panelselect doesn't work if proxy component adds new wrapper div #1390

Open dhardtke opened 3 years ago

dhardtke commented 3 years ago

Bug Report

Current Behavior Suppose I have the following template inside my proxy component tabs.html:

<div data-sly-include="/apps/core/wcm/components/tabs/v1/tabs/tabs.html" class="aem-tabs"></div>

This leads to the panelselect button to be missing inside the editor because of checks here and here.

Expected behavior/code The panelselect should still work like when the wrapper div is not present.

Environment

Possible Solution I would suggest ditching the whole "is one of the children of the editable a panel container type" check and rather search recursively, e.g.

getPanelContainerHTMLElement: function(editable) {
    var container = getPanelContainerType(editable);
    var element;

    if (container) {
        return $(editable.dom[0]).find(container.selector)[0];
    }

    return element;
},

(and since editable.dom is already a jQuery object we could call find directly on editable.dom, e.g. editable.dom.find(container.selector)[0])

Additional context / Screenshots It's about this button: image

bpauli commented 3 years ago

fixed in https://github.com/adobe/aem-core-wcm-components/commit/a33f85d26f78d5e0bdd3cfdffeacf3a40b8cf5f7

jckautzmann commented 3 years ago

I'm reopening this issue as it's not working for me. Here are the steps to reproduce the issue:

  1. in crxde, add a tabs.html file to /apps/core-components-examples/components/tabs
  2. add content to the file: <div data-sly-include=“/libs/core/wcm/components/tabs/v1/tabs/tabs.html” class=“aem-tabs”></div>
  3. go to http://localhost:4502/editor.html/content/core-components-examples/library/container/tabs.html

-> the panel select button is missing

dhardtke commented 3 years ago

Hi @jckautzmann,

the reason this is happening is that the changed JavaScript is looking for an element with [data-panelselect="tabs"] (see https://github.com/adobe/aem-core-wcm-components/blob/a712f27d780a6a8d18305af95c57e4ab64e2e054/content/src/content/jcr_root/apps/core/wcm/components/tabs/v1/tabs/clientlibs/editorhook/js/panelcontainer.js#L25 and https://github.com/adobe/aem-core-wcm-components/blob/a33f85d26f78d5e0bdd3cfdffeacf3a40b8cf5f7/content/src/content/jcr_root/apps/core/wcm/components/commons/editor/clientlibs/panelcontainer/v1/js/panelContainerUtils.js#L98) So for your example to work you would have to put e.g. <div data-sly-include="/apps/core/wcm/components/tabs/v1/tabs/tabs.html" class="aem-tabs" data-panelcontainer="tabs"></div> in your tabs.html.

Since neither editable.dom[0] (the wrapper div generated by the container) nor any of its direct children are matching [data-panelselect="tabs"] the panel select isn't registering.

We should document this in case another developer comes across this, maybe as part of https://github.com/adobe/aem-core-wcm-components/tree/master/content/src/content/jcr_root/apps/core/wcm/components/accordion/v1/accordion#enabling-accordion-editing-functionality (and similarly for carousel and tabs).