[feat] pfelement | Update composed default to true in the emitEvent method

[wesruv]

Description of the issue

Working with some analytics folks they mentioned they couldn't get the show/hide events from the tabs on redhat.com's contact page. The tabs are inside of a pfe-content-set and it looks like the events from the child components don't make it outside of pfe-content-set.

Impacted component(s)

• pfe-content-set

Steps to reproduce

1. Go to https://www.redhat.com/en/contact
2. In the console, execute this code to show that the document isn't picking up the event: document.addEventListener('pfe-tabs:shown-tab', function(event) {console.log('lightDomListener', event)});
3. Then execute this code to show that the shadowRoot does pick up the event: document.querySelector('pfe-content-set').shadowRoot.addEventListener('pfe-tabs:shown-tab', function(event) {console.log('shadowDomListener', event)});
4. Click on tabs (e.g. Sales, Customer Service, Product & learning support, etc)

Expected behavior

I'm thinking pfe-content-set could boost the child events, and it might be nice if it pushed out a generic event (so a site dev wouldn't have to know which state the content set was in).

[wesruv]

I don't see how that PR addressed the issue, which is still occuring. Reopening.

[bennypowers]

I'd like to suggest a different approach.

Composed events might not be the way we want to go here. Other design systems use non composed events. For example these events on <vaadin-upload>

https://github.com/vaadin/web-components/blob/9da367cb581a5f2a5636e9355003ec28f72196c6/packages/upload/src/vaadin-upload.js#L662-L711

or these events on <mwc-snackbar>

https://github.com/material-components/material-web/blob/6278ee5df98ca7eae0d694e58b052fdbf4b0519d/packages/snackbar/mwc-snackbar-base.ts#L95-L113

By counter-example, shoelace does tend to compose events: https://github.com/shoelace-style/shoelace/blob/9c31d148feff0c40b6d3407d85b4c4872e0832f3/src/internal/event.ts#L11

as does FAST:

https://github.com/microsoft/fast/blob/bdd28c8861b3523a9984ca8feda2af9019861a8a/packages/web-components/fast-element/src/components/controller.ts#L12-L16

So for analytics events, I propose that we in fact remove analytics events from PFE, subclass elements in <rh-*> and implement composed analytics events there.

Proposal doc: https://docs.google.com/document/d/1tSAV0aysOrMl3XupUuY7oV50Cxlu7AcxZdV-gtowDzg/edit#heading=h.mx5h1h2oj4l2

[bennypowers]

A nice counter-point by the author of the linked article

https://twitter.com/WestbrookJ/status/1471830306617450498?s=20

[wesruv]

Arg, well apparently that PR did address this issue, as it made all events composed, but now we're talking about not using composed events.

So that let's hash this out a bit @bennypowers

The issue was:

So if we have something like this:

body
- parent component
 - shadow-root
   - child component
     - shadow-root
       - button

A 'non-composed' event from button could be written to emit from the child component tag, making it out to parent component, but not to the body. Since the analytics script is listening in the body, and we lose the ability to reliably track UX interactions.

In the case of pfe-content-set it'd make sense for it to capture events from pfe-tabs and pfe-accordion. We know those are expected children components, and should send out a generic opened/closed events.

But we have a few components that use 'light DOM as data' and don't know what components will be in it's shadow root (e.g. the second version pfe-navigation that's in the branch https://github.com/patternfly/patternfly-elements/tree/CPFED-3689-new-navigation-epic/elements/pfe-navigation) and cp-documentation from cp-elements.

Both are using 'light DOM as data' to get style encapsulation so the parent site can't accidentally effect the appearance of important parts of it's UI that involve content that the site (not the component) should provide.

So if I understand the composed: true article, by setting composed:true universally we're using more performance budget than we may want to, but if we turn switch back we reintroduce significant issues tracking UX interactions with analytics; and I don't see a straightforward fix to this issue.

Hopefully this makes sense, glad I mistakenly reopened this, if it doesn't make sense maybe we can do a call.

[bennypowers]

Hey yeah I think your read on this is good. In the twitter thread I linked above quite a number of wc movers-and-shakers replied and the general consensus is that "UI events" should compose but "app events" should not, where "UI events" are things like click, change, opened, select, etc and "app events" are things like user-added, product-liked etc - app events are relevant to app builders, less so to PFE.

I will review our new events and make sure they compose.