Wrapper / item behavior consolidation

Grimreaper → credited pdureau → .

We need to rethink the "drilling" without breaking compatibility (we stay in 1.x)

Because they are 2 issues:

• First, the wrapping happens to early, instead of first looking down the render tree.

)

• Second, the current implementation shows our lesser knowledge of the Render API 4 years ago. We are better and more confident now.

1. Let's create a lot of unit test with various render tree. Some will be OK with current codebase, they are the regression we want to avoid. Some will be KO with the current codebase, they are the improvements we want to achieve.

2. Let's implement a recursive function which is drilling/digging the render tree to find a renderable (or a list of renderables, like we already do for in UI Patterns ) which accept an attribute object. If none is found, we go back to the top of tree and we add the wrapper.

Doing so, let's find a more elegant and generic way, all in one place :

• Removing the logic from StylePluginManager::addClass() (and maybe removing this method in UI Styles 2.0.0)
• Removing the StylePluginManager::addStyleToBlockContent() method
• Removing the StylePluginManager::addStyleToFieldFormatterItems() method

Reworked the drilling.
We lack some Functional tests for the fixed case when we have block layout builder with multiple sections.

Hi,

I have added a test on layout builder, when there is a section with multiple regions.

I have also added a MR with only the tests to be able to compare with the refactoring. Currently only one test fail.

Is it the expected results?
Are there other tests to implement?

The bug about empty div is when there is a "_layout_builder" key, script does not check if there is something inside or not. So it creates an empty div.

As the empty div is already present when using the 8.x-1.x dev version, I propose to merge this refactoring, commenting the failing test and handling it in another issue.

Finally, I think it is better to handle that in this issue. The problem is already present and no one complained before, so it can wait a little bit more.

Changing into bug report as there is a behavior we do not want.

grimreaper → closed merge request !61

Automatically closed - issue fixed for 2 weeks with no activity.