[WNMGDS-3009][WNMGDS-3010] Add Angular example project and fix Angular web component content issue

[pwolfert]

Summary

WNMGDS-3009 and WNMGDS-3010

Angular example project

I took the basic Angular project off of stackblitz.com and added some our web components. Because I lack modern Angular experience, I'm looking for feedback and help to...

1. Make sure this is a realistic and helpful example of an Angular project
2. Implement event handling so it's more interactive and exercises the different features of our web components

Fix for Angular providing content to web components

In this Angular project, we're trying to provide inner HTML content to certain web components like the ds-alert and ds-button by Angular's standard methods like putting the content in a template like this:

<ds-alert heading="You've loaded the web-components example">
  <p>
    This is an example of a success alert. If you want to see an error alert, click the
    button below.
  </p>
  <ds-button>Break things</ds-button>
</ds-alert>

However, what we found is that when the CustomElement instances (web components) tried to read their this.innerHTML to get that content, it was blank—just an empty string. That's the bug. That's what our Angular product teams were experiencing. They had a fix, which was to supply the content through an innerHTML attribute (a special Angular binding), but this method is cumbersome and not ideal.

It turns out that Angular wasn't failing to supply the content to the CustomElement; it was just supplying it late. We already had a MutationObserver that was looking out for changes to the inner content, and it was indeed finding those late arrivals. However, our renderPreactComponent wasn't set up to respond well to this "it's blank—psych!—no it's not" behavior.

How to test

1. Update dependencies with yarn install
2. From the examples/angular directory, run yarn start

Checklist

• Prefixed the PR title with the Jira ticket number as [WNMGDS-####] Title or [NO-TICKET] if this is unticketed work.
• Selected appropriate Type (only one) label for this PR, if it is a breaking change, label should only be Type: Breaking
• Selected appropriate Impacts, multiple can be selected.
• Selected appropriate release milestone

[pwolfert]

Regarding c578c7c, the only place I see a problem without that "fix" is with the Astro example. The nested button inside the alert gets essentially a double-render that puts a button inside the button. This is a similar symptom to what we had before we implemented the template memory thing (using a template to store the original inner content and falling back to that when we need to "re-hydrate" the web component). The problem doesn't show up in any of the other nested components like the accordions or the choice checked children, and it doesn't seem to be a problem in any of the other example projects or inside Storybook. Perhaps Astro is doing something special to the DOM?

[pwolfert]

In our last pairing session, we started putting dynamic content into the <ds-alert> body, like this:

<ds-alert heading="Alert with dynamic content">
  <p>Count: {{count}}</p>
  <ds-button>Increment</ds-button>
</ds-alert>

but we found that the web component content did not update on its own when the Angular variable count changed. If we could possibly get the Angular component itself to re-render its template, it might cause the root elements of the already rendered web component to change and trigger its MutationObserver. However, it doesn't seem to be doing that. We did find that if we accessed the ds-alert element instance within an Angular function and manually called that CustomElement's renderPreactComponent() function, it would re-render with the updated dynamic content. For instance, if the alert text said, "Count: 0" before and we incremented it by 1, re-rendering would result in it successfully showing "Count: 1". There's conceivably some sort of wrapper we could create for our web components in Angular that could detect changes and manually call our renderPreactComponent function. A similar solution would be to allow a framework to pass some sort of publisher that the web component can subscribe to for changes.

Another problem we noticed that we didn't come up with any solution for was that nested components can't have Angular bindings. If a web component has various kinds of bindings defined through the Angular template, but that web component is inside another web component, the rendering of the top-level web component clobbers all of that. When one of our web components renders, it reads in its inner HTML content through a <template> element that then gets converted to Preact VirtualDOM elements. In that process, it's basically making clones of everything, but it doesn't retain any of its event bindings or other meta-data. This seems like a fundamental difference in how Preact works vs Angular, and I really don't know what the solution is here. The only comfort we have is in the fact that you can use the browser native APIs like element.addEventListener to bind events even if you can't use Angular's way.

[pwolfert]

A note on event binding: The only place it doesn't seem to work is in nested web components—because the original element that had the binding gets clobbered by the parent web component rendering. I don't see a way around that in Angular, and it could be problematic for container-like components like the dialog, drawers, and accordions.

[tamara-corbalt]

I implemented the following polling code at the end of the renderPreactComponent function:

  let previousContentSnapshot = template.content.cloneNode(true);

  const hasContentChanged = () => {
    const currentSnapshot = template.content.cloneNode(true);
    const isDifferent = !currentSnapshot.isEqualNode(previousContentSnapshot);

    if (isDifferent) {
      // console.log('Template content modified!', currentSnapshot);
      previousContentSnapshot = currentSnapshot;
      this.renderPreactComponent([...template.content.firstChild.childNodes]);
    }
  }

  if ((this as any).__pollInterval) {
    clearInterval((this as any).__pollInterval)
  }
  (this as any).__pollInterval = setInterval(hasContentChanged, 500);

It works, but I imagine it's not terribly performant. Every web component on the page will be comparing DOM nodes every half second. If we really did want to use this, we could put it in an Angular wrapper so at least we only get the performance hit in Angular.

Hey @pwolfert, thanks for this detailed explanation!
I have a question about the polling code and the idea of using an Angular wrapper. My assumption is that to isolate the polling to Angular situations, we’d move this logic into a directive in our Angular project. My question is, could we try an event-driven approach instead of using a set interval to check for changes in the template snapshot? For example, could we subscribe to an observable that detects changes in the Angular environment and then call hasContentChanged only when needed?

Just a heads up, I’m not totally confident I understand all the context here.

[pwolfert]

@tamara-corbalt, being able to detect changes would be vastly superior, but that is the very thing I can't seem to do. Take a look at the comments for this commit. I've tried many different things but haven't been able to solve that problem yet. I dislike the polling solution too, but I haven't been able to come up with anything else that works. Everyone should feel free to experiment with this and try to detect when Angular makes changes!

[pwolfert]

I want to put it down in writing here because this is a pretty good record of these problems we're working through:

I've said recently that we likely wouldn't have some of the same problems if we were using the Shadow DOM in our components, but I also couldn't articulate on the spot the accessibility reasons we chose to not use the Shadow DOM in the first place. This article is likely one of the ones we referenced when coming to this decision, and I think it does a great job explaining the problem and the work that has been done to try to find solutions. We also didn't make the decision in a vacuum; accessibility specialists were in agreement with us that introducing a Shadow DOM would create accessibility problems. If the team were to want to move towards using Shadow DOMs, there'd need to be a pretty extensive research phase that includes working with product teams to find out how they use ARIA within their applications beyond what we've mentioned in our own guidance in order to make sure the design system solutions can support all accessibility needs.

One of the problems we're facing right now with Angular is that the web component render process loses event bindings on nested web components. I don't think that would actually be fixed by switching to the Shadow DOM, because the events that Angular binds are on the input elements and not the output elements. The elements provided to web components as content are not the same exact elements that are rendered in the final product. I'd be curious to see some research into whether other web component frameworks do support that scenario. Does Lit allow Angular to bind a click event listener to an interactive custom element inside a container-like custom element and not lose it?

[pwolfert]

I'd be curious to see some research into whether other web component frameworks do support that scenario. Does Lit allow Angular to bind a click event listener to an interactive custom element inside a container-like custom element and not lose it?

Update: Angular bindings do work with Lit because the "input" elements using slots are preserved in the Shadow DOM "output" (which is rendered in a <slot> element).

[tamara-corbalt]

Hi @pwolfert, I’m just trying to briefly synthesize this for myself and check my understanding.

Basically we're saying that using the shadow DOM would improve change detection by creating a clear boundary within which we could reliably observe changes, eliminating the need to poll or scan the entire DOM over and over.

However, this doesn't solve the problem of angular’s event bindings on nested elements because angular's bindings are applied to the original input elements in the DOM. When these elements are handed over to Preact’s runtime—whether they’re in the shadow DOM or part of <template> innerHTML—Preact’s Virtual DOM processing takes over and re-renders those elements as new instances in the final output, which is where angular’s bindings are lost.

From your last comment, we might be able to preserve angular bindings on nested elements with Lit, specifically through its use of slots within the shadow DOM. If we were to explore this option, it sounds like we would first need to conduct more research on two key areas: the ARIA concerns related to using the shadow DOM and the potential challenges with managing global styles.

[pwolfert]

@tamara-corbalt, that's exactly it! Well said.

[pwolfert]

@tamara-corbalt, @jack-ryan-nava-pbc, and @kim-cmsds,

As per the discussion we had in the planning meeting, I've broken off the more experimental solutions into what will be a new pull request for a follow-up ticket. I've cleaned up this current PR to be 1) the creation of the new Angular example (WNMGDS-3009) and 2) the simpler fix for static content only (WNMGDS-3010).

