fix: use composedPath instead of target, fixes #278

[silvester-pari]

This PR introduces e.composedPath()[0] as an alternative for e.target to make the library usable inside a shadow DOM. Background: Because of Event target Retargeting using event.target returns the ancestor (e.g. custom element) instead of the clicked element, resulting in erroneous behavior like closing the dropdown on click (see #278).

All instances of `e.target' have been replaced. The built bundle was tested in the minimal reproduction example and provided the desired result (behavior of dropdown inside shadow DOM was the same as without shadow DOM).

Closes #278.

[bolt-new-by-stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[github-actions]

Playwright E2E Test Results

78 tests   78 ✅  1m 51s ⏱️
69 suites   0 💤
 1 files     0 ❌

Results for commit 580b2ce.

♻️ This comment has been updated with latest results.

[ghiscoding]

@silvester-pari so just to be clear, should I assume correctly that in a non-shadow environment, then e.composedPath()[0] is equal to e.target? What I wonder though is the following, can e.composedPath() ever be null or have an array with less than 1 item? I'm basically concerned about e.composedPath()[0] possible throwing because either composedPath() is null or is not returning at least 1 item. If that ever happens then it's not an equivalent code to e.target and if so then maybe we need to separate it into a small util function?

[silvester-pari]

@ghiscoding I think (not being an expert) that it equals to event.target for non-shadow and for closed shadow cases, the real benefit is in open shadow cases. By using it in all cases it could be considered a bit of "overkill", but it's the only fine-grained way to access the true list of EventTargets on which an event listener will be invoked. If I understand correctly, it should always return an array (see MDN - "Return value"). See also here a demo in non-shadow cases (like in your multiple-select-vanilla package, where it's also working for non-shadow cases). The only potential downside could be that composedPath() only returns a filled array during event dispatch: https://stackoverflow.com/a/74610934. Not sure how exactly event.target behaves in this case.

If you'd like to add an util function, something like

function isInShadow(node) {
   return node.getRootNode() instanceof ShadowRoot;
}

could be used in order to determine if the element is currently used in a shadow root (see SO answer here). In this case I'd ask you to add the function or to provide guidance on structurizing this.

[ghiscoding]

ok after doing some googling too, it seems to always be filled on modern browser (available since Jan. 2020), but maybe just to be on the safe side we could do something like this external function

protected getEventTarget(e: Event & { target: HTMLElement }) {
  if (e.composedPath) {
    return e.composedPath()[0] as HTMLElement;
  }
  return e.target as HTMLElement;
}

and then call it everywhere you had composed path

protected infiniteScrollHandler(e: (MouseEvent & { target: HTMLElement }) | null, idx?: number, fullCount?: number) {
  const target = this.getEventTarget(e);

if we have problem in the future, then we only have 1 function to fix :)

Side note, I also found this article Handling web components and drag and drop with event.composedPath() interesting and makes me think that I should revisit my data grid library as well.

[silvester-pari]

I did as you suggested, however, I had to add a // @ts-expect-error, since without it it says:

This condition will always return true since this function is always defined. Did you mean to call it instead? ts(2774)

Seems like TS is assuming every browser to have this already...

Edit: Sorry, this was because of a typo on my side!

Thanks for the link, interesting read indeed!

[ghiscoding]

ok thanks even if it always goes into the first condition, I still find it cleaner with the external function :)

So I assume that the PR is ready to be merged now and will go ahead, thanks again.

[ghiscoding]

I've pushed v3.2.2 release that includes your fix. Thanks for you contribution to the project 🎉

[ghiscoding]

side note, if you like the lib, it would be nice to upvote it since it helps to make it more visible. Thanks