From 92556a03c62bce363c6ec253d5d93d0d99a8c830 Mon Sep 17 00:00:00 2001 From: Stefano Baldan Date: Fri, 30 Aug 2024 14:57:18 +0200 Subject: [PATCH 1/5] Reconfigurable Quill --- packages/quill/src/blots/scroll.ts | 11 +- packages/quill/src/core/composition.ts | 6 +- packages/quill/src/core/emitter.ts | 8 + packages/quill/src/core/quill.ts | 56 +++++-- packages/quill/src/core/subscriber.ts | 136 +++++++++++++++++ packages/quill/src/formats/list.ts | 15 +- packages/quill/src/modules/clipboard.ts | 11 +- packages/quill/src/modules/history.ts | 4 +- packages/quill/src/modules/input.ts | 4 +- packages/quill/src/modules/keyboard.ts | 4 +- packages/quill/src/modules/syntax.ts | 4 +- packages/quill/src/modules/toolbar.ts | 4 +- packages/quill/src/modules/uiNode.ts | 8 +- packages/quill/src/modules/uploader.ts | 4 +- packages/quill/src/themes/base.ts | 11 +- packages/quill/src/themes/bubble.ts | 5 +- packages/quill/src/themes/snow.ts | 46 +++--- packages/quill/src/ui/color-picker.ts | 9 +- packages/quill/src/ui/icon-picker.ts | 9 +- packages/quill/src/ui/picker.ts | 15 +- packages/quill/src/ui/tooltip.ts | 4 +- .../quill/test/unit/__helpers__/factory.ts | 6 +- packages/quill/test/unit/blots/scroll.spec.ts | 13 ++ .../core/__snapshots__/quill.spec.ts.snap | 83 +++++++++++ .../quill/test/unit/core/composition.spec.ts | 5 +- packages/quill/test/unit/core/editor.spec.ts | 5 +- packages/quill/test/unit/core/emitter.spec.ts | 18 +++ packages/quill/test/unit/core/quill.spec.ts | 49 +++++++ .../quill/test/unit/core/subscriber.spec.ts | 138 ++++++++++++++++++ packages/quill/test/unit/formats/list.spec.ts | 13 +- packages/quill/test/unit/ui/picker.spec.ts | 2 + 31 files changed, 627 insertions(+), 79 deletions(-) create mode 100644 packages/quill/src/core/subscriber.ts create mode 100644 packages/quill/test/unit/core/__snapshots__/quill.spec.ts.snap create mode 100644 packages/quill/test/unit/core/subscriber.spec.ts diff --git a/packages/quill/src/blots/scroll.ts b/packages/quill/src/blots/scroll.ts index 9975e12831..2d84482a6e 100644 --- a/packages/quill/src/blots/scroll.ts +++ b/packages/quill/src/blots/scroll.ts @@ -3,6 +3,7 @@ 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 { getSubscriber, Subscriber } from '../core/subscriber.js'; import Block, { BlockEmbed, bubbleFormats } from './block.js'; import Break from './break.js'; import Container from './container.js'; @@ -35,6 +36,7 @@ class Scroll extends ScrollBlot { static defaultChild = Block; static allowedChildren = [Block, BlockEmbed, Container]; + subscriber: Subscriber; emitter: Emitter; batch: false | MutationRecord[]; @@ -44,11 +46,18 @@ class Scroll extends ScrollBlot { { emitter }: { emitter: Emitter }, ) { super(registry, domNode); + this.subscriber = getSubscriber(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() { diff --git a/packages/quill/src/core/composition.ts b/packages/quill/src/core/composition.ts index 2a7038b46e..a553edebea 100644 --- a/packages/quill/src/core/composition.ts +++ b/packages/quill/src/core/composition.ts @@ -1,6 +1,7 @@ import Embed from '../blots/embed.js'; import type Scroll from '../blots/scroll.js'; import Emitter from './emitter.js'; +import { getSubscriber } from './subscriber.js'; class Composition { isComposing = false; @@ -13,13 +14,14 @@ class Composition { } private setupListeners() { - this.scroll.domNode.addEventListener('compositionstart', (event) => { + const subscriber = getSubscriber(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. diff --git a/packages/quill/src/core/emitter.ts b/packages/quill/src/core/emitter.ts index 7e981ed47b..73d6ebdf85 100644 --- a/packages/quill/src/core/emitter.ts +++ b/packages/quill/src/core/emitter.ts @@ -67,6 +67,14 @@ class Emitter extends EventEmitter { } this.domListeners[eventName].push({ node, handler }); } + + ignoreDOM() { + this.domListeners = {}; + } + + getDomListeners(): Record { + return { ...this.domListeners }; + } } export type EmitterSource = diff --git a/packages/quill/src/core/quill.ts b/packages/quill/src/core/quill.ts index dae5267bcb..c11a771cf0 100644 --- a/packages/quill/src/core/quill.ts +++ b/packages/quill/src/core/quill.ts @@ -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 { createSubscriber, getSubscriber } from './subscriber.js'; import Composition from './composition.js'; import Theme from './theme.js'; import type { ThemeConstructor } from './theme.js'; @@ -193,22 +194,45 @@ 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 = {}) { + this.setup(this.container, options); + } + + detach(): boolean { + if (!this.root) { + return false; } - if (this.options.debug) { - Quill.debug(this.options.debug); + + const subscriber = getSubscriber(this.root); + subscriber.removeAllListeners(); + this.emitter.removeAllListeners(); + this.emitter.ignoreDOM(); + this.scroll.detach(); + return true; + } + + 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'); + createSubscriber(this.root); + 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); if (!ScrollBlot || !('blotName' in ScrollBlot)) { @@ -272,9 +296,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; } diff --git a/packages/quill/src/core/subscriber.ts b/packages/quill/src/core/subscriber.ts new file mode 100644 index 0000000000..4754a6aefc --- /dev/null +++ b/packages/quill/src/core/subscriber.ts @@ -0,0 +1,136 @@ +/** + * 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(); + +/** + * Creates a Subscriber instance, and binds it to an object. + */ +export function createSubscriber(object: Source): Subscriber { + const subscriber = new Subscriber(); + subscribers.set(object, subscriber); + return subscriber; +} + +/** + * Gets the Subscriber instance bound to the given object. + * Throws an error if the binding does not exist. + */ +export function getSubscriber(object: Source): Subscriber { + const subscriber = subscribers.get(object); + if (!subscriber) { + throw new Error( + `Subscriber not found for object ${object.constructor.name}`, + ); + } + 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( + source: Source, + target: Document, + event: T, + handler: (ev: DocumentEventMap[T]) => void, + options?: boolean | AddEventListenerOptions, + ): void; + on( + 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 }; diff --git a/packages/quill/src/formats/list.ts b/packages/quill/src/formats/list.ts index ed1352fb19..4254f25550 100644 --- a/packages/quill/src/formats/list.ts +++ b/packages/quill/src/formats/list.ts @@ -1,7 +1,8 @@ import Block from '../blots/block.js'; import Container from '../blots/container.js'; -import type Scroll from '../blots/scroll.js'; +import Scroll from '../blots/scroll.js'; import Quill from '../core/quill.js'; +import { getSubscriber, Subscriber } from '../core/subscriber.js'; class ListContainer extends Container {} ListContainer.blotName = 'list-container'; @@ -22,8 +23,11 @@ class ListItem extends Block { Quill.register(ListContainer); } + subscriber: Subscriber; + constructor(scroll: Scroll, domNode: HTMLElement) { super(scroll, domNode); + this.subscriber = getSubscriber(this.scroll.domNode); const ui = domNode.ownerDocument.createElement('span'); const listEventHandler = (e: Event) => { if (!scroll.isEnabled()) return; @@ -36,11 +40,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); diff --git a/packages/quill/src/modules/clipboard.ts b/packages/quill/src/modules/clipboard.ts index e4c3f755b5..59d8a4c7a7 100644 --- a/packages/quill/src/modules/clipboard.ts +++ b/packages/quill/src/modules/clipboard.ts @@ -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 { getSubscriber } from '../core/subscriber.js'; import { AlignAttribute, AlignStyle } from '../formats/align.js'; import { BackgroundStyle } from '../formats/background.js'; import CodeBlock from '../formats/code.js'; @@ -80,11 +81,11 @@ class Clipboard extends Module { constructor(quill: Quill, options: Partial) { 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 = getSubscriber(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]) => { diff --git a/packages/quill/src/modules/history.ts b/packages/quill/src/modules/history.ts index 7bc5cdb689..6fb018d9c2 100644 --- a/packages/quill/src/modules/history.ts +++ b/packages/quill/src/modules/history.ts @@ -4,6 +4,7 @@ import Module from '../core/module.js'; import Quill from '../core/quill.js'; import type Scroll from '../blots/scroll.js'; import type { Range } from '../core/selection.js'; +import { getSubscriber } from '../core/subscriber.js'; export interface HistoryOptions { userOnly: boolean; @@ -71,7 +72,8 @@ class History extends Module { ); } - this.quill.root.addEventListener('beforeinput', (event) => { + const subscriber = getSubscriber(this.quill.root); + subscriber.on(this, this.quill.root, 'beforeinput', (event) => { if (event.inputType === 'historyUndo') { this.undo(); event.preventDefault(); diff --git a/packages/quill/src/modules/input.ts b/packages/quill/src/modules/input.ts index c9b7e83eaf..990b0df630 100644 --- a/packages/quill/src/modules/input.ts +++ b/packages/quill/src/modules/input.ts @@ -2,6 +2,7 @@ import Delta from 'quill-delta'; import Module from '../core/module.js'; import Quill from '../core/quill.js'; import type { Range } from '../core/selection.js'; +import { getSubscriber } from '../core/subscriber.js'; import { deleteRange } from './keyboard.js'; const INSERT_TYPES = ['insertText', 'insertReplacementText']; @@ -10,7 +11,8 @@ class Input extends Module { constructor(quill: Quill, options: Record) { super(quill, options); - quill.root.addEventListener('beforeinput', (event) => { + const subscriber = getSubscriber(this.quill.root); + subscriber.on(this, this.quill.root, 'beforeinput', (event) => { this.handleBeforeInput(event); }); diff --git a/packages/quill/src/modules/keyboard.ts b/packages/quill/src/modules/keyboard.ts index 7941b370f5..da426007a1 100644 --- a/packages/quill/src/modules/keyboard.ts +++ b/packages/quill/src/modules/keyboard.ts @@ -7,6 +7,7 @@ import logger from '../core/logger.js'; import Module from '../core/module.js'; import type { BlockEmbed } from '../blots/block.js'; import type { Range } from '../core/selection.js'; +import { getSubscriber } from '../core/subscriber.js'; const debug = logger('quill:keyboard'); @@ -171,7 +172,8 @@ class Keyboard extends Module { } listen() { - this.quill.root.addEventListener('keydown', (evt) => { + const subscriber = getSubscriber(this.quill.root); + subscriber.on(this, this.quill.root, 'keydown', (evt) => { if (evt.defaultPrevented || evt.isComposing) return; // evt.isComposing is false when pressing Enter/Backspace when composing in Safari diff --git a/packages/quill/src/modules/syntax.ts b/packages/quill/src/modules/syntax.ts index da99fdc41d..4307c3760c 100644 --- a/packages/quill/src/modules/syntax.ts +++ b/packages/quill/src/modules/syntax.ts @@ -4,6 +4,7 @@ import type { Blot, ScrollBlot } from 'parchment'; import Inline from '../blots/inline.js'; import Quill from '../core/quill.js'; import Module from '../core/module.js'; +import { getSubscriber } from '../core/subscriber.js'; import { blockDelta } from '../blots/block.js'; import BreakBlot from '../blots/break.js'; import CursorBlot from '../blots/cursor.js'; @@ -243,7 +244,8 @@ class Syntax extends Module { option.setAttribute('value', key); select.appendChild(option); }); - select.addEventListener('change', () => { + const subscriber = getSubscriber(this.quill.root); + subscriber.on(this, select, 'change', () => { blot.format(SyntaxCodeBlock.blotName, select.value); this.quill.root.focus(); // Prevent scrolling this.highlight(blot, true); diff --git a/packages/quill/src/modules/toolbar.ts b/packages/quill/src/modules/toolbar.ts index a178d25936..721471c9e9 100644 --- a/packages/quill/src/modules/toolbar.ts +++ b/packages/quill/src/modules/toolbar.ts @@ -4,6 +4,7 @@ import Quill from '../core/quill.js'; import logger from '../core/logger.js'; import Module from '../core/module.js'; import type { Range } from '../core/selection.js'; +import { getSubscriber } from '../core/subscriber.js'; const debug = logger('quill:toolbar'); @@ -88,7 +89,8 @@ class Toolbar extends Module { return; } const eventName = input.tagName === 'SELECT' ? 'change' : 'click'; - input.addEventListener(eventName, (e) => { + const subscriber = getSubscriber(this.quill.root); + subscriber.on(this, input, eventName, (e) => { let value; if (input.tagName === 'SELECT') { // @ts-expect-error diff --git a/packages/quill/src/modules/uiNode.ts b/packages/quill/src/modules/uiNode.ts index 780713a3ba..c098eab3d2 100644 --- a/packages/quill/src/modules/uiNode.ts +++ b/packages/quill/src/modules/uiNode.ts @@ -1,6 +1,7 @@ import { ParentBlot } from 'parchment'; import Module from '../core/module.js'; import Quill from '../core/quill.js'; +import { getSubscriber, Subscriber } from '../core/subscriber.js'; const isMac = /Mac/i.test(navigator.platform); @@ -28,12 +29,13 @@ const canMoveCaretBeforeUINode = (event: KeyboardEvent) => { }; class UINode extends Module { + subscriber: Subscriber; isListening = false; selectionChangeDeadline = 0; constructor(quill: Quill, options: Record) { super(quill, options); - + this.subscriber = getSubscriber(this.quill.root); this.handleArrowKeys(); this.handleNavigationShortcuts(); } @@ -67,7 +69,7 @@ class UINode extends Module { } private handleNavigationShortcuts() { - this.quill.root.addEventListener('keydown', (event) => { + this.subscriber.on(this, this.quill.root, 'keydown', (event) => { if (!event.defaultPrevented && canMoveCaretBeforeUINode(event)) { this.ensureListeningToSelectionChange(); } @@ -94,7 +96,7 @@ class UINode extends Module { } }; - document.addEventListener('selectionchange', listener, { + this.subscriber.on(this, document, 'selectionchange', listener, { once: true, }); } diff --git a/packages/quill/src/modules/uploader.ts b/packages/quill/src/modules/uploader.ts index c53058c51b..93eb98ff13 100644 --- a/packages/quill/src/modules/uploader.ts +++ b/packages/quill/src/modules/uploader.ts @@ -3,6 +3,7 @@ import type Quill from '../core/quill.js'; import Emitter from '../core/emitter.js'; import Module from '../core/module.js'; import type { Range } from '../core/selection.js'; +import { getSubscriber } from '../core/subscriber.js'; interface UploaderOptions { mimetypes: string[]; @@ -14,7 +15,8 @@ class Uploader extends Module { constructor(quill: Quill, options: Partial) { super(quill, options); - quill.root.addEventListener('drop', (e) => { + const subscriber = getSubscriber(this.quill.root); + subscriber.on(this, this.quill.root, 'drop', (e) => { e.preventDefault(); let native: ReturnType | null = null; if (document.caretRangeFromPoint) { diff --git a/packages/quill/src/themes/base.ts b/packages/quill/src/themes/base.ts index 3caf771ad1..d762233e5b 100644 --- a/packages/quill/src/themes/base.ts +++ b/packages/quill/src/themes/base.ts @@ -1,6 +1,7 @@ import { merge } from 'lodash-es'; import type Quill from '../core/quill.js'; import Emitter from '../core/emitter.js'; +import { getSubscriber } from '../core/subscriber.js'; import Theme from '../core/theme.js'; import type { ThemeOptions } from '../core/theme.js'; import ColorPicker from '../ui/color-picker.js'; @@ -141,13 +142,14 @@ class BaseTheme extends Theme { selects: NodeListOf, icons: Record>, ) { + const subscriber = getSubscriber(this.quill.root); this.pickers = Array.from(selects).map((select) => { if (select.classList.contains('ql-align')) { if (select.querySelector('option') == null) { fillSelect(select, ALIGNS); } if (typeof icons.align === 'object') { - return new IconPicker(select, icons.align); + return new IconPicker(select, subscriber, icons.align); } } if ( @@ -164,7 +166,7 @@ class BaseTheme extends Theme { format === 'background' ? '#ffffff' : '#000000', ); } - return new ColorPicker(select, icons[format] as string); + return new ColorPicker(select, subscriber, icons[format] as string); } if (select.querySelector('option') == null) { if (select.classList.contains('ql-font')) { @@ -175,7 +177,7 @@ class BaseTheme extends Theme { fillSelect(select, SIZES); } } - return new Picker(select); + return new Picker(select, subscriber); }); const update = () => { this.pickers.forEach((picker) => { @@ -232,8 +234,9 @@ class BaseTooltip extends Tooltip { } listen() { + const subscriber = getSubscriber(this.quill.root); // @ts-expect-error Fix me later - this.textbox.addEventListener('keydown', (event) => { + subscriber.on(this, this.textbox, 'keydown', (event) => { if (event.key === 'Enter') { this.save(); event.preventDefault(); diff --git a/packages/quill/src/themes/bubble.ts b/packages/quill/src/themes/bubble.ts index 5bb8519141..c5c001d144 100644 --- a/packages/quill/src/themes/bubble.ts +++ b/packages/quill/src/themes/bubble.ts @@ -3,6 +3,7 @@ import Emitter from '../core/emitter.js'; import BaseTheme, { BaseTooltip } from './base.js'; import { Range } from '../core/selection.js'; import type { Bounds } from '../core/selection.js'; +import { getSubscriber } from '../core/subscriber.js'; import icons from '../ui/icons.js'; import Quill from '../core/quill.js'; import type { ThemeOptions } from '../core/theme.js'; @@ -69,8 +70,10 @@ class BubbleTooltip extends BaseTooltip { listen() { super.listen(); + const subscriber = getSubscriber(this.quill.root); + const closeNode = this.root.querySelector('.ql-close'); // @ts-expect-error Fix me later - this.root.querySelector('.ql-close').addEventListener('click', () => { + subscriber.on(this, closeNode, 'click', () => { this.root.classList.remove('ql-editing'); }); this.quill.on(Emitter.events.SCROLL_OPTIMIZE, () => { diff --git a/packages/quill/src/themes/snow.ts b/packages/quill/src/themes/snow.ts index 153a834c26..0689942464 100644 --- a/packages/quill/src/themes/snow.ts +++ b/packages/quill/src/themes/snow.ts @@ -3,6 +3,7 @@ import Emitter from '../core/emitter.js'; import BaseTheme, { BaseTooltip } from './base.js'; import LinkBlot from '../formats/link.js'; import { Range } from '../core/selection.js'; +import { getSubscriber } from '../core/subscriber.js'; import icons from '../ui/icons.js'; import Quill from '../core/quill.js'; import type { Context } from '../modules/keyboard.js'; @@ -29,31 +30,30 @@ class SnowTooltip extends BaseTooltip { listen() { super.listen(); + const subscriber = getSubscriber(this.quill.root); + const actionNode = this.root.querySelector('a.ql-action'); // @ts-expect-error Fix me later - this.root - .querySelector('a.ql-action') - .addEventListener('click', (event) => { - if (this.root.classList.contains('ql-editing')) { - this.save(); - } else { - // @ts-expect-error Fix me later - this.edit('link', this.preview.textContent); - } - event.preventDefault(); - }); + subscriber.on(this, actionNode, 'click', (event) => { + if (this.root.classList.contains('ql-editing')) { + this.save(); + } else { + // @ts-expect-error Fix me later + this.edit('link', this.preview.textContent); + } + event.preventDefault(); + }); + const removeNode = this.root.querySelector('a.ql-remove'); // @ts-expect-error Fix me later - this.root - .querySelector('a.ql-remove') - .addEventListener('click', (event) => { - if (this.linkRange != null) { - const range = this.linkRange; - this.restoreFocus(); - this.quill.formatText(range, 'link', false, Emitter.sources.USER); - delete this.linkRange; - } - event.preventDefault(); - this.hide(); - }); + subscriber.on(this, removeNode, 'click', (event) => { + if (this.linkRange != null) { + const range = this.linkRange; + this.restoreFocus(); + this.quill.formatText(range, 'link', false, Emitter.sources.USER); + delete this.linkRange; + } + event.preventDefault(); + this.hide(); + }); this.quill.on( Emitter.events.SELECTION_CHANGE, (range, oldRange, source) => { diff --git a/packages/quill/src/ui/color-picker.ts b/packages/quill/src/ui/color-picker.ts index f585578135..7469b33675 100644 --- a/packages/quill/src/ui/color-picker.ts +++ b/packages/quill/src/ui/color-picker.ts @@ -1,8 +1,13 @@ +import { Subscriber } from '../core/subscriber.js'; import Picker from './picker.js'; class ColorPicker extends Picker { - constructor(select: HTMLSelectElement, label: string) { - super(select); + constructor( + select: HTMLSelectElement, + subscriber: Subscriber, + label: string, + ) { + super(select, subscriber); this.label.innerHTML = label; this.container.classList.add('ql-color-picker'); Array.from(this.container.querySelectorAll('.ql-picker-item')) diff --git a/packages/quill/src/ui/icon-picker.ts b/packages/quill/src/ui/icon-picker.ts index 883f21c607..f8179f6935 100644 --- a/packages/quill/src/ui/icon-picker.ts +++ b/packages/quill/src/ui/icon-picker.ts @@ -1,10 +1,15 @@ +import { Subscriber } from '../core/subscriber.js'; import Picker from './picker.js'; class IconPicker extends Picker { defaultItem: HTMLElement | null; - constructor(select: HTMLSelectElement, icons: Record) { - super(select); + constructor( + select: HTMLSelectElement, + subscriber: Subscriber, + icons: Record, + ) { + super(select, subscriber); this.container.classList.add('ql-icon-picker'); Array.from(this.container.querySelectorAll('.ql-picker-item')).forEach( (item) => { diff --git a/packages/quill/src/ui/picker.ts b/packages/quill/src/ui/picker.ts index d6b387c1a4..740c5673e7 100644 --- a/packages/quill/src/ui/picker.ts +++ b/packages/quill/src/ui/picker.ts @@ -1,4 +1,5 @@ import DropdownIcon from '../assets/icons/dropdown.svg'; +import { Subscriber } from '../core/subscriber'; let optionsCounter = 0; @@ -11,21 +12,23 @@ function toggleAriaAttribute(element: HTMLElement, attribute: string) { class Picker { select: HTMLSelectElement; + subscriber: Subscriber; container: HTMLElement; label: HTMLElement; - constructor(select: HTMLSelectElement) { + constructor(select: HTMLSelectElement, subscriber: Subscriber) { this.select = select; + this.subscriber = subscriber; this.container = document.createElement('span'); this.buildPicker(); this.select.style.display = 'none'; // @ts-expect-error Fix me later this.select.parentNode.insertBefore(this.container, this.select); - this.label.addEventListener('mousedown', () => { + this.subscriber.on(this, this.label, 'mousedown', () => { this.togglePicker(); }); - this.label.addEventListener('keydown', (event) => { + this.subscriber.on(this, this.label, 'keydown', (event) => { switch (event.key) { case 'Enter': this.togglePicker(); @@ -37,7 +40,7 @@ class Picker { default: } }); - this.select.addEventListener('change', this.update.bind(this)); + this.subscriber.on(this, this.select, 'change', this.update.bind(this)); } togglePicker() { @@ -61,10 +64,10 @@ class Picker { if (option.textContent) { item.setAttribute('data-label', option.textContent); } - item.addEventListener('click', () => { + this.subscriber.on(this, item, 'click', () => { this.selectItem(item, true); }); - item.addEventListener('keydown', (event) => { + this.subscriber.on(this, item, 'keydown', (event) => { switch (event.key) { case 'Enter': this.selectItem(item, true); diff --git a/packages/quill/src/ui/tooltip.ts b/packages/quill/src/ui/tooltip.ts index 07bf0013f8..d493a6baed 100644 --- a/packages/quill/src/ui/tooltip.ts +++ b/packages/quill/src/ui/tooltip.ts @@ -1,5 +1,6 @@ import type Quill from '../core.js'; import type { Bounds } from '../core/selection.js'; +import { getSubscriber } from '../core/subscriber.js'; const isScrollable = (el: Element) => { const { overflowY } = getComputedStyle(el, null); @@ -18,7 +19,8 @@ class Tooltip { // @ts-expect-error this.root.innerHTML = this.constructor.TEMPLATE; if (isScrollable(this.quill.root)) { - this.quill.root.addEventListener('scroll', () => { + const subscriber = getSubscriber(this.quill.root); + subscriber.on(this, this.quill.root, 'scroll', () => { this.root.style.marginTop = `${-1 * this.quill.root.scrollTop}px`; }); } diff --git a/packages/quill/test/unit/__helpers__/factory.ts b/packages/quill/test/unit/__helpers__/factory.ts index 6fa4b7f49d..2b0f88079b 100644 --- a/packages/quill/test/unit/__helpers__/factory.ts +++ b/packages/quill/test/unit/__helpers__/factory.ts @@ -10,6 +10,7 @@ import ListItem, { ListContainer } from '../../../src/formats/list.js'; import Inline from '../../../src/blots/inline.js'; import Emitter from '../../../src/core/emitter.js'; import { normalizeHTML } from './utils.js'; +import { createSubscriber } from '../../../src/core/subscriber.js'; export const createRegistry = (formats: unknown[] = []) => { const registry = new Registry(); @@ -37,8 +38,7 @@ export const createScroll = ( const emitter = new Emitter(); const root = container.appendChild(document.createElement('div')); root.innerHTML = normalizeHTML(html); - const scroll = new Scroll(registry, root, { - emitter, - }); + createSubscriber(root); + const scroll = new Scroll(registry, root, { emitter }); return scroll; }; diff --git a/packages/quill/test/unit/blots/scroll.spec.ts b/packages/quill/test/unit/blots/scroll.spec.ts index a977373e44..3711e97728 100644 --- a/packages/quill/test/unit/blots/scroll.spec.ts +++ b/packages/quill/test/unit/blots/scroll.spec.ts @@ -8,12 +8,17 @@ import { createRegistry } from '../__helpers__/factory.js'; import { normalizeHTML, sleep } from '../__helpers__/utils.js'; import Underline from '../../../src/formats/underline.js'; import Strike from '../../../src/formats/strike.js'; +import { + createSubscriber, + getSubscriber, +} from '../../../src/core/subscriber.js'; const createScroll = (html: string) => { const emitter = new Emitter(); const registry = createRegistry([Underline, Strike]); const container = document.body.appendChild(document.createElement('div')); container.innerHTML = normalizeHTML(html); + createSubscriber(container); return new Scroll(registry, container, { emitter }); }; @@ -59,6 +64,14 @@ describe('Scroll', () => { expect(dragstart.preventDefault).toHaveBeenCalled(); }); + test('remove event listeners on detach', () => { + const scroll = createScroll('

Hello World!

'); + const subscriber = getSubscriber(scroll.domNode); + vitest.spyOn(subscriber, 'removeSourceListeners'); + scroll.detach(); + expect(subscriber.removeSourceListeners).toHaveBeenCalledWith(scroll); + }); + describe('leaf()', () => { test('text', () => { const scroll = createScroll('

Tests

'); diff --git a/packages/quill/test/unit/core/__snapshots__/quill.spec.ts.snap b/packages/quill/test/unit/core/__snapshots__/quill.spec.ts.snap new file mode 100644 index 0000000000..b12d0e3a14 --- /dev/null +++ b/packages/quill/test/unit/core/__snapshots__/quill.spec.ts.snap @@ -0,0 +1,83 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`Quill > construction > event listeners tracking 1`] = ` +[ + { + "event": "dragstart", + "handler": [Function], + "options": undefined, + "source": "Scroll", + "target": "HTMLDivElement", + }, + { + "event": "compositionstart", + "handler": [Function], + "options": undefined, + "source": "Composition", + "target": "HTMLDivElement", + }, + { + "event": "compositionend", + "handler": [Function], + "options": undefined, + "source": "Composition", + "target": "HTMLDivElement", + }, + { + "event": "keydown", + "handler": [Function], + "options": undefined, + "source": "Keyboard", + "target": "HTMLDivElement", + }, + { + "event": "copy", + "handler": [Function], + "options": undefined, + "source": "Clipboard", + "target": "HTMLDivElement", + }, + { + "event": "cut", + "handler": [Function], + "options": undefined, + "source": "Clipboard", + "target": "HTMLDivElement", + }, + { + "event": "paste", + "handler": [Function], + "options": undefined, + "source": "Clipboard", + "target": "HTMLDivElement", + }, + { + "event": "beforeinput", + "handler": [Function], + "options": undefined, + "source": "History", + "target": "HTMLDivElement", + }, + { + "event": "drop", + "handler": [Function], + "options": undefined, + "source": "Uploader", + "target": "HTMLDivElement", + }, + { + "event": "beforeinput", + "handler": [Function], + "options": undefined, + "source": "Input", + "target": "HTMLDivElement", + }, + { + "event": "keydown", + "handler": [Function], + "options": undefined, + "source": "UINode", + "target": "HTMLDivElement", + }, +] +`; diff --git a/packages/quill/test/unit/core/composition.spec.ts b/packages/quill/test/unit/core/composition.spec.ts index c306318599..a5bdf383b2 100644 --- a/packages/quill/test/unit/core/composition.spec.ts +++ b/packages/quill/test/unit/core/composition.spec.ts @@ -4,11 +4,14 @@ import Scroll from '../../../src/blots/scroll.js'; import { describe, expect, test, vitest } from 'vitest'; import { createRegistry } from '../__helpers__/factory.js'; import Quill from '../../../src/core.js'; +import { createSubscriber } from '../../../src/core/subscriber.js'; describe('Composition', () => { test('triggers events on compositionstart', async () => { const emitter = new Emitter(); - const scroll = new Scroll(createRegistry(), document.createElement('div'), { + const container = document.createElement('div'); + createSubscriber(container); + const scroll = new Scroll(createRegistry(), container, { emitter, }); new Composition(scroll, emitter); diff --git a/packages/quill/test/unit/core/editor.spec.ts b/packages/quill/test/unit/core/editor.spec.ts index 0c595332bb..dcc3a37358 100644 --- a/packages/quill/test/unit/core/editor.spec.ts +++ b/packages/quill/test/unit/core/editor.spec.ts @@ -27,6 +27,7 @@ import IndentClass from '../../../src/formats/indent.js'; import { ColorClass } from '../../../src/formats/color.js'; import Quill from '../../../src/core.js'; import { normalizeHTML } from '../__helpers__/utils.js'; +import { createSubscriber } from '../../../src/core/subscriber.js'; const createEditor = (html: string) => { const container = document.createElement('div'); @@ -880,8 +881,10 @@ describe('Editor', () => { const registry = new Registry(); registry.register(MyBlot, Block, Break, Text); + const container = document.createElement('div'); + createSubscriber(container); const editor = new Editor( - new Scroll(registry, document.createElement('div'), { + new Scroll(registry, container, { emitter: new Emitter(), }), ); diff --git a/packages/quill/test/unit/core/emitter.spec.ts b/packages/quill/test/unit/core/emitter.spec.ts index 35bccc7383..fadf6ffc39 100644 --- a/packages/quill/test/unit/core/emitter.spec.ts +++ b/packages/quill/test/unit/core/emitter.spec.ts @@ -35,4 +35,22 @@ describe('emitter', () => { document.body.click(); expect(calls).toEqual(2); }); + + test('ignoreDOM', () => { + const quill = new Quill(document.createElement('div')); + document.body.appendChild(quill.container); + + let calls = 0; + quill.emitter.listenDOM('click', document.body, () => { + calls += 1; + }); + + document.body.click(); + expect(calls).toEqual(1); + + quill.emitter.ignoreDOM(); + + document.body.click(); + expect(calls).toEqual(1); + }); }); diff --git a/packages/quill/test/unit/core/quill.spec.ts b/packages/quill/test/unit/core/quill.spec.ts index c128c0dc12..a24d95415a 100644 --- a/packages/quill/test/unit/core/quill.spec.ts +++ b/packages/quill/test/unit/core/quill.spec.ts @@ -14,6 +14,7 @@ import Quill, { import { Range } from '../../../src/core/selection.js'; import Snow from '../../../src/themes/snow.js'; import { normalizeHTML } from '../__helpers__/utils.js'; +import { getSubscriber } from '../../../src/core/subscriber.js'; const createContainer = (html: string | { html: string } = '') => { const container = document.createElement('div'); @@ -104,6 +105,20 @@ describe('Quill', () => { '"

Test

"', ); }); + + test('event listeners tracking', () => { + const container = createContainer(); + const quill = new Quill(container); + const subscriber = getSubscriber(quill.root); + const subscriptions = subscriber + .getSubscriptions() + .map((subscription) => ({ + ...subscription, + source: subscription.source.constructor.name, + target: subscription.target.constructor.name, + })); + expect(subscriptions).toMatchSnapshot(); + }); }); describe('api', () => { @@ -114,6 +129,29 @@ describe('Quill', () => { return { quill, oldDelta }; }; + test('configure', () => { + const { quill } = setup(); + const { options, root, scroll, theme } = quill; + const childNodes = [...root.childNodes]; + const modules = { ...quill.theme.modules }; + vitest.spyOn(quill, 'detach'); + quill.configure(); + // The new configuration detaches the old one + expect(quill.detach).toHaveBeenCalled(); + // The editor's HTML content is preserved + expect(quill.root).toBe(root); + childNodes.forEach((child, index) => { + expect(child).toBe(quill.root.childNodes[index]); + }); + // Options, scroll, theme, and modules are reinstantiated + expect(quill.options).not.toBe(options); + expect(quill.scroll).not.toBe(scroll); + expect(quill.theme).not.toBe(theme); + Object.keys(modules).forEach((key) => { + expect(quill.theme.modules[key]).not.toBe(modules[key]); + }); + }); + test('deleteText()', () => { const { quill, oldDelta } = setup(); quill.deleteText(3, 2); @@ -129,6 +167,17 @@ describe('Quill', () => { ); }); + test('detach()', () => { + const { quill } = setup(); + vitest.spyOn(quill.scroll, 'detach'); + quill.detach(); + const subscriber = getSubscriber(quill.root); + expect(subscriber.getSubscriptions().length).toEqual(0); + expect(quill.emitter.eventNames()).toEqual([]); + expect(quill.emitter.getDomListeners()).toEqual({}); + expect(quill.scroll.detach).toHaveBeenCalled(); + }); + test('format()', () => { const { quill, oldDelta } = setup(); quill.setSelection(3, 2); diff --git a/packages/quill/test/unit/core/subscriber.spec.ts b/packages/quill/test/unit/core/subscriber.spec.ts new file mode 100644 index 0000000000..4573446853 --- /dev/null +++ b/packages/quill/test/unit/core/subscriber.spec.ts @@ -0,0 +1,138 @@ +import { beforeEach, describe, expect, test, vitest } from 'vitest'; +import { createSubscriber, getSubscriber } from '../../../src/core/subscriber'; + +describe('Subscriber', () => { + class Test {} + let object: Test; + + beforeEach(() => { + object = new Test(); + }); + + describe('registration', () => { + test('maps a Subscriber to an object', () => { + const subscriber = createSubscriber(object); + expect(subscriber).toBeTruthy(); + expect(getSubscriber(object)).toBe(subscriber); + }); + + test('throws an error if no Subscriber is bound to an object', () => { + expect(() => getSubscriber(object)).toThrow( + 'Subscriber not found for object Test', + ); + }); + }); + + describe('on()', () => { + test('calls addEventListener on the target', () => { + const subscriber = createSubscriber(object); + const source = new Test(); + const target = document.createElement('div'); + const event = 'keydown'; + const handler = () => {}; + const options = {}; + vitest.spyOn(target, 'addEventListener'); + subscriber.on(source, target, event, handler, options); + expect(target.addEventListener).toHaveBeenCalledWith( + event, + handler, + options, + ); + }); + + test('keeps track of the subscription', () => { + const subscriber = createSubscriber(object); + const source = new Test(); + const target = document.createElement('div'); + const event = 'keydown'; + const handler = () => {}; + const options = {}; + subscriber.on(source, target, event, handler, options); + expect(subscriber.getSubscriptions()).toEqual([ + { source, target, event, handler, options }, + ]); + }); + }); + + describe('off()', () => { + test('calls removeEventListener on the target', () => { + const subscriber = createSubscriber(object); + const target = document.createElement('div'); + const event = 'keydown'; + const handler = () => {}; + const options = {}; + vitest.spyOn(target, 'removeEventListener'); + subscriber.off(target, event, handler, options); + expect(target.removeEventListener).toHaveBeenCalledWith( + event, + handler, + options, + ); + }); + + test('forgets the subscription', () => { + const subscriber = createSubscriber(object); + const source = new Test(); + const target = document.createElement('div'); + const event = 'keydown'; + const handler = () => {}; + const options = {}; + subscriber.on(source, target, event, handler, options); + subscriber.off(target, event, handler, options); + expect(subscriber.getSubscriptions()).toEqual([]); + }); + }); + + describe('removeSourceListeners()', () => { + test('removes all listeners related to a source', () => { + const subscriber = createSubscriber(object); + const source1 = new Test(); + const source2 = new Test(); + const target1 = document.createElement('div'); + const target2 = document.createElement('div'); + const event = 'keydown'; + const handler = () => {}; + vitest.spyOn(target1, 'removeEventListener'); + vitest.spyOn(target2, 'removeEventListener'); + subscriber.on(source1, target1, event, handler); + subscriber.on(source2, target2, event, handler); + subscriber.removeSourceListeners(source1); + expect(target1.removeEventListener).toHaveBeenCalledWith( + 'keydown', + handler, + undefined, + ); + expect(target2.removeEventListener).not.toHaveBeenCalled(); + expect(subscriber.getSubscriptions().length).toEqual(1); + expect(subscriber.getSubscriptions()[0].source).toEqual(source2); + }); + }); + + describe('removeAllListeners()', () => { + test('removes all listeners', () => { + const subscriber = createSubscriber(object); + const source1 = new Test(); + const source2 = new Test(); + const target1 = document.createElement('div'); + const target2 = document.createElement('div'); + const event = 'keydown'; + const handler = () => {}; + vitest.spyOn(target1, 'removeEventListener'); + vitest.spyOn(target2, 'removeEventListener'); + subscriber.on(source1, target1, event, handler); + subscriber.on(source2, target2, event, handler); + subscriber.removeAllListeners(); + expect(target1.removeEventListener).toHaveBeenCalledWith( + 'keydown', + handler, + undefined, + ); + expect(target2.removeEventListener).toHaveBeenCalledWith( + 'keydown', + handler, + undefined, + ); + expect(subscriber.getSubscriptions()).toEqual([]); + }); + }); +}); diff --git a/packages/quill/test/unit/formats/list.spec.ts b/packages/quill/test/unit/formats/list.spec.ts index a12f020706..37f45b052c 100644 --- a/packages/quill/test/unit/formats/list.spec.ts +++ b/packages/quill/test/unit/formats/list.spec.ts @@ -4,11 +4,13 @@ import { createRegistry, } from '../__helpers__/factory.js'; import Editor from '../../../src/core/editor.js'; -import { describe, expect, test } from 'vitest'; +import { describe, expect, test, vitest } from 'vitest'; import List, { ListContainer } from '../../../src/formats/list.js'; import IndentClass from '../../../src/formats/indent.js'; import { AlignClass } from '../../../src/formats/align.js'; import Video from '../../../src/formats/video.js'; +import { getSubscriber } from '../../../src/core/subscriber.js'; +import ListItem from '../../../src/formats/list.js'; const createScroll = (html: string) => baseCreateScroll( @@ -390,4 +392,13 @@ describe('List', () => { `); }); + + test('remove event listeners on detach', () => { + const scroll = createScroll('

Hello World!

'); + const listItem = new ListItem(scroll, scroll.domNode); + const subscriber = getSubscriber(scroll.domNode); + vitest.spyOn(subscriber, 'removeSourceListeners'); + listItem.detach(); + expect(subscriber.removeSourceListeners).toHaveBeenCalledWith(listItem); + }); }); diff --git a/packages/quill/test/unit/ui/picker.spec.ts b/packages/quill/test/unit/ui/picker.spec.ts index 2eb14e43a2..d3adf5f1dd 100644 --- a/packages/quill/test/unit/ui/picker.spec.ts +++ b/packages/quill/test/unit/ui/picker.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from 'vitest'; import Picker from '../../../src/ui/picker.js'; +import { createSubscriber } from '../../../src/core/subscriber.js'; describe('Picker', () => { const setup = () => { @@ -8,6 +9,7 @@ describe('Picker', () => { ''; const pickerSelectorInstance = new Picker( container.firstChild as HTMLSelectElement, + createSubscriber(container), ); const pickerSelector = container.querySelector('.ql-picker') as HTMLElement; return { container, pickerSelectorInstance, pickerSelector }; From 025e585d733138a6d8aa2e16e9ad3294ef807b27 Mon Sep 17 00:00:00 2001 From: Stefano Baldan Date: Wed, 4 Sep 2024 10:13:57 +0200 Subject: [PATCH 2/5] Addressing Ben's comments --- packages/quill/src/core/quill.ts | 3 +- packages/quill/src/core/subscriber.ts | 23 +++++------ .../quill/test/unit/__helpers__/factory.ts | 2 - packages/quill/test/unit/blots/scroll.spec.ts | 6 +-- .../quill/test/unit/core/composition.spec.ts | 2 - packages/quill/test/unit/core/editor.spec.ts | 2 - .../quill/test/unit/core/subscriber.spec.ts | 35 ++++++++++------- packages/quill/test/unit/ui/picker.spec.ts | 4 +- packages/website/content/docs/api.mdx | 38 +++++++++++++++++++ 9 files changed, 73 insertions(+), 42 deletions(-) diff --git a/packages/quill/src/core/quill.ts b/packages/quill/src/core/quill.ts index c11a771cf0..7b638da6ba 100644 --- a/packages/quill/src/core/quill.ts +++ b/packages/quill/src/core/quill.ts @@ -18,7 +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 { createSubscriber, getSubscriber } from './subscriber.js'; +import { getSubscriber } from './subscriber.js'; import Composition from './composition.js'; import Theme from './theme.js'; import type { ThemeConstructor } from './theme.js'; @@ -230,7 +230,6 @@ class Quill { instances.set(this.container, this); this.root = this.addContainer('ql-editor'); this.root.classList.add('ql-blank'); - createSubscriber(this.root); this.emitter = new Emitter(); } const scrollBlotName = Parchment.ScrollBlot.blotName; diff --git a/packages/quill/src/core/subscriber.ts b/packages/quill/src/core/subscriber.ts index 4754a6aefc..c306956570 100644 --- a/packages/quill/src/core/subscriber.ts +++ b/packages/quill/src/core/subscriber.ts @@ -1,3 +1,7 @@ +import logger from './logger.js'; + +const debug = logger('quill:subscriber'); + /** * Any object with a named constructor can request an event subscription. */ @@ -20,25 +24,16 @@ interface Subscription { const subscribers = new WeakMap(); -/** - * Creates a Subscriber instance, and binds it to an object. - */ -export function createSubscriber(object: Source): Subscriber { - const subscriber = new Subscriber(); - subscribers.set(object, subscriber); - return subscriber; -} - /** * Gets the Subscriber instance bound to the given object. - * Throws an error if the binding does not exist. + * Creates a new one if the binding does not exist yet. */ export function getSubscriber(object: Source): Subscriber { - const subscriber = subscribers.get(object); + let subscriber = subscribers.get(object); if (!subscriber) { - throw new Error( - `Subscriber not found for object ${object.constructor.name}`, - ); + debug.info(`Creating new Subscriber for ${object.constructor.name}`); + subscriber = new Subscriber(); + subscribers.set(object, subscriber); } return subscriber; } diff --git a/packages/quill/test/unit/__helpers__/factory.ts b/packages/quill/test/unit/__helpers__/factory.ts index 2b0f88079b..4b74f532fc 100644 --- a/packages/quill/test/unit/__helpers__/factory.ts +++ b/packages/quill/test/unit/__helpers__/factory.ts @@ -10,7 +10,6 @@ import ListItem, { ListContainer } from '../../../src/formats/list.js'; import Inline from '../../../src/blots/inline.js'; import Emitter from '../../../src/core/emitter.js'; import { normalizeHTML } from './utils.js'; -import { createSubscriber } from '../../../src/core/subscriber.js'; export const createRegistry = (formats: unknown[] = []) => { const registry = new Registry(); @@ -38,7 +37,6 @@ export const createScroll = ( const emitter = new Emitter(); const root = container.appendChild(document.createElement('div')); root.innerHTML = normalizeHTML(html); - createSubscriber(root); const scroll = new Scroll(registry, root, { emitter }); return scroll; }; diff --git a/packages/quill/test/unit/blots/scroll.spec.ts b/packages/quill/test/unit/blots/scroll.spec.ts index 3711e97728..d1c149b568 100644 --- a/packages/quill/test/unit/blots/scroll.spec.ts +++ b/packages/quill/test/unit/blots/scroll.spec.ts @@ -8,17 +8,13 @@ import { createRegistry } from '../__helpers__/factory.js'; import { normalizeHTML, sleep } from '../__helpers__/utils.js'; import Underline from '../../../src/formats/underline.js'; import Strike from '../../../src/formats/strike.js'; -import { - createSubscriber, - getSubscriber, -} from '../../../src/core/subscriber.js'; +import { getSubscriber } from '../../../src/core/subscriber.js'; const createScroll = (html: string) => { const emitter = new Emitter(); const registry = createRegistry([Underline, Strike]); const container = document.body.appendChild(document.createElement('div')); container.innerHTML = normalizeHTML(html); - createSubscriber(container); return new Scroll(registry, container, { emitter }); }; diff --git a/packages/quill/test/unit/core/composition.spec.ts b/packages/quill/test/unit/core/composition.spec.ts index a5bdf383b2..efb4e80978 100644 --- a/packages/quill/test/unit/core/composition.spec.ts +++ b/packages/quill/test/unit/core/composition.spec.ts @@ -4,13 +4,11 @@ import Scroll from '../../../src/blots/scroll.js'; import { describe, expect, test, vitest } from 'vitest'; import { createRegistry } from '../__helpers__/factory.js'; import Quill from '../../../src/core.js'; -import { createSubscriber } from '../../../src/core/subscriber.js'; describe('Composition', () => { test('triggers events on compositionstart', async () => { const emitter = new Emitter(); const container = document.createElement('div'); - createSubscriber(container); const scroll = new Scroll(createRegistry(), container, { emitter, }); diff --git a/packages/quill/test/unit/core/editor.spec.ts b/packages/quill/test/unit/core/editor.spec.ts index dcc3a37358..8cae1db2a8 100644 --- a/packages/quill/test/unit/core/editor.spec.ts +++ b/packages/quill/test/unit/core/editor.spec.ts @@ -27,7 +27,6 @@ import IndentClass from '../../../src/formats/indent.js'; import { ColorClass } from '../../../src/formats/color.js'; import Quill from '../../../src/core.js'; import { normalizeHTML } from '../__helpers__/utils.js'; -import { createSubscriber } from '../../../src/core/subscriber.js'; const createEditor = (html: string) => { const container = document.createElement('div'); @@ -882,7 +881,6 @@ describe('Editor', () => { const registry = new Registry(); registry.register(MyBlot, Block, Break, Text); const container = document.createElement('div'); - createSubscriber(container); const editor = new Editor( new Scroll(registry, container, { emitter: new Emitter(), diff --git a/packages/quill/test/unit/core/subscriber.spec.ts b/packages/quill/test/unit/core/subscriber.spec.ts index 4573446853..2f3b0edfef 100644 --- a/packages/quill/test/unit/core/subscriber.spec.ts +++ b/packages/quill/test/unit/core/subscriber.spec.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, test, vitest } from 'vitest'; -import { createSubscriber, getSubscriber } from '../../../src/core/subscriber'; +import logger from '../../../src/core/logger.js'; +import { getSubscriber } from '../../../src/core/subscriber.js'; describe('Subscriber', () => { class Test {} @@ -9,23 +10,31 @@ describe('Subscriber', () => { object = new Test(); }); - describe('registration', () => { + describe('getSubscriber()', () => { test('maps a Subscriber to an object', () => { - const subscriber = createSubscriber(object); - expect(subscriber).toBeTruthy(); + const subscriber = getSubscriber(object); expect(getSubscriber(object)).toBe(subscriber); + expect(getSubscriber(new Test())).not.toBe(subscriber); }); - test('throws an error if no Subscriber is bound to an object', () => { - expect(() => getSubscriber(object)).toThrow( - 'Subscriber not found for object Test', + test('logs the creation of a new Subscriber instance', () => { + logger.level('info'); + vitest.spyOn(console, 'info'); + getSubscriber(object); + expect(console.info).toHaveBeenCalledWith( + 'quill:subscriber', + 'Creating new Subscriber for Test', ); + getSubscriber(object); + expect(console.info).toHaveBeenCalledTimes(1); + getSubscriber(new Test()); + expect(console.info).toHaveBeenCalledTimes(2); }); }); describe('on()', () => { test('calls addEventListener on the target', () => { - const subscriber = createSubscriber(object); + const subscriber = getSubscriber(object); const source = new Test(); const target = document.createElement('div'); const event = 'keydown'; @@ -41,7 +50,7 @@ describe('Subscriber', () => { }); test('keeps track of the subscription', () => { - const subscriber = createSubscriber(object); + const subscriber = getSubscriber(object); const source = new Test(); const target = document.createElement('div'); const event = 'keydown'; @@ -56,7 +65,7 @@ describe('Subscriber', () => { describe('off()', () => { test('calls removeEventListener on the target', () => { - const subscriber = createSubscriber(object); + const subscriber = getSubscriber(object); const target = document.createElement('div'); const event = 'keydown'; const handler = () => {}; @@ -71,7 +80,7 @@ describe('Subscriber', () => { }); test('forgets the subscription', () => { - const subscriber = createSubscriber(object); + const subscriber = getSubscriber(object); const source = new Test(); const target = document.createElement('div'); const event = 'keydown'; @@ -85,7 +94,7 @@ describe('Subscriber', () => { describe('removeSourceListeners()', () => { test('removes all listeners related to a source', () => { - const subscriber = createSubscriber(object); + const subscriber = getSubscriber(object); const source1 = new Test(); const source2 = new Test(); const target1 = document.createElement('div'); @@ -110,7 +119,7 @@ describe('Subscriber', () => { describe('removeAllListeners()', () => { test('removes all listeners', () => { - const subscriber = createSubscriber(object); + const subscriber = getSubscriber(object); const source1 = new Test(); const source2 = new Test(); const target1 = document.createElement('div'); diff --git a/packages/quill/test/unit/ui/picker.spec.ts b/packages/quill/test/unit/ui/picker.spec.ts index d3adf5f1dd..5ebb877cac 100644 --- a/packages/quill/test/unit/ui/picker.spec.ts +++ b/packages/quill/test/unit/ui/picker.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from 'vitest'; import Picker from '../../../src/ui/picker.js'; -import { createSubscriber } from '../../../src/core/subscriber.js'; +import { getSubscriber } from '../../../src/core/subscriber.js'; describe('Picker', () => { const setup = () => { @@ -9,7 +9,7 @@ describe('Picker', () => { ''; const pickerSelectorInstance = new Picker( container.firstChild as HTMLSelectElement, - createSubscriber(container), + getSubscriber(container), ); const pickerSelector = container.querySelector('.ql-picker') as HTMLElement; return { container, pickerSelectorInstance, pickerSelector }; diff --git a/packages/website/content/docs/api.mdx b/packages/website/content/docs/api.mdx index 66d456eb76..9cb5e7cf42 100644 --- a/packages/website/content/docs/api.mdx +++ b/packages/website/content/docs/api.mdx @@ -21,6 +21,28 @@ title: API ## Content +### configure + +Applies a new [configuration](/docs/configuration/) to an existing instance of Quill. + +**Methods** + +```typescript +configure(options: QuillOptions = {}): void +``` + +**Example 1** + +```typescript +quill.configure(); +``` + +**Example 2** + +```typescript +quill.configure({ theme: 'bubble', readOnly: true }); +``` + ### deleteText Deletes text from the editor, returning a [Delta](/docs/delta/) representing the change. [Source](/docs/api/#events) may be `"user"`, `"api"`, or `"silent"`. Calls where the `source` is `"user"` when the editor is [disabled](#disable) are ignored. @@ -59,6 +81,22 @@ document.querySelector('#deleteButton').addEventListener('click', () => { }} /> +### detach + +Deactivates a Quill instance by removing all event subscriptions. Useful to cleanup the environment and prevent memory leaks. + +**Methods** + +```typescript +detach(): void +``` + +**Examples** + +```typescript +quill.detach(); +``` + ### getContents Retrieves contents of the editor, with formatting data, represented by a [Delta](/docs/delta/) object. From 947ab47eb8d331d227efd5e056a238287e415fb2 Mon Sep 17 00:00:00 2001 From: Stefano Baldan Date: Wed, 4 Sep 2024 10:17:40 +0200 Subject: [PATCH 3/5] Minor cleanup --- packages/quill/test/unit/core/composition.spec.ts | 3 +-- packages/quill/test/unit/core/editor.spec.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/quill/test/unit/core/composition.spec.ts b/packages/quill/test/unit/core/composition.spec.ts index efb4e80978..c306318599 100644 --- a/packages/quill/test/unit/core/composition.spec.ts +++ b/packages/quill/test/unit/core/composition.spec.ts @@ -8,8 +8,7 @@ import Quill from '../../../src/core.js'; describe('Composition', () => { test('triggers events on compositionstart', async () => { const emitter = new Emitter(); - const container = document.createElement('div'); - const scroll = new Scroll(createRegistry(), container, { + const scroll = new Scroll(createRegistry(), document.createElement('div'), { emitter, }); new Composition(scroll, emitter); diff --git a/packages/quill/test/unit/core/editor.spec.ts b/packages/quill/test/unit/core/editor.spec.ts index 8cae1db2a8..0c595332bb 100644 --- a/packages/quill/test/unit/core/editor.spec.ts +++ b/packages/quill/test/unit/core/editor.spec.ts @@ -880,9 +880,8 @@ describe('Editor', () => { const registry = new Registry(); registry.register(MyBlot, Block, Break, Text); - const container = document.createElement('div'); const editor = new Editor( - new Scroll(registry, container, { + new Scroll(registry, document.createElement('div'), { emitter: new Emitter(), }), ); From eb26cf9dca2c6b5538d1acf2316aa863322caf77 Mon Sep 17 00:00:00 2001 From: Stefano Baldan Date: Wed, 4 Sep 2024 19:02:52 +0200 Subject: [PATCH 4/5] Applying Ben's suggestions --- packages/quill/src/blots/scroll.ts | 5 +- packages/quill/src/core.ts | 1 + packages/quill/src/core/composition.ts | 4 +- packages/quill/src/core/module.ts | 2 + packages/quill/src/core/quill.ts | 6 +- packages/quill/src/core/subscriber.ts | 2 +- packages/quill/src/core/theme.ts | 9 ++ packages/quill/src/formats/list.ts | 7 +- packages/quill/src/modules/clipboard.ts | 4 +- packages/quill/src/modules/history.ts | 4 +- packages/quill/src/modules/input.ts | 4 +- packages/quill/src/modules/keyboard.ts | 4 +- packages/quill/src/modules/syntax.ts | 4 +- packages/quill/src/modules/toolbar.ts | 4 +- packages/quill/src/modules/uiNode.ts | 5 +- packages/quill/src/modules/uploader.ts | 4 +- packages/quill/src/quill.ts | 1 + packages/quill/src/themes/base.ts | 6 +- packages/quill/src/themes/bubble.ts | 4 +- packages/quill/src/themes/snow.ts | 4 +- packages/quill/src/ui/tooltip.ts | 4 +- packages/quill/test/unit/blots/scroll.spec.ts | 4 +- packages/quill/test/unit/core/quill.spec.ts | 12 ++- .../quill/test/unit/core/subscriber.spec.ts | 28 +++---- packages/quill/test/unit/formats/list.spec.ts | 7 +- packages/quill/test/unit/ui/picker.spec.ts | 4 +- packages/website/content/docs/api.mdx | 83 ++++++++++--------- .../docs/guides/building-a-custom-module.mdx | 39 +++++++++ 28 files changed, 166 insertions(+), 99 deletions(-) diff --git a/packages/quill/src/blots/scroll.ts b/packages/quill/src/blots/scroll.ts index 2d84482a6e..ef0ed3fb9d 100644 --- a/packages/quill/src/blots/scroll.ts +++ b/packages/quill/src/blots/scroll.ts @@ -3,7 +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 { getSubscriber, Subscriber } from '../core/subscriber.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'; @@ -46,7 +47,7 @@ class Scroll extends ScrollBlot { { emitter }: { emitter: Emitter }, ) { super(registry, domNode); - this.subscriber = getSubscriber(this.domNode); + this.subscriber = findOrCreateSubscriber(this.domNode); this.emitter = emitter; this.batch = false; this.optimize(); diff --git a/packages/quill/src/core.ts b/packages/quill/src/core.ts index 5b8946ad5f..fed77fcfda 100644 --- a/packages/quill/src/core.ts +++ b/packages/quill/src/core.ts @@ -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, diff --git a/packages/quill/src/core/composition.ts b/packages/quill/src/core/composition.ts index a553edebea..39a73146a8 100644 --- a/packages/quill/src/core/composition.ts +++ b/packages/quill/src/core/composition.ts @@ -1,7 +1,7 @@ import Embed from '../blots/embed.js'; import type Scroll from '../blots/scroll.js'; import Emitter from './emitter.js'; -import { getSubscriber } from './subscriber.js'; +import { findOrCreateSubscriber } from './subscriber.js'; class Composition { isComposing = false; @@ -14,7 +14,7 @@ class Composition { } private setupListeners() { - const subscriber = getSubscriber(this.scroll.domNode); + const subscriber = findOrCreateSubscriber(this.scroll.domNode); subscriber.on(this, this.scroll.domNode, 'compositionstart', (event) => { if (!this.isComposing) { this.handleCompositionStart(event); diff --git a/packages/quill/src/core/module.ts b/packages/quill/src/core/module.ts index feeac58ad7..dc1ae28802 100644 --- a/packages/quill/src/core/module.ts +++ b/packages/quill/src/core/module.ts @@ -7,6 +7,8 @@ abstract class Module { public quill: Quill, protected options: Partial = {}, ) {} + + detach() {} } export default Module; diff --git a/packages/quill/src/core/quill.ts b/packages/quill/src/core/quill.ts index 7b638da6ba..969cf9391d 100644 --- a/packages/quill/src/core/quill.ts +++ b/packages/quill/src/core/quill.ts @@ -18,7 +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 { getSubscriber } from './subscriber.js'; +import { findOrCreateSubscriber } from './subscriber.js'; import Composition from './composition.js'; import Theme from './theme.js'; import type { ThemeConstructor } from './theme.js'; @@ -206,11 +206,12 @@ class Quill { return false; } - const subscriber = getSubscriber(this.root); + const subscriber = findOrCreateSubscriber(this.root); subscriber.removeAllListeners(); this.emitter.removeAllListeners(); this.emitter.ignoreDOM(); this.scroll.detach(); + this.theme.detach(); return true; } @@ -234,6 +235,7 @@ class Quill { } const scrollBlotName = Parchment.ScrollBlot.blotName; const ScrollBlot = this.options.registry.query(scrollBlotName); + console.log(ScrollBlot); if (!ScrollBlot || !('blotName' in ScrollBlot)) { throw new Error( `Cannot initialize Quill without "${scrollBlotName}" blot`, diff --git a/packages/quill/src/core/subscriber.ts b/packages/quill/src/core/subscriber.ts index c306956570..cc60fb58ad 100644 --- a/packages/quill/src/core/subscriber.ts +++ b/packages/quill/src/core/subscriber.ts @@ -28,7 +28,7 @@ const subscribers = new WeakMap(); * Gets the Subscriber instance bound to the given object. * Creates a new one if the binding does not exist yet. */ -export function getSubscriber(object: Source): Subscriber { +export function findOrCreateSubscriber(object: Source): Subscriber { let subscriber = subscribers.get(object); if (!subscriber) { debug.info(`Creating new Subscriber for ${object.constructor.name}`); diff --git a/packages/quill/src/core/theme.ts b/packages/quill/src/core/theme.ts index a23d212270..fda8124d70 100644 --- a/packages/quill/src/core/theme.ts +++ b/packages/quill/src/core/theme.ts @@ -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'; @@ -27,6 +28,14 @@ class Theme { protected options: ThemeOptions, ) {} + detach() { + Object.values(this.modules).forEach((module) => { + if (module instanceof Module) { + module.detach(); + } + }); + } + init() { Object.keys(this.options.modules).forEach((name) => { if (this.modules[name] == null) { diff --git a/packages/quill/src/formats/list.ts b/packages/quill/src/formats/list.ts index 4254f25550..dae08a269a 100644 --- a/packages/quill/src/formats/list.ts +++ b/packages/quill/src/formats/list.ts @@ -1,8 +1,9 @@ import Block from '../blots/block.js'; import Container from '../blots/container.js'; -import Scroll from '../blots/scroll.js'; +import type Scroll from '../blots/scroll.js'; import Quill from '../core/quill.js'; -import { getSubscriber, Subscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; +import type { Subscriber } from '../core/subscriber.js'; class ListContainer extends Container {} ListContainer.blotName = 'list-container'; @@ -27,7 +28,7 @@ class ListItem extends Block { constructor(scroll: Scroll, domNode: HTMLElement) { super(scroll, domNode); - this.subscriber = getSubscriber(this.scroll.domNode); + this.subscriber = findOrCreateSubscriber(this.scroll.domNode); const ui = domNode.ownerDocument.createElement('span'); const listEventHandler = (e: Event) => { if (!scroll.isEnabled()) return; diff --git a/packages/quill/src/modules/clipboard.ts b/packages/quill/src/modules/clipboard.ts index 59d8a4c7a7..1bc9657570 100644 --- a/packages/quill/src/modules/clipboard.ts +++ b/packages/quill/src/modules/clipboard.ts @@ -14,7 +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 { getSubscriber } from '../core/subscriber.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'; @@ -82,7 +82,7 @@ class Clipboard extends Module { constructor(quill: Quill, options: Partial) { super(quill, options); const { root } = this.quill; - const subscriber = getSubscriber(root); + 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)); diff --git a/packages/quill/src/modules/history.ts b/packages/quill/src/modules/history.ts index 6fb018d9c2..744a54fa99 100644 --- a/packages/quill/src/modules/history.ts +++ b/packages/quill/src/modules/history.ts @@ -4,7 +4,7 @@ import Module from '../core/module.js'; import Quill from '../core/quill.js'; import type Scroll from '../blots/scroll.js'; import type { Range } from '../core/selection.js'; -import { getSubscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; export interface HistoryOptions { userOnly: boolean; @@ -72,7 +72,7 @@ class History extends Module { ); } - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); subscriber.on(this, this.quill.root, 'beforeinput', (event) => { if (event.inputType === 'historyUndo') { this.undo(); diff --git a/packages/quill/src/modules/input.ts b/packages/quill/src/modules/input.ts index 990b0df630..83d52540c4 100644 --- a/packages/quill/src/modules/input.ts +++ b/packages/quill/src/modules/input.ts @@ -2,7 +2,7 @@ import Delta from 'quill-delta'; import Module from '../core/module.js'; import Quill from '../core/quill.js'; import type { Range } from '../core/selection.js'; -import { getSubscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; import { deleteRange } from './keyboard.js'; const INSERT_TYPES = ['insertText', 'insertReplacementText']; @@ -11,7 +11,7 @@ class Input extends Module { constructor(quill: Quill, options: Record) { super(quill, options); - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); subscriber.on(this, this.quill.root, 'beforeinput', (event) => { this.handleBeforeInput(event); }); diff --git a/packages/quill/src/modules/keyboard.ts b/packages/quill/src/modules/keyboard.ts index da426007a1..8f69508543 100644 --- a/packages/quill/src/modules/keyboard.ts +++ b/packages/quill/src/modules/keyboard.ts @@ -7,7 +7,7 @@ import logger from '../core/logger.js'; import Module from '../core/module.js'; import type { BlockEmbed } from '../blots/block.js'; import type { Range } from '../core/selection.js'; -import { getSubscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; const debug = logger('quill:keyboard'); @@ -172,7 +172,7 @@ class Keyboard extends Module { } listen() { - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); subscriber.on(this, this.quill.root, 'keydown', (evt) => { if (evt.defaultPrevented || evt.isComposing) return; diff --git a/packages/quill/src/modules/syntax.ts b/packages/quill/src/modules/syntax.ts index 4307c3760c..60ad3ad3da 100644 --- a/packages/quill/src/modules/syntax.ts +++ b/packages/quill/src/modules/syntax.ts @@ -4,7 +4,7 @@ import type { Blot, ScrollBlot } from 'parchment'; import Inline from '../blots/inline.js'; import Quill from '../core/quill.js'; import Module from '../core/module.js'; -import { getSubscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; import { blockDelta } from '../blots/block.js'; import BreakBlot from '../blots/break.js'; import CursorBlot from '../blots/cursor.js'; @@ -244,7 +244,7 @@ class Syntax extends Module { option.setAttribute('value', key); select.appendChild(option); }); - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); subscriber.on(this, select, 'change', () => { blot.format(SyntaxCodeBlock.blotName, select.value); this.quill.root.focus(); // Prevent scrolling diff --git a/packages/quill/src/modules/toolbar.ts b/packages/quill/src/modules/toolbar.ts index 721471c9e9..445d67f1d7 100644 --- a/packages/quill/src/modules/toolbar.ts +++ b/packages/quill/src/modules/toolbar.ts @@ -4,7 +4,7 @@ import Quill from '../core/quill.js'; import logger from '../core/logger.js'; import Module from '../core/module.js'; import type { Range } from '../core/selection.js'; -import { getSubscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; const debug = logger('quill:toolbar'); @@ -89,7 +89,7 @@ class Toolbar extends Module { return; } const eventName = input.tagName === 'SELECT' ? 'change' : 'click'; - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); subscriber.on(this, input, eventName, (e) => { let value; if (input.tagName === 'SELECT') { diff --git a/packages/quill/src/modules/uiNode.ts b/packages/quill/src/modules/uiNode.ts index c098eab3d2..7e3da482ec 100644 --- a/packages/quill/src/modules/uiNode.ts +++ b/packages/quill/src/modules/uiNode.ts @@ -1,7 +1,8 @@ import { ParentBlot } from 'parchment'; import Module from '../core/module.js'; import Quill from '../core/quill.js'; -import { getSubscriber, Subscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; +import type { Subscriber } from '../core/subscriber.js'; const isMac = /Mac/i.test(navigator.platform); @@ -35,7 +36,7 @@ class UINode extends Module { constructor(quill: Quill, options: Record) { super(quill, options); - this.subscriber = getSubscriber(this.quill.root); + this.subscriber = findOrCreateSubscriber(this.quill.root); this.handleArrowKeys(); this.handleNavigationShortcuts(); } diff --git a/packages/quill/src/modules/uploader.ts b/packages/quill/src/modules/uploader.ts index 93eb98ff13..c8ff456963 100644 --- a/packages/quill/src/modules/uploader.ts +++ b/packages/quill/src/modules/uploader.ts @@ -3,7 +3,7 @@ import type Quill from '../core/quill.js'; import Emitter from '../core/emitter.js'; import Module from '../core/module.js'; import type { Range } from '../core/selection.js'; -import { getSubscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; interface UploaderOptions { mimetypes: string[]; @@ -15,7 +15,7 @@ class Uploader extends Module { constructor(quill: Quill, options: Partial) { super(quill, options); - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); subscriber.on(this, this.quill.root, 'drop', (e) => { e.preventDefault(); let native: ReturnType | null = null; diff --git a/packages/quill/src/quill.ts b/packages/quill/src/quill.ts index 9ae4198430..e32cc66a14 100644 --- a/packages/quill/src/quill.ts +++ b/packages/quill/src/quill.ts @@ -118,6 +118,7 @@ Quill.register( export { AttributeMap, Delta, + findOrCreateSubscriber, Module, Op, OpIterator, diff --git a/packages/quill/src/themes/base.ts b/packages/quill/src/themes/base.ts index d762233e5b..090fc656eb 100644 --- a/packages/quill/src/themes/base.ts +++ b/packages/quill/src/themes/base.ts @@ -1,7 +1,7 @@ import { merge } from 'lodash-es'; import type Quill from '../core/quill.js'; import Emitter from '../core/emitter.js'; -import { getSubscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; import Theme from '../core/theme.js'; import type { ThemeOptions } from '../core/theme.js'; import ColorPicker from '../ui/color-picker.js'; @@ -142,7 +142,7 @@ class BaseTheme extends Theme { selects: NodeListOf, icons: Record>, ) { - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); this.pickers = Array.from(selects).map((select) => { if (select.classList.contains('ql-align')) { if (select.querySelector('option') == null) { @@ -234,7 +234,7 @@ class BaseTooltip extends Tooltip { } listen() { - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); // @ts-expect-error Fix me later subscriber.on(this, this.textbox, 'keydown', (event) => { if (event.key === 'Enter') { diff --git a/packages/quill/src/themes/bubble.ts b/packages/quill/src/themes/bubble.ts index c5c001d144..700946f161 100644 --- a/packages/quill/src/themes/bubble.ts +++ b/packages/quill/src/themes/bubble.ts @@ -3,7 +3,7 @@ import Emitter from '../core/emitter.js'; import BaseTheme, { BaseTooltip } from './base.js'; import { Range } from '../core/selection.js'; import type { Bounds } from '../core/selection.js'; -import { getSubscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; import icons from '../ui/icons.js'; import Quill from '../core/quill.js'; import type { ThemeOptions } from '../core/theme.js'; @@ -70,7 +70,7 @@ class BubbleTooltip extends BaseTooltip { listen() { super.listen(); - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); const closeNode = this.root.querySelector('.ql-close'); // @ts-expect-error Fix me later subscriber.on(this, closeNode, 'click', () => { diff --git a/packages/quill/src/themes/snow.ts b/packages/quill/src/themes/snow.ts index 0689942464..710b6fa1e2 100644 --- a/packages/quill/src/themes/snow.ts +++ b/packages/quill/src/themes/snow.ts @@ -3,7 +3,7 @@ import Emitter from '../core/emitter.js'; import BaseTheme, { BaseTooltip } from './base.js'; import LinkBlot from '../formats/link.js'; import { Range } from '../core/selection.js'; -import { getSubscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; import icons from '../ui/icons.js'; import Quill from '../core/quill.js'; import type { Context } from '../modules/keyboard.js'; @@ -30,7 +30,7 @@ class SnowTooltip extends BaseTooltip { listen() { super.listen(); - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); const actionNode = this.root.querySelector('a.ql-action'); // @ts-expect-error Fix me later subscriber.on(this, actionNode, 'click', (event) => { diff --git a/packages/quill/src/ui/tooltip.ts b/packages/quill/src/ui/tooltip.ts index d493a6baed..669fa93bc9 100644 --- a/packages/quill/src/ui/tooltip.ts +++ b/packages/quill/src/ui/tooltip.ts @@ -1,6 +1,6 @@ import type Quill from '../core.js'; import type { Bounds } from '../core/selection.js'; -import { getSubscriber } from '../core/subscriber.js'; +import { findOrCreateSubscriber } from '../core/subscriber.js'; const isScrollable = (el: Element) => { const { overflowY } = getComputedStyle(el, null); @@ -19,7 +19,7 @@ class Tooltip { // @ts-expect-error this.root.innerHTML = this.constructor.TEMPLATE; if (isScrollable(this.quill.root)) { - const subscriber = getSubscriber(this.quill.root); + const subscriber = findOrCreateSubscriber(this.quill.root); subscriber.on(this, this.quill.root, 'scroll', () => { this.root.style.marginTop = `${-1 * this.quill.root.scrollTop}px`; }); diff --git a/packages/quill/test/unit/blots/scroll.spec.ts b/packages/quill/test/unit/blots/scroll.spec.ts index d1c149b568..c8b0f93384 100644 --- a/packages/quill/test/unit/blots/scroll.spec.ts +++ b/packages/quill/test/unit/blots/scroll.spec.ts @@ -8,7 +8,7 @@ import { createRegistry } from '../__helpers__/factory.js'; import { normalizeHTML, sleep } from '../__helpers__/utils.js'; import Underline from '../../../src/formats/underline.js'; import Strike from '../../../src/formats/strike.js'; -import { getSubscriber } from '../../../src/core/subscriber.js'; +import { findOrCreateSubscriber } from '../../../src/core/subscriber.js'; const createScroll = (html: string) => { const emitter = new Emitter(); @@ -62,7 +62,7 @@ describe('Scroll', () => { test('remove event listeners on detach', () => { const scroll = createScroll('

Hello World!

'); - const subscriber = getSubscriber(scroll.domNode); + const subscriber = findOrCreateSubscriber(scroll.domNode); vitest.spyOn(subscriber, 'removeSourceListeners'); scroll.detach(); expect(subscriber.removeSourceListeners).toHaveBeenCalledWith(scroll); diff --git a/packages/quill/test/unit/core/quill.spec.ts b/packages/quill/test/unit/core/quill.spec.ts index a24d95415a..9edecb079a 100644 --- a/packages/quill/test/unit/core/quill.spec.ts +++ b/packages/quill/test/unit/core/quill.spec.ts @@ -14,7 +14,8 @@ import Quill, { import { Range } from '../../../src/core/selection.js'; import Snow from '../../../src/themes/snow.js'; import { normalizeHTML } from '../__helpers__/utils.js'; -import { getSubscriber } from '../../../src/core/subscriber.js'; +import { findOrCreateSubscriber } from '../../../src/core/subscriber.js'; +import { Module } from '../../../src/quill.js'; const createContainer = (html: string | { html: string } = '') => { const container = document.createElement('div'); @@ -109,7 +110,7 @@ describe('Quill', () => { test('event listeners tracking', () => { const container = createContainer(); const quill = new Quill(container); - const subscriber = getSubscriber(quill.root); + const subscriber = findOrCreateSubscriber(quill.root); const subscriptions = subscriber .getSubscriptions() .map((subscription) => ({ @@ -170,12 +171,17 @@ describe('Quill', () => { test('detach()', () => { const { quill } = setup(); vitest.spyOn(quill.scroll, 'detach'); + vitest.spyOn(quill.theme, 'detach'); + vitest.spyOn(Module.prototype, 'detach'); quill.detach(); - const subscriber = getSubscriber(quill.root); + const subscriber = findOrCreateSubscriber(quill.root); expect(subscriber.getSubscriptions().length).toEqual(0); expect(quill.emitter.eventNames()).toEqual([]); expect(quill.emitter.getDomListeners()).toEqual({}); expect(quill.scroll.detach).toHaveBeenCalled(); + expect(quill.theme.detach).toHaveBeenCalled(); + const nModules = Object.entries(quill.theme.modules).length; + expect(Module.prototype.detach).toHaveBeenCalledTimes(nModules); }); test('format()', () => { diff --git a/packages/quill/test/unit/core/subscriber.spec.ts b/packages/quill/test/unit/core/subscriber.spec.ts index 2f3b0edfef..7a76629243 100644 --- a/packages/quill/test/unit/core/subscriber.spec.ts +++ b/packages/quill/test/unit/core/subscriber.spec.ts @@ -1,6 +1,6 @@ import { beforeEach, describe, expect, test, vitest } from 'vitest'; import logger from '../../../src/core/logger.js'; -import { getSubscriber } from '../../../src/core/subscriber.js'; +import { findOrCreateSubscriber } from '../../../src/core/subscriber.js'; describe('Subscriber', () => { class Test {} @@ -10,31 +10,31 @@ describe('Subscriber', () => { object = new Test(); }); - describe('getSubscriber()', () => { + describe('findOrCreateSubscriber()', () => { test('maps a Subscriber to an object', () => { - const subscriber = getSubscriber(object); - expect(getSubscriber(object)).toBe(subscriber); - expect(getSubscriber(new Test())).not.toBe(subscriber); + const subscriber = findOrCreateSubscriber(object); + expect(findOrCreateSubscriber(object)).toBe(subscriber); + expect(findOrCreateSubscriber(new Test())).not.toBe(subscriber); }); test('logs the creation of a new Subscriber instance', () => { logger.level('info'); vitest.spyOn(console, 'info'); - getSubscriber(object); + findOrCreateSubscriber(object); expect(console.info).toHaveBeenCalledWith( 'quill:subscriber', 'Creating new Subscriber for Test', ); - getSubscriber(object); + findOrCreateSubscriber(object); expect(console.info).toHaveBeenCalledTimes(1); - getSubscriber(new Test()); + findOrCreateSubscriber(new Test()); expect(console.info).toHaveBeenCalledTimes(2); }); }); describe('on()', () => { test('calls addEventListener on the target', () => { - const subscriber = getSubscriber(object); + const subscriber = findOrCreateSubscriber(object); const source = new Test(); const target = document.createElement('div'); const event = 'keydown'; @@ -50,7 +50,7 @@ describe('Subscriber', () => { }); test('keeps track of the subscription', () => { - const subscriber = getSubscriber(object); + const subscriber = findOrCreateSubscriber(object); const source = new Test(); const target = document.createElement('div'); const event = 'keydown'; @@ -65,7 +65,7 @@ describe('Subscriber', () => { describe('off()', () => { test('calls removeEventListener on the target', () => { - const subscriber = getSubscriber(object); + const subscriber = findOrCreateSubscriber(object); const target = document.createElement('div'); const event = 'keydown'; const handler = () => {}; @@ -80,7 +80,7 @@ describe('Subscriber', () => { }); test('forgets the subscription', () => { - const subscriber = getSubscriber(object); + const subscriber = findOrCreateSubscriber(object); const source = new Test(); const target = document.createElement('div'); const event = 'keydown'; @@ -94,7 +94,7 @@ describe('Subscriber', () => { describe('removeSourceListeners()', () => { test('removes all listeners related to a source', () => { - const subscriber = getSubscriber(object); + const subscriber = findOrCreateSubscriber(object); const source1 = new Test(); const source2 = new Test(); const target1 = document.createElement('div'); @@ -119,7 +119,7 @@ describe('Subscriber', () => { describe('removeAllListeners()', () => { test('removes all listeners', () => { - const subscriber = getSubscriber(object); + const subscriber = findOrCreateSubscriber(object); const source1 = new Test(); const source2 = new Test(); const target1 = document.createElement('div'); diff --git a/packages/quill/test/unit/formats/list.spec.ts b/packages/quill/test/unit/formats/list.spec.ts index 37f45b052c..74d2c309ab 100644 --- a/packages/quill/test/unit/formats/list.spec.ts +++ b/packages/quill/test/unit/formats/list.spec.ts @@ -9,8 +9,7 @@ import List, { ListContainer } from '../../../src/formats/list.js'; import IndentClass from '../../../src/formats/indent.js'; import { AlignClass } from '../../../src/formats/align.js'; import Video from '../../../src/formats/video.js'; -import { getSubscriber } from '../../../src/core/subscriber.js'; -import ListItem from '../../../src/formats/list.js'; +import { findOrCreateSubscriber } from '../../../src/core/subscriber.js'; const createScroll = (html: string) => baseCreateScroll( @@ -395,8 +394,8 @@ describe('List', () => { test('remove event listeners on detach', () => { const scroll = createScroll('

Hello World!

'); - const listItem = new ListItem(scroll, scroll.domNode); - const subscriber = getSubscriber(scroll.domNode); + const listItem = new List(scroll, scroll.domNode); + const subscriber = findOrCreateSubscriber(scroll.domNode); vitest.spyOn(subscriber, 'removeSourceListeners'); listItem.detach(); expect(subscriber.removeSourceListeners).toHaveBeenCalledWith(listItem); diff --git a/packages/quill/test/unit/ui/picker.spec.ts b/packages/quill/test/unit/ui/picker.spec.ts index 5ebb877cac..4a85fc4340 100644 --- a/packages/quill/test/unit/ui/picker.spec.ts +++ b/packages/quill/test/unit/ui/picker.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from 'vitest'; import Picker from '../../../src/ui/picker.js'; -import { getSubscriber } from '../../../src/core/subscriber.js'; +import { findOrCreateSubscriber } from '../../../src/core/subscriber.js'; describe('Picker', () => { const setup = () => { @@ -9,7 +9,7 @@ describe('Picker', () => { ''; const pickerSelectorInstance = new Picker( container.firstChild as HTMLSelectElement, - getSubscriber(container), + findOrCreateSubscriber(container), ); const pickerSelector = container.querySelector('.ql-picker') as HTMLElement; return { container, pickerSelectorInstance, pickerSelector }; diff --git a/packages/website/content/docs/api.mdx b/packages/website/content/docs/api.mdx index 9cb5e7cf42..69e357c5d7 100644 --- a/packages/website/content/docs/api.mdx +++ b/packages/website/content/docs/api.mdx @@ -7,7 +7,7 @@ title: API