Closed boehsermoe closed 5 years ago
Code Climate has analyzed commit 037059fa and detected 16 issues on this pull request.
Here's the issue category breakdown:
Category | Count |
---|---|
Complexity | 1 |
Duplication | 15 |
The test coverage on the diff in this pull request is 0.0% (50% is the threshold).
This pull request will bring the total coverage in the repository to 77.5% (-17.7% change).
View more on Code Climate.
We did this already once for bootstrap 4 - and we found out its to complex for end users, it might work for "developers". So currently i would say this deserves an own repo, maybe from your private github account. The bootstrap 3 repo should not have to much blocks, also not blocks which do the "same".
There is also a discussion about dynamic placeholders (https://github.com/luyadev/luya-module-cms/issues/10) which is something we might improve in the cms core.
luyadev/luya-module-cms#10 is about dependency between child and parent blocks. But currently it is the easiest way to implement this feature and I think it would add a lot of value to LUYA. The DynamicLayout could replace the LayoutBlock, to reduce the number of blocks.
We can not replace the layout block, otherwise all websites using LUYA block would not work anymore.
But we could rebuild the layout block with bc. As default with 2 Columns und map the left and right column. Or add the dynamic layout and mark the layout block as deprecated. So the old LayoutBlock will still work but new layouts using the new dynamic layout?
@boehsermoe Not sure its possible to make this option BC safe because the placeholder name is stored in the table (cms_nav_item_page_block_item) which would be left
and right
. I also would say we should not spend more time in bootstrap 3 improvements/changes - this is a thing from the past.
For me we can close this task if there is no more need to improve the bootstrap 3 module.
A LayoutBlock with multiple columns. What do you think?