Creating wrapper elements with tagged templates

[chrischrischris]

Moving this conversation started by @mokimo to the public forum. This is a copy of the thread before my response:

---

Description

We are looking for a more standardised way of creating wrapper elements for the GNAV. IMO this is a good candidate for a broader audience as milo utility function/paradigm to use.

The current way

How things are currently working is creating a lot of tags via Javascript in the GNAV

const searchBar = createTag('aside', { id: 'gnav-search-bar', class: 'gnav-search-bar' });
const searchField = createTag('div', { class: 'gnav-search-field' }, SEARCH_ICON);
const searchInput = createTag('input', {
  class: 'gnav-search-input',
  placeholder: label,
  'daa-ll': 'search-results:standard search',
});
const searchResults = createTag('div', { class: 'gnav-search-results' });
const searchResultsUl = createTag('ul');

The proposed way

To separate the JS and HTML where possible, we are proposing using a function or tagged templates to generate DOM-Fragments.

const stringToFragment = (htmlString) => document.createRange().createContextualFragment(htmlString);

const fragment = stringToFragment(`
  <aside id="gnav-search-bar" class="gnav-search-bar">
    <div class="gnav-search-field">
      ${SEARCH_ICON}
      <input class="gnav-search-input" placeholder=${label} daa-ll="search-results:standard search"/>
    </div>
    <div class="gnav-search-results">
      <ul></ul>
    </div>
  </aside>
`)

Based on @chrischrischris feedback an interesting approach could be tagged templates instead of a function so that it can be used as h'<stuff></here>'

More input appreciated!

[chrischrischris]

@auniverseaway responded with

I think this is good for certain use cases, and not ideal for others.

If you have existing DOM you need to blend into this, you're stuck doing something like myImg.outerHTML which means you're asking the browser to do work it's already done.

The other piece is that if you need to bind events or add attributes due to state, you are querying the DOM again after you inject this raw HTML.

I typically like to have a single code path for accomplishing the same task, which is why I tend to use createTag but I'm not at all opposed to the above.

[chrischrischris]

@overmyheadandbody responded with

Following up on this, I came up with an approach that should satisfy our desired way of handling templates but also the valid concerns from the Milo side.

Below you'll find a quick example of the current way of building templates compared to our proposal.

Current

decorateSearchBar = (label) => {
  const searchBar = createTag('aside', { id: 'gnav-search-bar', class: 'gnav-search-bar' });
  const searchField = createTag('div', { class: 'gnav-search-field' }, SEARCH_ICON);
  const searchInput = createTag('input', {
    class: 'gnav-search-input',
    placeholder: label,
    'daa-ll': 'search-results:standard search',
  });
  const searchResults = createTag('div', { class: 'gnav-search-results' });
  const searchResultsUl = createTag('ul');
  searchResults.append(searchResultsUl);
  const locale = getLocale();

  searchInput.addEventListener('input', (e) => {
    this.onSearchInput(e.target.value, searchResultsUl, locale, searchInput);
  });

  searchInput.addEventListener('keydown', (e) => {
    if (e.code === 'Enter') {
      window.open(this.getHelpxLink(e.target.value, getCountry()));
    }
  });

  searchField.append(searchInput);
  searchBar.append(searchField, searchResults);
  return searchBar;
};

Proposal

decorateSearchBar = (label) => {
  const searchInput = toFragment`<input class="gnav-search-input" placeholder="${label}" daa-ll="search-results:standard search" />`;
  const searchResultsList = toFragment`<ul></ul>`;
  const searchBar = toFragment`
    <aside id="gnav-search-bar" class="gnav-search-bar">
      <div class="gnav-search-field">
        ${CONFIG.search.icon}
        ${searchInput}
      </div>
      <div class="gnav-search-results">
        ${searchResultsList}
      </div>
    </aside>`;

  const locale = getLocale();

  searchInput.addEventListener('input', (e) => {
    this.onSearchInput(e.target.value, searchResultsList, locale, searchInput);
  });

  searchInput.addEventListener('keydown', (e) => {
    if (e.code === 'Enter') {
      window.open(this.getHelpxLink(e.target.value, getCountry()));
    }
  });

  return searchBar;
};

Advantages

• template structure is clear and readable, creating less confusion around the final output. New contributors have a straightforward overview of the resulting structure, making it easier to make updates when needed;
• elements that already exist (either created with the toFragment method itself or just referencing from the already existing DOM) can be simply added anywhere inside the template, with their event listeners maintained;
• elements created with this method can be referenced in other parts of the logic if needed;
• no need for additional .querySelector calls after an element/template is created, elements can be referenced directly.

POC

You can find a barebones version of the method here - https://codepen.io/ramuntea/pen/zYabPJR?editors=1010

Feedback

Do you see this is a good path forward, @auniverseaway & @chrischrischris?

I'm asking because we'll be refactoring the gnav code in a feature branch and likely a slightly different folder, so we don't bump into merge/rebase issues as multiple engineers add conflicting logic.

If you think this templating approach would be valuable for the whole Milo project, I can create a PR and add my toFragment method in the utils.js file of the main branch.

[chrischrischris]

@auniverseaway responded with

Thanks @overmyheadandbody! This is clever!

Couple comments:

1. I love the comment, "template structure is clear and readable..." I don't know that it's much better than createTag, but I do like the consistency aspect. The goal is great.
2. What is happening here seems to be an abstraction of the things I said I wasn't a fan of. Instead of having to hit the DOM with querySelector to add your events, you're pre-creating an element, then using a querySelectorAll('elem') to swap out the elements from <elem ref=". Then adding your event. So now we are building DOM we won't use and making the CPU swap things out. Seems like having a more direct, querySelector('.my-thing') would be the lesser of evils. It's really clever, but it is also adding more work and still requires that querySelector (albeit used differently).
3. Small nit: I think we need to make sure if we develop a function, we should have a clear delineation of vocabulary. We obviously cannot change DocumentFragment but we also have an internal feature called "fragments" which are effectively Experience Fragments. In this light, I would probably have a function called, toEl so we know the result is a proper element.

gnav and footer are our #1 and #2 contributors to TBT that are under our control. By adding more work to the CPU, I do worry we are not moving in the right direction to get our TBT down.

The thing I always say is that developers should be considered last when it comes to priorities...

1. what is good for the visitor
2. what is good for seo (perf)
3. what is good for authors
4. what is good for developers?

From what I'm hearing above, this conversation is about "readability" above all. Which is a developer problem.

If you want to prototype your idea with gnav and see if you find it materially better, I'm 100% supportive. I don't think I'm ready to ship the above until we see it in use and we collectively agree it's a better solution for everyone.