Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Globals - Support ShadowDom for modern browsers and ShadyDOM polyfill for IE #2133

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

panthershark
Copy link

Overview

I'm wrapping quill with a webcomponent so it can be used with common diff/patch libraries (we are using Elm, but this should be possible with any diff/patch implementation). In order to make things work correctly with ShadowDOM, all of the global stuff needs to be refactored. This is what I attempted to do here.

Changes

  • Remove all assumptions that there is only 1 document and that document is global.
  • Separate the assumption that document is the only thing that implements getSelection() see MDN docs
  • Initialize with sensible defaults that do not change the current global assumptions
  • Refactor the global listeners so they avoid using the DOM to store the quill instance node.__quill it assumes things that break ShadowDOM and the separation which is intended.

How do we ge this published?

I'm not sure how to trigger all of the build/test stuff in this library or what processes y'all use to get a release published. If there is anything I can do to help with getting this published, please LMK.

}

destroy() {
this.globalListeners.forEach(({ eventName, target, handler }) => target.removeEventListener(eventName, handler));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the case where someone uses this globally in plainJS, the destroy function needs to be called.

  • Update docs to require users to call destroy
  • Setup some kind of detach listener which calls destroy

This is probably worth a conversation.


this.document = document;
this.selectionRoot = selectionRoot;
this.options = expandConfig(container, options, selectionRoot);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new optional parameters, this is what the initialization in a webcomponent looks like (with simplification b/c of the HTMLElement lifecycle)

class QuillEditor extends HTMLElement {
  constructor() {
    super();
    const root = this.attachShadow({ mode: 'open' });
    root.innerHTML = `<div id="editor"></div>`;

    const editor=  root.getElementById('#editor');
    const quill = new Quill(editor, {
      modules: { .. },
      theme:  'snow',
      document: root.ownerDocument,
      selectionRoot: root
    });
  }
}

@@ -73,7 +78,7 @@ class Quill {
this.root.classList.add('ql-blank');
this.root.setAttribute('data-gramm', false);
this.scrollingContainer = this.options.scrollingContainer || this.root;
this.emitter = new Emitter();
this.emitter = new Emitter({ selectionRoot });
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectionRoot can be a Document or ShadowRoot. If a Document is passed when using a webcomponent, then it will never select any of the nodes inside ShadowDOM and all of the selection/intersection validation inside of Quill breaks.

}

return window.document;
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above function should provide a reasonable fallback for use cases where

  • it is assumed that there is one document
  • ShadyDOM is polyfilled in IE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant