Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Elements for Helix Blocks #211

Open
trieloff opened this issue Aug 17, 2021 · 6 comments
Open

Custom Elements for Helix Blocks #211

trieloff opened this issue Aug 17, 2021 · 6 comments
Labels
question Further information is requested

Comments

@trieloff
Copy link
Contributor

Overview

Should we change the Markup generated by helix-pipeline-service for blocks (Tables in Word/GDocs) from generic div to custom HTML elements?

Details

I've built a tiny POC here https://github.com/trieloff/superdevoluy (thanks to @kptdobe for offering a small project to start with)

The site is using three types of block markup:

<helix-card><div><div><p><strong>Propriétaires</strong></p><p>Courriel: <a href="mailto:[email protected]">[email protected]</a></p></div></div></helix-card>
<helix-card sealed="true"><div><div><p><strong>Propriétaires</strong></p><p>Courriel: <a href="mailto:[email protected]">[email protected]</a></p></div></div></helix-card>
<div class="card"><div><div><p><strong>Propriétaires</strong></p><p>Courriel: <a href="mailto:[email protected]">[email protected]</a></p></div></div></div>
  1. a regular custom element
  2. a custom element using Shadow DOM
  3. Helix as usual

I'm loading the JS for the custom elements using

<script src="/elements/card/card.js" type="module" async></script>

Where Helix as usual needs a small script to bootstrap the decoration of block divs like this

/**
 * Loads JS and CSS for all blocks in a container element.
 * @param {Element} $main The container element
 */
async function loadBlocks($main) {
  $main
    .querySelectorAll('div.section-wrapper > div > .block')
    .forEach(async ($block) => loadBlock($block));
}

/**
 * Loads JS and CSS for a block.
 * @param {Element} $block The block element
 */
export async function loadBlock($block) {
  const blockName = $block.getAttribute('data-block-name');
  try {
    const mod = await import(`/blocks/${blockName}/${blockName}.js`);
    if (mod.default) {
      await mod.default($block, blockName, document);
    }
  } catch (err) {
    // eslint-disable-next-line no-console
    console.log(`failed to load module for ${blockName}`, err);
  }

  loadCSS(`/blocks/${blockName}/${blockName}.css`);
}

The Custom Elements approach just uses the built-in features of the browser.

The code to register a block or custom element is very similar:

async function decorateBlockCard($block) {
  let html = '';
  $block.querySelectorAll('p').forEach((p) => {
    html += `${p.innerHTML}<br>`;
  });
  $block.innerHTML = html;
}

export default async function decorate($block) {
  decorateBlockCard($block);
}

vs.

import { loadCSS } from '/scripts/scripts.js';

class Card extends HTMLElement {
  constructor() {
    super();
    let html = '';
    let root = this;

    if (this.getAttribute('sealed')) {
      this.attachShadow({ mode: 'open' });
      root = this.shadowRoot;
    }

    this.querySelectorAll('p').forEach((p) => {
      html += `${p.innerHTML}<br>`;
    });
    root.innerHTML = `<div style="background: blue; margin-bottom: 50px;">${html}</div>`;
  }
}

customElements.define('helix-card', Card);
loadCSS(`/elements/card/card.css`);

The if condition and this.attachShadow({ mode: 'open' }); and root = this.shadowRoot; are only needed if you want to use Shadow DOM, but without getting too much ahead of myself, you probably don't want to use Shadow DOM.

The result are three blocks, styled slightly differently to outline the differences of the approaches:

Screen Shot 2021-08-17 at 16 29 17

Regular Custom Elements (green)

They behave just like normal elements, and the custom CSS rules in a separate file

main .section-wrapper helix-card > div {
  text-align: left;
  margin-bottom: 50px;
  background: green!important;
}

work just as expected. There is no need to keep these rules in a separate file, you can just put them into the main CSS if you like – or not. Also note that a rule from the main CSS is able to seep into the compontent and add a (strong) into the strong tag's contents.

strong::after {
  content: " (strong)";
}

Custom Elements with Shadow DOM (blue)

These elements behave as if they would be living in an iframe on the document, there is no external styling that can seep into the element, not even the background: green!important; line from the custom CSS is able to override the inline style="background: blue; margin-bottom: 50px;" attribute.

Note that neither the styling of the strong tag (with extra content) nor the styling of the a tag is making it through, giving the entire component an extremely strong vanilla flavor.

Unless you really, really hate the cascading nature of CSS, there is almost no reason to use Shadow DOM in Helix projects, as there is no boundary of trust between the person that writes the component and the person that writes the main CSS.

If you don't want Shadow DOM, the Card class can be simplified to:

class Card extends HTMLElement {
  constructor() {
    super();
    let html = '';

    this.querySelectorAll('p').forEach((p) => {
      html += `${p.innerHTML}<br>`;
    });
    this.innerHTML = `<div>${html}</div>`;
  }
}

