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

[Accordion][Carousel][Tabs] ./cq:isContainer="true" is in String format, should be Boolean #732

Closed surya057 closed 4 years ago

surya057 commented 4 years ago

Bug Report

Current Behavior Current "cq:isContainer" property of the Carousel component is a String type.

Expected behavior/code This property should be Boolean

Environment

Possible Solution Change the property type to boolean.

Additional context / Screenshots Add any other context about the problem here. If applicable, add screenshots to help explain.

surya057 commented 4 years ago

My mistake please ignore this. I will check and get back.

surya057 commented 4 years ago

Hi, Adobe help doc says this property is the type of string. But when I try to add this property to my component, I am unable to add this property. Getting an error message saying "property must be of type Boolean" in crx/de. Can someone please clarify this?.

image

richardhand commented 4 years ago

@surya057,

Adobe help doc says this property is the type of string

Could you please post the help document you are referring to, according to [0] they should be Boolean as you describe.

Change the property type to boolean.

Agree, but we should update all components that use this property: Accordion, Carousel, Tabs, and the Component Library Demo component.

Feel free to create a pull request for this, it should be a fairly simple change: cq:isContainer="true" -> cq:isContainer="{Boolean}true"

[0] - https://helpx.adobe.com/experience-manager/6-5/sites/developing/using/custom-nodetypes.html

surya057 commented 4 years ago

Yes, I will share PR. This is the helpx doc i mentioned - https://helpx.adobe.com/uk/experience-manager/6-3/sites/developing/using/components-basics.html.

Thank you @richardhand

richardhand commented 4 years ago

@bohnertchris - seems like these docs should be in line with each other: • https://helpx.adobe.com/uk/experience-manager/6-5/sites/developing/using/components-basics.htmlhttps://helpx.adobe.com/experience-manager/6-5/sites/developing/using/custom-nodetypes.html

with regard to the ./cq:isContainer format being Boolean.

surya057 commented 4 years ago

Pull request for this change - https://github.com/adobe/aem-core-wcm-components/pull/734

msagolj commented 4 years ago

verified on master branch

bohnertchris commented 4 years ago

@bohnertchris - seems like these docs should be in line with each other: • https://helpx.adobe.com/uk/experience-manager/6-5/sites/developing/using/components-basics.htmlhttps://helpx.adobe.com/experience-manager/6-5/sites/developing/using/custom-nodetypes.html

with regard to the ./cq:isContainer format being Boolean.

Thanks for pointing this out, @richardhand. This was addressed in CQDOC-14959 and the corrections are now live, but might take a bit to propagate to the 6.4 content.