This is now ready for review.

Thanks!

[jack-ryan-nava-pbc]

Just noting for myself that this file will need to be manually updated when we call apply-example-templates

[jack-ryan-nava-pbc]

Minor comments and questions. Looks good! Changes to define don't seem to impact existing web components, which is great. Nice work!!!

[jack-ryan-nava-pbc]

Nit: does it make sense to coalesce a null value to null?

[pwolfert]

It's the only way I can keep Angular from passing an empty string to that prop. That actually highlights an issue that I didn't want to try to fix in this PR but that should probably be discussed: It's actually not very easy for a downstream app to have the variation as a variable and pass it on, because the only way someone can make an info alert is if this is undefined. Our web component wrapper doesn't treat an empty string as undefined, either, which means the only way to pass on the lack of a variation is to not set the attribute in the first place. Two possible fixes:

1. Just change the ds-alert wrapper to interpret empty or nullish values as undefined when passing it on to the Alert React component or
2. Change the varations that the Alert React component accepts to include something like "info" as one of the options.

[jack-ryan-nava-pbc]

Ah ok. Is a possible issue with option 2 that it would become a breaking change? In cases where users have no variation expressed they would have to supply an option?

[kim-cmsds]

Interestingly there's an open PR (#3301) that is asking to expose 'informational' (or the Alert's default type)

[pwolfert]

I was thinking about that. It's not a bad idea. Maybe info is a good abbreviation for it, and we might need to take a look at all the places where it's used and change some actual code and not just the types in order to merge that.

[pwolfert]

@jack-ryan-nava-pbc, regarding it being a breaking change, I actually think we could make it a completely backwards-compatible change. Leaving it off would still default to info like it always has.