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
734 stars 739 forks source link

[Tab][Accordion][Carousel] Layout Grid does not inherit from parent #1133

Open nsamerctl opened 4 years ago

nsamerctl commented 4 years ago

Bug Report

Current Behavior When adding container/components inside Tab Component the grid layout of inner container/component is not inheriting from parent component (Tab component), instead having default 12 grid.

Expected behavior When adding the component inside the Tab, Accordion, Carousel component the inner component should inherit the grid layout from its parent component (i.e tab component). eg: Suppose we add Tab component and change its layout to 6 grid, now when we add text component inside tab component, the expected behavior should be that the text component should have layout of 6 grid. In the current code the text component has layout of 12 grid.

Steps to reproduce:

  1. Add Tab component to page.
  2. Add text component to tab.
  3. Change the layout of Tab component to 6 grid.
  4. Now try to change the layout of text component. Instead of inheriting grid layout "6" from tab component, text component will have 12 grid layout.

Environment

Additional context / Screenshots

image

ky940819 commented 4 years ago

This is a general issue with the OOTB ResponsiveGrid handling, not specifically a Core-Components issue. You can always break the inheritance of the layout by nesting responsive grids.

In this situation the responsive grid would behave properly if the container component its self was the responsive grid, however it's a actually a responsive grid inside of the container.

I think the options for resolving this are: 1) adobe rewrites the responsive grid logic so that it reconciles the columns with more than just the parent responsive config. 2) break the container component reliance on the OOTB responsive grid and re-implement it as part of this project.

Either way, the behaviour I would like to see for determining the columns and widths is to do the following: For any responsive grid, walk up the resource tree until you find a resource whose policy sets the number of columns When you find one explicitly set in the style, then that is your reference point, anything above that doesn't matter. If you reach the cq:Page resource without finding any such configuration, then assume 12 as default. Walk back down the tree and figure out how many columns are actually available.

Here's an example:

  1. Page 2.-- jcr:content 3 ---- responsiveGrid (design: 6) 4 -------- responsiveGrid (design: 12) 5 ------------ responsiveGrid (width: 3) 6 -------------- tabComponent (width 12) 7 ---------------- responsiveGrid (width: ?)

What is ? My answer to that is to go up to level 4, where we have a design policy setting the number of columns to 12. It doesn't matter that the responsive grid above it has 6 columns, our policy says 12. Then we go down the tree and find the minimum width. Level 5 sets the width to 3. Although level 6 sets the width to 12, the effective width for that content is 3 because it cannot go outside of the columns. so ? = Math.min(12, 3, 12) = 3

gabrielwalt commented 3 years ago

Maybe the container should always inherit the layout mode from its parent, but this would require a new version of the container.