From 6345ee68777da4050e5b7996f57a785b569e7469 Mon Sep 17 00:00:00 2001 From: arturovt Date: Thu, 14 Nov 2024 01:23:47 +0200 Subject: [PATCH] fix: reduce asynchronous stuff to resolve flickering In this commit, we introduce a slightly smarter approach to handling custom module registration and executing the `beforeRender` function. The current issue is that the `quill-editor-toolbar` may be projected before the editor is rendered due to the `Promise.all` in `mergeMap`. This is noticeable on slower devices, where the toolbar appears first (without styles), and then after a microtask delay, Quill is created. Switching to observables resolves this issue because observables can emit both synchronously and asynchronously, and most observables can cache their emissions using `shareReplay`, ensuring that subsequent emissions are always synchronous. --- .../config/src/quill-editor.interfaces.ts | 7 +- .../src/lib/quill-editor.component.spec.ts | 6 +- .../src/lib/quill-editor.component.ts | 13 +-- .../ngx-quill/src/lib/quill-view.component.ts | 33 +++--- projects/ngx-quill/src/lib/quill.service.ts | 102 +++++++++++------- 5 files changed, 87 insertions(+), 74 deletions(-) diff --git a/projects/ngx-quill/config/src/quill-editor.interfaces.ts b/projects/ngx-quill/config/src/quill-editor.interfaces.ts index 62abdc20..80383a16 100644 --- a/projects/ngx-quill/config/src/quill-editor.interfaces.ts +++ b/projects/ngx-quill/config/src/quill-editor.interfaces.ts @@ -1,7 +1,8 @@ import { InjectionToken } from '@angular/core' +import type { QuillOptions } from 'quill' +import type { Observable } from 'rxjs' import { defaultModules } from './quill-defaults' -import type { QuillOptions } from 'quill' export interface CustomOption { import: string @@ -62,6 +63,8 @@ export interface QuillModules { export type QuillFormat = 'object' | 'json' | 'html' | 'text' +export type QuillBeforeRender = (() => Promise) | (() => Observable) + export interface QuillConfig { bounds?: HTMLElement | string customModules?: CustomModule[] @@ -82,7 +85,7 @@ export interface QuillConfig { sanitize?: boolean // A function, which is executed before the Quill editor is rendered, this might be useful // for lazy-loading CSS. - beforeRender?: () => Promise + beforeRender?: QuillBeforeRender } export const QUILL_CONFIG_TOKEN = new InjectionToken('config', { diff --git a/projects/ngx-quill/src/lib/quill-editor.component.spec.ts b/projects/ngx-quill/src/lib/quill-editor.component.spec.ts index 45471aeb..e8fb07a0 100644 --- a/projects/ngx-quill/src/lib/quill-editor.component.spec.ts +++ b/projects/ngx-quill/src/lib/quill-editor.component.spec.ts @@ -1457,7 +1457,7 @@ describe('QuillEditor - beforeRender', () => { imports: [QuillModule.forRoot(config)], }) - spyOn(config, 'beforeRender') + spyOn(config, 'beforeRender').and.callThrough() fixture = TestBed.createComponent(BeforeRenderTestComponent) fixture.detectChanges() @@ -1474,11 +1474,11 @@ describe('QuillEditor - beforeRender', () => { imports: [QuillModule.forRoot(config)], }) - spyOn(config, 'beforeRender') + spyOn(config, 'beforeRender').and.callThrough() fixture = TestBed.createComponent(BeforeRenderTestComponent) fixture.componentInstance.beforeRender = () => Promise.resolve() - spyOn(fixture.componentInstance, 'beforeRender') + spyOn(fixture.componentInstance, 'beforeRender').and.callThrough() fixture.detectChanges() await fixture.whenStable() diff --git a/projects/ngx-quill/src/lib/quill-editor.component.ts b/projects/ngx-quill/src/lib/quill-editor.component.ts index 33762019..fbb24514 100644 --- a/projects/ngx-quill/src/lib/quill-editor.component.ts +++ b/projects/ngx-quill/src/lib/quill-editor.component.ts @@ -34,7 +34,7 @@ import { debounceTime, mergeMap } from 'rxjs/operators' import { ControlValueAccessor, NG_VALIDATORS, NG_VALUE_ACCESSOR, Validator } from '@angular/forms' -import { CustomModule, CustomOption, defaultModules, QuillModules } from 'ngx-quill/config' +import { CustomModule, CustomOption, defaultModules, QuillBeforeRender, QuillModules } from 'ngx-quill/config' import type History from 'quill/modules/history' import type Toolbar from 'quill/modules/toolbar' @@ -92,7 +92,7 @@ export abstract class QuillEditorBase implements AfterViewInit, ControlValueAcce readonly formats = input(undefined) readonly customToolbarPosition = input<'top' | 'bottom'>('top') readonly sanitize = input(undefined) - readonly beforeRender = input<() => Promise | undefined>(undefined) + readonly beforeRender = input(undefined) readonly styles = input(null) readonly registry = input( undefined @@ -223,14 +223,7 @@ export abstract class QuillEditorBase implements AfterViewInit, ControlValueAcce // this will lead to runtime exceptions, since the code will be executed on DOM nodes that don't exist within the tree. this.quillSubscription = this.service.getQuill().pipe( - mergeMap((Quill) => { - const promises = [this.service.registerCustomModules(Quill, this.customModules())] - const beforeRender = this.beforeRender() ?? this.service.config.beforeRender - if (beforeRender) { - promises.push(beforeRender()) - } - return Promise.all(promises).then(() => Quill) - }) + mergeMap((Quill) => this.service.beforeRender(Quill, this.customModules(), this.beforeRender())) ).subscribe(Quill => { this.editorElem = this.elementRef.nativeElement.querySelector( '[quill-editor-element]' diff --git a/projects/ngx-quill/src/lib/quill-view.component.ts b/projects/ngx-quill/src/lib/quill-view.component.ts index 5cd1f28e..def15420 100644 --- a/projects/ngx-quill/src/lib/quill-view.component.ts +++ b/projects/ngx-quill/src/lib/quill-view.component.ts @@ -4,31 +4,31 @@ import type QuillType from 'quill' import { AfterViewInit, Component, + DestroyRef, ElementRef, + EventEmitter, Inject, + NgZone, OnChanges, + OnDestroy, + Output, PLATFORM_ID, Renderer2, + SecurityContext, SimpleChanges, ViewEncapsulation, - NgZone, - SecurityContext, - OnDestroy, - input, - EventEmitter, - Output, inject, - DestroyRef + input } from '@angular/core' import { takeUntilDestroyed } from '@angular/core/rxjs-interop' -import { Subscription } from 'rxjs' +import { DomSanitizer } from '@angular/platform-browser' +import type { Subscription } from 'rxjs' import { mergeMap } from 'rxjs/operators' -import { CustomOption, CustomModule, QuillModules } from 'ngx-quill/config' +import { CustomModule, CustomOption, QuillBeforeRender, QuillModules } from 'ngx-quill/config' import { getFormat, raf$ } from './helpers' import { QuillService } from './quill.service' -import { DomSanitizer } from '@angular/platform-browser' @Component({ encapsulation: ViewEncapsulation.None, @@ -52,7 +52,7 @@ export class QuillViewComponent implements AfterViewInit, OnChanges, OnDestroy { readonly debug = input<'warn' | 'log' | 'error' | false>(false) readonly formats = input(undefined) readonly sanitize = input(undefined) - readonly beforeRender = input<() => Promise | undefined>(undefined) + readonly beforeRender = input() readonly strict = input(true) readonly content = input() readonly customModules = input([]) @@ -74,7 +74,7 @@ export class QuillViewComponent implements AfterViewInit, OnChanges, OnDestroy { protected service: QuillService, protected domSanitizer: DomSanitizer, @Inject(PLATFORM_ID) protected platformId: any, - ) {} + ) { } valueSetter = (quillEditor: QuillType, value: any): any => { const format = getFormat(this.format(), this.service.config.format) @@ -114,14 +114,7 @@ export class QuillViewComponent implements AfterViewInit, OnChanges, OnDestroy { } this.quillSubscription = this.service.getQuill().pipe( - mergeMap((Quill) => { - const promises = [this.service.registerCustomModules(Quill, this.customModules())] - const beforeRender = this.beforeRender() ?? this.service.config.beforeRender - if (beforeRender) { - promises.push(beforeRender()) - } - return Promise.all(promises).then(() => Quill) - }) + mergeMap((Quill) => this.service.beforeRender(Quill, this.customModules(), this.beforeRender())) ).subscribe(Quill => { const modules = Object.assign({}, this.modules() || this.service.config.modules) modules.toolbar = false diff --git a/projects/ngx-quill/src/lib/quill.service.ts b/projects/ngx-quill/src/lib/quill.service.ts index faf95f61..fe8bf3be 100644 --- a/projects/ngx-quill/src/lib/quill.service.ts +++ b/projects/ngx-quill/src/lib/quill.service.ts @@ -1,21 +1,25 @@ import { DOCUMENT } from '@angular/common' -import { Inject, Injectable, Injector, Optional } from '@angular/core' -import { defer, firstValueFrom, isObservable, Observable } from 'rxjs' -import { shareReplay } from 'rxjs/operators' +import { inject, Injectable } from '@angular/core' +import { defer, firstValueFrom, forkJoin, from, isObservable, Observable, of } from 'rxjs' +import { map, shareReplay, tap } from 'rxjs/operators' import { CustomModule, defaultModules, QUILL_CONFIG_TOKEN, - QuillConfig, + QuillConfig } from 'ngx-quill/config' @Injectable({ providedIn: 'root', }) export class QuillService { + readonly config = inject(QUILL_CONFIG_TOKEN) || { modules:defaultModules } as QuillConfig + + private document = inject(DOCUMENT) + private Quill!: any - private document: Document + private quill$: Observable = defer(async () => { if (!this.Quill) { // Quill adds events listeners on import https://github.com/quilljs/quill/blob/develop/core/emitter.js#L8 @@ -54,54 +58,74 @@ export class QuillService { ) }) - return await this.registerCustomModules( + return firstValueFrom(this.registerCustomModules( this.Quill, this.config.customModules, this.config.suppressGlobalRegisterWarning - ) - }).pipe(shareReplay({ bufferSize: 1, -refCount: true })) - - constructor( - injector: Injector, - @Optional() @Inject(QUILL_CONFIG_TOKEN) public config: QuillConfig - ) { - this.document = injector.get(DOCUMENT) + )) + }).pipe( + shareReplay({ + bufferSize: 1, + refCount: false + }) + ) - if (!this.config) { - this.config = { modules: defaultModules } - } - } + // A list of custom modules that have already been registered, + // so we don’t need to await their implementation. + private registeredModules = new Set() getQuill() { return this.quill$ } - /** - * Marked as internal so it won't be available for `ngx-quill` consumers, this is only - * internal method to be used within the library. - * - * @internal - */ - async registerCustomModules( + /** @internal */ + beforeRender(Quill: any, customModules: CustomModule[] | undefined, beforeRender = this.config.beforeRender) { + // This function is called each time the editor needs to be rendered, + // so it operates individually per component. If no custom module needs to be + // registered and no `beforeRender` function is provided, it will emit + // immediately and proceed with the rendering. + const sources = [this.registerCustomModules(Quill, customModules)] + if (beforeRender) { + sources.push(from(beforeRender())) + } + return forkJoin(sources).pipe(map(() => Quill)) + } + + /** @internal */ + private registerCustomModules( Quill: any, customModules: CustomModule[] | undefined, suppressGlobalRegisterWarning?: boolean - ): Promise { - if (Array.isArray(customModules)) { - // eslint-disable-next-line prefer-const - for (let { implementation, path } of customModules) { - // The `implementation` might be an observable that resolves the actual implementation, - // e.g. if it should be lazy loaded. - if (isObservable(implementation)) { - implementation = await firstValueFrom(implementation) - } - Quill.register(path, implementation, suppressGlobalRegisterWarning) + ) { + if (!Array.isArray(customModules)) { + return of(Quill) + } + + const sources: Observable[] = [] + + for (const customModule of customModules) { + const { path, implementation: maybeImplementation } = customModule + + // If the module is already registered, proceed to the next module... + if (this.registeredModules.has(path)) { + continue + } + + this.registeredModules.add(path) + + if (isObservable(maybeImplementation)) { + // If the implementation is an observable, we will wait for it to load and + // then register it with Quill. The caller will wait until the module is registered. + sources.push(maybeImplementation.pipe( + tap((implementation) => { + Quill.register(path, implementation, suppressGlobalRegisterWarning) + }) + )) + } else { + Quill.register(path, maybeImplementation, suppressGlobalRegisterWarning) } } - // Return `Quill` constructor so we'll be able to re-use its return value except of using - // `map` operators, etc. - return Quill + return sources.length > 0 ? forkJoin(sources).pipe(map(() => Quill)) : of(Quill) } }