project-bricks / helix-website-wc

aem.live site on Web Components
https://bricks.albertodicagno.com
Apache License 2.0
0 stars 0 forks source link

#47 - HtmlTemplateBrick #48

Closed bdelacretaz closed 11 months ago

bdelacretaz commented 12 months ago

Fix #47 - work in progress, the hero CSS is somewhat broken at this point

Test URLs:

aem-code-sync[bot] commented 12 months ago

Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed. In case there are problems, just click the checkbox below to rerun the respective action.

aem-code-sync[bot] commented 12 months ago
Page Scores Audits Google
/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
fnhipster commented 12 months ago

This is interesting. Could this be out of the box in the Brick class itself?

bdelacretaz commented 12 months ago

From my point of view this would be ready, but the linter complains, need to fix a few details.

At commit 3ce3e8a this demonstrates a low-code HTML template, with one element populated from a data-aem-selector attribute, and another one populated with JS code.

The template uses shadowroot='false', CSS "isolation" is provided by prefixing rules with the name of our custom aem-hero element. I think that's good enough, no need for shadow DOM in this case.

bdelacretaz commented 12 months ago

Could this be out of the box in the Brick class itself?

Ah maybe, if that doesn't make that class too complex. And I think it's good to make shadow DOM optional, as in many cases it's not needed.

Feel free to refactor, I'm still not a JS guru anyway ;-)

fnhipster commented 12 months ago

I think the shadow root as default got in when we initially used slots but that's not the case anymore. It might have simplified at lot if we didn't use it at all.

bdelacretaz commented 11 months ago

I have simplified the injectMoreContent thing, really ready to merge now from my point of view.

Not sure if the failed aem-psi-check is a concern? Lighthouse Score 62 from that check at this point, but if I run it myself with Chrome dev tools on https://codeless-bricks--helix-website-wc--adobe-gw2023-project-bricks.hlx.page/home I get performance score = 99.

dicagno commented 11 months ago

@bdelacretaz yeah looks like there is a problem with the PSI app this week.