aklinker1 / webext-core

Collection of essential libraries and tools for building web extensions
https://webext-core.aklinker1.io
MIT License
96 stars 11 forks source link

Feasibility of Adding Event Bubbling Control to createIsolatedElement #52

Closed mefengl closed 6 months ago

mefengl commented 6 months ago

https://github.com/aklinker1/webext-core/blob/f0a131c21f17bf17782ab0306e262a76b78f1ad8/packages/isolated-element/src/index.ts#L27

I'm currently using the createContentScriptUi from WXT for dialog components and facing an issue where some keyboard events bubble up to the host page. I found a workaround using stopPropagation in the dialog, but I'm considering a more streamlined approach.

I'm curious whether it would be a good idea to introduce an optional parameter in createIsolatedElement (used by createContentScriptUi) to control the bubbling of events, especially for mouse and keyboard interactions. This could potentially make it easier for developers to manage event handling in isolated components.

Is this something that would be useful to others, and would it fit well with the design and goals of WXT? I'd appreciate any thoughts or feedback on this suggestion.

aklinker1 commented 6 months ago

@mefengl Yes, that sounds fine to me, as long as it's simple and easily configurable. What are you doing in your code, and what do you propose this control would look like in @webext-core/isolated-element?

mefengl commented 6 months ago

My approach in my code is basically as follows:

<input
  type="text"
  onKeyDown={(e) => e.stopPropagation()} // Stop event bubbling up
/>

My proposal might look something like this:

export async function createIsolatedElement(options: CreateIsolatedElementOptions & { isolateEvents?: boolean }): Promise<{
  parentElement: HTMLElement;
  isolatedElement: HTMLElement;
  shadow: ShadowRoot;
}> {
  const { name, mode = 'closed', css, isolateEvents = false } = options;

  // ... create elements and Shadow DOM code

  // Add logic to prevent event bubbling
  if (isolateEvents) {
    // Prevent 'keydown', 'keyup', 'keypress' events from bubbling
    ['keydown', 'keyup', 'keypress'].forEach(eventType => {
      isolatedElement.addEventListener(eventType, e => e.stopPropagation());
    });
  }

  // ... other existing code

  return {
    parentElement,
    shadow,
    isolatedElement: body,
  };
}
mefengl commented 6 months ago

I'm still not sure if adding event bubbling control to createIsolatedElement is a good idea. It might be premature for a feature that's not widely needed. Also, using iframes might be a simpler solution for complete isolation. What do you think?

aklinker1 commented 6 months ago

I'm still not sure if adding event bubbling control to createIsolatedElement is a good idea. It might be premature for a feature that's not widely needed. Also, using iframes might be a simpler solution for complete isolation. What do you think?

I think it fits with the concept of an "isolated element", so I'm on board with adding this feature.

My proposal might look something like this...

I think this looks good. Lets keep the default false so it doesn't break existing projects and also let you define a list of events to prevent bubbling for.

export interface CreateIsolatedElementOptions {
  // ...
  /**
   * When enabled, `event.stopPropogation` will be called on events trying to bubble out of the shadow root.
   *
   * - Set to `true` to stop the propogation of a default set of events, `["keyup", "keydown", "keypress"]`
   * - Set to an array of event names to stop the propogation of a custom list of events
   */
  isolateEvents?: boolean | string[];
}

Worst case, if the provided behavior doesn't work for someone, they can setup the event listeners themselves to add exceptions or whatever.

Feel free to open a PR!

mefengl commented 6 months ago

Make sense, I'll get on the PR.

aklinker1 commented 6 months ago

Released in https://github.com/aklinker1/webext-core/releases/tag/isolated-element-v1.1.0