Helix as usual

Your block is just regular HTML on the page, with regular styling, albeit with the neccessity of a custom loader and a non-standard constructor API like this (repeated from above):

async function decorateBlockCard($block) {
  let html = '';
  $block.querySelectorAll('p').forEach((p) => {
    html += `${p.innerHTML}<br>`;
  });
  $block.innerHTML = html;
}

export default async function decorate($block) {
  decorateBlockCard($block);
}

Proposed Actions

  1. port the rest of the superdevoluy site to use HTML Custom elements (regular) with a static index.html
  2. measure page speed and determine the best way to load the card.js scripts for each of the custom elements (straight in the HTML, with a bootstrap script, etc.)
  3. adjust https://github.com/adobe/helix-pipeline-service/blob/main/src/steps/create-page-blocks.js to create custom element markup instead of plain divs (@tripodsan any idea how we can turn this into a feature flag)
  4. determine impact
  5. decide
@trieloff trieloff added the question Further information is requested label Aug 17, 2021
@trieloff
Copy link
Contributor Author

Supported in all browsers relevant for Helix https://caniuse.com/mdn-api_window_customelements

@tripodsan
Copy link
Contributor

tripodsan commented Aug 17, 2021

3. adjust https://github.com/adobe/helix-pipeline-service/blob/main/src/steps/create-page-blocks.js to create custom element markup instead of plain divs (@tripodsan any idea how we can turn this into a feature flag)

The easiest would be to use the version lock edge-dict and hardcode (custom--superdevoluy--kptadobe=ci112233) (@stefan-guggisberg we might not hardcode the pipeline version right now :-)

@rofe
Copy link
Contributor

rofe commented Aug 18, 2021

Interesting! Fwiw, I started playing with using a shadow-DOMed web component for the sidekick to keep the page CSS from bleeding into it. Would love to share/compare outcomes.

Since this issue is essentuially asking a question, should we turn it into a discussion?

@trieloff
Copy link
Contributor Author

As of trieloff/superdevoluy@ce4c9f0 all blocks have been ported to custom elements.

@kptdobe
Copy link
Contributor

kptdobe commented Aug 20, 2021

Minor concern: if the server gives me a helix-card in my DOM, I would assume this is a standard "helix" component shared across all projects with a predefined structure... But this is wrong, this is a project specific element.
If these represent "blocks", block-card may imply less things. Maybe.

@trieloff
Copy link
Contributor Author

trieloff commented Sep 2, 2021

After the complete porting, I've tried some more variants:

  1. Eager Loading of Custom Elements (each custom element has a script tag before use) like this: https://github.com/trieloff/superdevoluy/blob/8d85a67e15b90f210c9e7f412e0b587a9c7c006a/index.html#L15-L16
  2. Lazy Loading of Custom Elements (each custom element will be imported when first encountered: https://github.com/trieloff/superdevoluy/blob/9f5dd4dd1b5e2fff27c24dcfa4196ab74c89ff79/scripts/scripts.js#L151-L152)
  3. Reduction of Boilerplate (the customElements.define call is centralized in the block decoration script like this: trieloff/superdevoluy@ec8015c)
  4. Inline Styles (instead of having one CSS file for each block, the CSS is part of the JS in a template literal like this: trieloff/superdevoluy@noboilerplate...trieloff:inlinestyles)

I've run the Chrome Profiler on each variant (using hlx.live) and included @kptdobe's original site, too:

Original

Screen Shot 2021-09-02 at 13 07 07

Eager Custom Elements

Screen Shot 2021-09-02 at 13 07 16

Lazy Custom Elements

Screen Shot 2021-09-02 at 13 07 20

No Boiler Plate

Screen Shot 2021-09-02 at 13 07 24

Inline Styles

Screen Shot 2021-09-02 at 13 07 28

Summary

Variant URL Loading Scripting Rendering Painting System Idle Total FCP LCP Onload
Original https://main--superdevoluy--kptdobe.hlx.live/ 10 138 141 89 79 1092 1549 249 413 424
Eager Custom Elements https://main--superdevoluy--trieloff.hlx.live/index.html 13 152 181 66 109 525 1045 237 383 397
Lazy Custom Elements https://lazy--superdevoluy--trieloff.hlx.live/index.html 8 144 69 51 42 699 1013 235 386 388
No Boilerplate Custom Elements https://noboilerplate--superdevoluy--trieloff.hlx.live/index.html 9 133 77 93 45 598 955 259 367 378
Inline Styles Custom Elements https://inlinestyles--superdevoluy--trieloff.hlx.live/index.html 13 191 101 41 54 568 967 309 440 460
  • Overall, Custom Elements seem to be beneficial when it comes to rendering performance
  • the approach that I would pick for loading/coding custom elements is No Boilerplate
  • Having CSS in JavaScript does not just look weird, it is also slower than the reference and I'd abstain from this approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants