569-new-section-carousel-first-full-layout #570
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
|
|
rgravitvl
requested review from
duynguyen,
amlan18041996,
davenichols-DHLS and
sharanagoudapatil
| <hr> | ||
| <hr> |
There was a problem hiding this comment.
Please check these 2 sections? The homepage is not using any carousel, so it should not be impacted.
There was a problem hiding this comment.
As we separated section of first full layout, home page is also got additional
| @@ -4,6 +4,7 @@ const createFullLayoutSection = (main, document) => { | |||
| const div = e.querySelector('div'); | |||
| const style = div.getAttribute('class'); | |||
| if (style) { | |||
| if (i === 0 && e.parentNode.previousElementSibling) e.prepend(document.createElement('hr')); | |||
There was a problem hiding this comment.
just wondering why we need this in the section? I thought the hr tag can be added to individual blocks.
There was a problem hiding this comment.
This would needed to separate fulllayout from other above components. I can explain over the call. Thanks.
There was a problem hiding this comment.
@duynguyen we (Ravi & I) just discussed we should have the sections controlled by the containers and not the blocks. Adding
before / after a block is risky, might work on one place but can have side effects on other places the same block is used.
Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):
Fix #
Test URLs: