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 2 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
5 changes: 3 additions & 2 deletions packages/quill/src/blots/scroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
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
4 changes: 2 additions & 2 deletions packages/quill/src/core/composition.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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);
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;
6 changes: 4 additions & 2 deletions packages/quill/src/core/quill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
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

}

Expand All @@ -234,6 +235,7 @@ class Quill {
}
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
2 changes: 1 addition & 1 deletion packages/quill/src/core/subscriber.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ 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 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}`);
Expand Down
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
7 changes: 4 additions & 3 deletions packages/quill/src/formats/list.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions packages/quill/src/modules/clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -82,7 +82,7 @@ class Clipboard extends Module<ClipboardOptions> {
constructor(quill: Quill, options: Partial<ClipboardOptions>) {
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));
Expand Down
4 changes: 2 additions & 2 deletions packages/quill/src/modules/history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -72,7 +72,7 @@ class History extends Module<HistoryOptions> {
);
}

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();
Expand Down
4 changes: 2 additions & 2 deletions packages/quill/src/modules/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand All @@ -11,7 +11,7 @@ class Input extends Module {
constructor(quill: Quill, options: Record<string, never>) {
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);
});
Expand Down
4 changes: 2 additions & 2 deletions packages/quill/src/modules/keyboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -172,7 +172,7 @@ class Keyboard extends Module<KeyboardOptions> {
}

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;

Expand Down
4 changes: 2 additions & 2 deletions packages/quill/src/modules/syntax.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -244,7 +244,7 @@ class Syntax extends Module<SyntaxOptions> {
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
Expand Down
4 changes: 2 additions & 2 deletions packages/quill/src/modules/toolbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -89,7 +89,7 @@ class Toolbar extends Module<ToolbarProps> {
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') {
Expand Down
5 changes: 3 additions & 2 deletions packages/quill/src/modules/uiNode.ts
Original file line number Diff line number Diff line change
@@ -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);

Expand Down Expand Up @@ -35,7 +36,7 @@ class UINode extends Module {

constructor(quill: Quill, options: Record<string, never>) {
super(quill, options);
this.subscriber = getSubscriber(this.quill.root);
this.subscriber = findOrCreateSubscriber(this.quill.root);
this.handleArrowKeys();
this.handleNavigationShortcuts();
}
Expand Down
4 changes: 2 additions & 2 deletions packages/quill/src/modules/uploader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand All @@ -15,7 +15,7 @@ class Uploader extends Module<UploaderOptions> {

constructor(quill: Quill, options: Partial<UploaderOptions>) {
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<typeof document.createRange> | null = null;
Expand Down
1 change: 1 addition & 0 deletions packages/quill/src/quill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ Quill.register(
export {
AttributeMap,
Delta,
findOrCreateSubscriber,
Module,
Op,
OpIterator,
Expand Down
6 changes: 3 additions & 3 deletions packages/quill/src/themes/base.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -142,7 +142,7 @@ class BaseTheme extends Theme {
selects: NodeListOf<HTMLSelectElement>,
icons: Record<string, string | Record<string, string>>,
) {
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) {
Expand Down Expand Up @@ -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') {
Expand Down
4 changes: 2 additions & 2 deletions packages/quill/src/themes/bubble.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/quill/src/themes/snow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/quill/src/ui/tooltip.ts
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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`;
});
Expand Down
4 changes: 2 additions & 2 deletions packages/quill/test/unit/blots/scroll.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -62,7 +62,7 @@ describe('Scroll', () => {

test('remove event listeners on detach', () => {
const scroll = createScroll('<p>Hello World!</p>');
const subscriber = getSubscriber(scroll.domNode);
const subscriber = findOrCreateSubscriber(scroll.domNode);
vitest.spyOn(subscriber, 'removeSourceListeners');
scroll.detach();
expect(subscriber.removeSourceListeners).toHaveBeenCalledWith(scroll);
Expand Down
Loading