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

Make Quill instances detachable and reconfigurable #4402

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/quill/src/blots/scroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import type { Blot, Parent, EmbedBlot, ParentBlot, Registry } from 'parchment';
import Delta, { AttributeMap, Op } from 'quill-delta';
import Emitter from '../core/emitter.js';
import type { EmitterSource } from '../core/emitter.js';
import { findOrCreateSubscriber } from '../core/subscriber.js';
import type { Subscriber } from '../core/subscriber.js';
import Block, { BlockEmbed, bubbleFormats } from './block.js';
import Break from './break.js';
import Container from './container.js';
Expand Down Expand Up @@ -35,6 +37,7 @@ class Scroll extends ScrollBlot {
static defaultChild = Block;
static allowedChildren = [Block, BlockEmbed, Container];

subscriber: Subscriber;
emitter: Emitter;
batch: false | MutationRecord[];

Expand All @@ -44,11 +47,18 @@ class Scroll extends ScrollBlot {
{ emitter }: { emitter: Emitter },
) {
super(registry, domNode);
this.subscriber = findOrCreateSubscriber(this.domNode);
this.emitter = emitter;
this.batch = false;
this.optimize();
this.enable();
this.domNode.addEventListener('dragstart', (e) => this.handleDragStart(e));
const { subscriber } = this;
subscriber.on(this, domNode, 'dragstart', (e) => this.handleDragStart(e));
}

detach() {
super.detach();
this.subscriber.removeSourceListeners(this);
}

batchStart() {
Expand Down
1 change: 1 addition & 0 deletions packages/quill/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import Input from './modules/input.js';
import UINode from './modules/uiNode.js';

export { default as Module } from './core/module.js';
export { findOrCreateSubscriber } from './core/subscriber.js';
export { Delta, Op, OpIterator, AttributeMap, Parchment, Range };
export type {
Bounds,
Expand Down
6 changes: 4 additions & 2 deletions packages/quill/src/core/composition.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Embed from '../blots/embed.js';
import type Scroll from '../blots/scroll.js';
import Emitter from './emitter.js';
import { findOrCreateSubscriber } from './subscriber.js';

class Composition {
isComposing = false;
Expand All @@ -13,13 +14,14 @@ class Composition {
}

private setupListeners() {
this.scroll.domNode.addEventListener('compositionstart', (event) => {
const subscriber = findOrCreateSubscriber(this.scroll.domNode);
subscriber.on(this, this.scroll.domNode, 'compositionstart', (event) => {
if (!this.isComposing) {
this.handleCompositionStart(event);
}
});

this.scroll.domNode.addEventListener('compositionend', (event) => {
subscriber.on(this, this.scroll.domNode, 'compositionend', (event) => {
if (this.isComposing) {
// Webkit makes DOM changes after compositionend, so we use microtask to
// ensure the order.
Expand Down
8 changes: 8 additions & 0 deletions packages/quill/src/core/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ class Emitter extends EventEmitter<string> {
}
this.domListeners[eventName].push({ node, handler });
}

ignoreDOM() {
this.domListeners = {};
}

getDomListeners(): Record<string, { node: Node; handler: Function }[]> {
return { ...this.domListeners };
}
}

export type EmitterSource =
Expand Down
2 changes: 2 additions & 0 deletions packages/quill/src/core/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ abstract class Module<T extends {} = {}> {
public quill: Quill,
protected options: Partial<T> = {},
) {}

detach() {}
}

export default Module;
57 changes: 43 additions & 14 deletions packages/quill/src/core/quill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type { DebugLevel } from './logger.js';
import Module from './module.js';
import Selection, { Range } from './selection.js';
import type { Bounds } from './selection.js';
import { findOrCreateSubscriber } from './subscriber.js';
import Composition from './composition.js';
import Theme from './theme.js';
import type { ThemeConstructor } from './theme.js';
Expand Down Expand Up @@ -193,24 +194,48 @@ class Quill {
options: ExpandedQuillOptions;

constructor(container: HTMLElement | string, options: QuillOptions = {}) {
this.options = expandConfig(container, options);
this.container = this.options.container;
if (this.container == null) {
debug.error('Invalid Quill container', container);
return;
this.setup(container, options);
}

configure(options: QuillOptions = {}) {
singintime marked this conversation as resolved.
Show resolved Hide resolved
this.setup(this.container, options);
}

detach(): boolean {
if (!this.root) {
return false;
}
if (this.options.debug) {
Quill.debug(this.options.debug);

const subscriber = findOrCreateSubscriber(this.root);
subscriber.removeAllListeners();
this.emitter.removeAllListeners();
this.emitter.ignoreDOM();
this.scroll.detach();
this.theme.detach();
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if I'm wrong but this method does not appear to fully tear down passed in options, modules, etc. It seems to me that this method should return the quill instance to "factory settings" no?

Additionally, any dynamically changed things (like calling quill.keyboard.addBinding) should also be cleaned up

Copy link
Author

Choose a reason for hiding this comment

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

This method is not supposed to restore the quill instance to "factory settings", that would be the job of configure() called without arguments. detach() makes the Quill instance unresponsive to all events and user actions.

This is also why I didn't reset dynamically changed modules (keyboard, clipboard, history, etc.): a detached Quill instance is "dead", and not meant to be used anymore until reconfigured via the configure() method

}

private setup(container: HTMLElement | string, options: QuillOptions) {
this.options = expandConfig(container, options);
Quill.debug(this.options.debug ?? false);
let html = '';
if (!this.detach()) {
this.container = this.options.container;
if (this.container == null) {
debug.error('Invalid Quill container', container);
return;
}
html = this.container.innerHTML.trim();
this.container.classList.add('ql-container');
this.container.innerHTML = '';
instances.set(this.container, this);
this.root = this.addContainer('ql-editor');
this.root.classList.add('ql-blank');
this.emitter = new Emitter();
}
const html = this.container.innerHTML.trim();
this.container.classList.add('ql-container');
this.container.innerHTML = '';
instances.set(this.container, this);
this.root = this.addContainer('ql-editor');
this.root.classList.add('ql-blank');
this.emitter = new Emitter();
const scrollBlotName = Parchment.ScrollBlot.blotName;
const ScrollBlot = this.options.registry.query(scrollBlotName);
console.log(ScrollBlot);
Copy link
Contributor

Choose a reason for hiding this comment

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

🗑️ ?

if (!ScrollBlot || !('blotName' in ScrollBlot)) {
throw new Error(
`Cannot initialize Quill without "${scrollBlotName}" blot`,
Expand Down Expand Up @@ -272,9 +297,13 @@ class Quill {
this.history.clear();
if (this.options.placeholder) {
this.root.setAttribute('data-placeholder', this.options.placeholder);
} else {
this.root.removeAttribute('data-placeholder');
}
if (this.options.readOnly) {
this.disable();
} else {
this.enable();
}
this.allowReadOnlyEdits = false;
}
Expand Down
131 changes: 131 additions & 0 deletions packages/quill/src/core/subscriber.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import logger from './logger.js';

const debug = logger('quill:subscriber');

/**
* Any object with a named constructor can request an event subscription.
*/
interface Source {
constructor: { name: string };
}

/**
* A subscription to an event listener with an originating object,
* an event target, an event type, a handler function for the event,
* and some optional configuration.
*/
interface Subscription {
source: Source;
target: EventTarget;
event: string;
handler: EventListenerOrEventListenerObject;
options?: boolean | AddEventListenerOptions;
}

const subscribers = new WeakMap<object, Subscriber>();

/**
* Gets the Subscriber instance bound to the given object.
* Creates a new one if the binding does not exist yet.
*/
export function findOrCreateSubscriber(object: Source): Subscriber {
let subscriber = subscribers.get(object);
if (!subscriber) {
debug.info(`Creating new Subscriber for ${object.constructor.name}`);
subscriber = new Subscriber();
subscribers.set(object, subscriber);
}
return subscriber;
}

/**
* Keeps track of subscriptions to event listeners,
* to enable future bulk unsubscription.
*/
class Subscriber {
private subscriptions: Subscription[];

constructor() {
this.subscriptions = [];
}

/**
* Get a copy of the current subscriptions.
*/
getSubscriptions(): Subscription[] {
return [...this.subscriptions];
}

/**
* Proxy to target.addEventListener()
*/
on<T extends keyof DocumentEventMap>(
source: Source,
target: Document,
event: T,
handler: (ev: DocumentEventMap[T]) => void,
options?: boolean | AddEventListenerOptions,
): void;
on<T extends keyof HTMLElementEventMap>(
source: Source,
target: HTMLElement,
event: T,
handler: (ev: HTMLElementEventMap[T]) => void,
options?: boolean | AddEventListenerOptions,
): void;
on(
source: Source,
target: EventTarget,
event: string,
handler: EventListenerOrEventListenerObject,
options?: boolean | AddEventListenerOptions,
) {
target.addEventListener(event, handler, options);
this.subscriptions.push({ source, target, event, handler, options });
}

/**
* Proxy to target.removeEventListener()
*/
off(
target: Element,
event: string,
handler: EventListenerOrEventListenerObject,
options?: boolean | AddEventListenerOptions,
) {
target.removeEventListener(event, handler, options);
this.subscriptions = this.subscriptions.filter(
(subscription) =>
subscription.target !== target ||
subscription.event !== event ||
subscription.handler !== handler ||
subscription.options !== options,
);
}

/**
* Remove all event subscriptions originated by the given source.
*/
removeSourceListeners(source: Source) {
this.subscriptions
.filter((subscription) => subscription.source === source)
.forEach(({ target, event, handler, options }) => {
target.removeEventListener(event, handler, options);
});
this.subscriptions = this.subscriptions.filter(
(subscription) => subscription.source !== source,
);
}

/**
* Remove all event subscriptions for all sources.
*/
removeAllListeners() {
this.subscriptions.forEach(({ target, event, handler, options }) => {
target.removeEventListener(event, handler, options);
});
this.subscriptions = [];
}
}

export type { Subscriber };
9 changes: 9 additions & 0 deletions packages/quill/src/core/theme.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type Quill from '../core.js';
import Module from '../core/module.js';
import type Clipboard from '../modules/clipboard.js';
import type History from '../modules/history.js';
import type Keyboard from '../modules/keyboard.js';
Expand Down Expand Up @@ -27,6 +28,14 @@ class Theme {
protected options: ThemeOptions,
) {}

detach() {
Object.values(this.modules).forEach((module) => {
if (module instanceof Module) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if instead a better check might be if ('detach' in module). You can register things with quill that aren't literally subclasses of Module

module.detach();
}
});
}

init() {
Object.keys(this.options.modules).forEach((name) => {
if (this.modules[name] == null) {
Expand Down
14 changes: 12 additions & 2 deletions packages/quill/src/formats/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import Block from '../blots/block.js';
import Container from '../blots/container.js';
import type Scroll from '../blots/scroll.js';
import Quill from '../core/quill.js';
import { findOrCreateSubscriber } from '../core/subscriber.js';
import type { Subscriber } from '../core/subscriber.js';

class ListContainer extends Container {}
ListContainer.blotName = 'list-container';
Expand All @@ -22,8 +24,11 @@ class ListItem extends Block {
Quill.register(ListContainer);
}

subscriber: Subscriber;

constructor(scroll: Scroll, domNode: HTMLElement) {
super(scroll, domNode);
this.subscriber = findOrCreateSubscriber(this.scroll.domNode);
const ui = domNode.ownerDocument.createElement('span');
const listEventHandler = (e: Event) => {
if (!scroll.isEnabled()) return;
Expand All @@ -36,11 +41,16 @@ class ListItem extends Block {
e.preventDefault();
}
};
ui.addEventListener('mousedown', listEventHandler);
ui.addEventListener('touchstart', listEventHandler);
this.subscriber.on(this, ui, 'mousedown', listEventHandler);
this.subscriber.on(this, ui, 'touchstart', listEventHandler);
this.attachUI(ui);
}

detach() {
super.detach();
this.subscriber.removeSourceListeners(this);
}

format(name: string, value: string) {
if (name === this.statics.blotName && value) {
this.domNode.setAttribute('data-list', value);
Expand Down
11 changes: 6 additions & 5 deletions packages/quill/src/modules/clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import logger from '../core/logger.js';
import Module from '../core/module.js';
import Quill from '../core/quill.js';
import type { Range } from '../core/selection.js';
import { findOrCreateSubscriber } from '../core/subscriber.js';
import { AlignAttribute, AlignStyle } from '../formats/align.js';
import { BackgroundStyle } from '../formats/background.js';
import CodeBlock from '../formats/code.js';
Expand Down Expand Up @@ -80,11 +81,11 @@ class Clipboard extends Module<ClipboardOptions> {

constructor(quill: Quill, options: Partial<ClipboardOptions>) {
super(quill, options);
this.quill.root.addEventListener('copy', (e) =>
this.onCaptureCopy(e, false),
);
this.quill.root.addEventListener('cut', (e) => this.onCaptureCopy(e, true));
this.quill.root.addEventListener('paste', this.onCapturePaste.bind(this));
const { root } = this.quill;
const subscriber = findOrCreateSubscriber(root);
subscriber.on(this, root, 'copy', (e) => this.onCaptureCopy(e, false));
subscriber.on(this, root, 'cut', (e) => this.onCaptureCopy(e, true));
subscriber.on(this, root, 'paste', this.onCapturePaste.bind(this));
this.matchers = [];
CLIPBOARD_CONFIG.concat(this.options.matchers ?? []).forEach(
([selector, matcher]) => {
Expand Down
Loading