From 2e059727a99b2923c49479efd34fea1815ff34bb Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 29 Jul 2022 14:47:57 +0200 Subject: [PATCH] [api-minor] Change the various factories, in the viewer, to accept Objects Currently all of these factories take a bunch of (randomly ordered) parameters, which first of all doesn't look that nice in the `PDFPageView`-class when some parameters are optional. Furthermore, it also makes deprecation/removal of any existing parameter a *potentially* breaking change. Finally, using an Object will provide a small amount of "documentation" at the call-site which isn't really the case with a bunch of "regular" parameters. Note that all of the `viewer component` examples still work as-is with this patch, which is why I don't believe that we necessarily have to deprecate in the usual fashion. --- web/base_viewer.js | 146 +++++++++++++++++++++++++---------------- web/default_factory.js | 111 +++++++++++++++++++------------ web/interfaces.js | 108 ++++++++++++++++++------------ web/pdf_page_view.js | 60 ++++++++--------- 4 files changed, 250 insertions(+), 175 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index 0ecb510ab..c03318a88 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -1632,22 +1632,27 @@ class BaseViewer { } /** - * @param {HTMLDivElement} textLayerDiv - * @param {number} pageIndex - * @param {PageViewport} viewport - * @param {boolean} enhanceTextSelection - * @param {EventBus} eventBus - * @param {TextHighlighter} highlighter + * @typedef {Object} CreateTextLayerBuilderParameters + * @property {HTMLDivElement} textLayerDiv + * @property {number} pageIndex + * @property {PageViewport} viewport + * @property {boolean} [enhanceTextSelection] + * @property {EventBus} eventBus + * @property {TextHighlighter} highlighter + */ + + /** + * @param {CreateTextLayerBuilderParameters} * @returns {TextLayerBuilder} */ - createTextLayerBuilder( + createTextLayerBuilder({ textLayerDiv, pageIndex, viewport, enhanceTextSelection = false, eventBus, - highlighter - ) { + highlighter, + }) { return new TextLayerBuilder({ textLayerDiv, eventBus, @@ -1661,11 +1666,16 @@ class BaseViewer { } /** - * @param {number} pageIndex - * @param {EventBus} eventBus + * @typedef {Object} CreateTextHighlighterParameters + * @property {number} pageIndex + * @property {EventBus} eventBus + */ + + /** + * @param {CreateTextHighlighterParameters} * @returns {TextHighlighter} */ - createTextHighlighter(pageIndex, eventBus) { + createTextHighlighter({ pageIndex, eventBus }) { return new TextHighlighter({ eventBus, pageIndex, @@ -1674,101 +1684,123 @@ class BaseViewer { } /** - * @param {HTMLDivElement} pageDiv - * @param {PDFPageProxy} pdfPage - * @param {AnnotationStorage} [annotationStorage] - Storage for annotation + * @typedef {Object} CreateAnnotationLayerBuilderParameters + * @property {HTMLDivElement} pageDiv + * @property {PDFPageProxy} pdfPage + * @property {AnnotationStorage} [annotationStorage] - Storage for annotation * data in forms. - * @param {string} [imageResourcesPath] - Path for image resources, mainly + * @property {string} [imageResourcesPath] - Path for image resources, mainly * for annotation icons. Include trailing slash. - * @param {boolean} renderForms - * @param {IL10n} l10n - * @param {boolean} [enableScripting] - * @param {Promise} [hasJSActionsPromise] - * @param {Object} [mouseState] - * @param {Promise> | null>} + * @property {boolean} renderForms + * @property {IL10n} l10n + * @property {boolean} [enableScripting] + * @property {Promise} [hasJSActionsPromise] + * @property {Object} [mouseState] + * @property {Promise> | null>} * [fieldObjectsPromise] - * @param {Map} [annotationCanvasMap] + * @property {Map} [annotationCanvasMap] - Map some + * annotation ids with canvases used to render them. + */ + + /** + * @param {CreateAnnotationLayerBuilderParameters} * @returns {AnnotationLayerBuilder} */ - createAnnotationLayerBuilder( + createAnnotationLayerBuilder({ pageDiv, pdfPage, - annotationStorage = null, + annotationStorage = this.pdfDocument?.annotationStorage, imageResourcesPath = "", renderForms = true, l10n = NullL10n, - enableScripting = null, - hasJSActionsPromise = null, - mouseState = null, - fieldObjectsPromise = null, - annotationCanvasMap = null - ) { + enableScripting = this.enableScripting, + hasJSActionsPromise = this.pdfDocument?.hasJSActions(), + mouseState = this._scriptingManager?.mouseState, + fieldObjectsPromise = this.pdfDocument?.getFieldObjects(), + annotationCanvasMap = null, + }) { return new AnnotationLayerBuilder({ pageDiv, pdfPage, - annotationStorage: - annotationStorage || this.pdfDocument?.annotationStorage, + annotationStorage, imageResourcesPath, renderForms, linkService: this.linkService, downloadManager: this.downloadManager, l10n, - enableScripting: enableScripting ?? this.enableScripting, - hasJSActionsPromise: - hasJSActionsPromise || this.pdfDocument?.hasJSActions(), - fieldObjectsPromise: - fieldObjectsPromise || this.pdfDocument?.getFieldObjects(), - mouseState: mouseState || this._scriptingManager?.mouseState, + enableScripting, + hasJSActionsPromise, + mouseState, + fieldObjectsPromise, annotationCanvasMap, }); } /** - * @param {HTMLDivElement} pageDiv - * @param {PDFPageProxy} pdfPage - * @param {IL10n} l10n - * @param {AnnotationStorage} [annotationStorage] - Storage for annotation + * @typedef {Object} CreateAnnotationEditorLayerBuilderParameters + * @property {AnnotationEditorUIManager} [uiManager] + * @property {HTMLDivElement} pageDiv + * @property {PDFPageProxy} pdfPage + * @property {IL10n} l10n + * @property {AnnotationStorage} [annotationStorage] - Storage for annotation * data in forms. + */ + + /** + * @param {CreateAnnotationEditorLayerBuilderParameters} * @returns {AnnotationEditorLayerBuilder} */ - createAnnotationEditorLayerBuilder( + createAnnotationEditorLayerBuilder({ + uiManager = this.#annotationEditorUIManager, pageDiv, pdfPage, l10n, - annotationStorage = null - ) { + annotationStorage = this.pdfDocument?.annotationStorage, + }) { return new AnnotationEditorLayerBuilder({ - uiManager: this.#annotationEditorUIManager, + uiManager, pageDiv, pdfPage, - annotationStorage: - annotationStorage || this.pdfDocument?.annotationStorage, + annotationStorage, l10n, }); } /** - * @param {HTMLDivElement} pageDiv - * @param {PDFPageProxy} pdfPage - * @param {AnnotationStorage} [annotationStorage] - Storage for annotation + * @typedef {Object} CreateXfaLayerBuilderParameters + * @property {HTMLDivElement} pageDiv + * @property {PDFPageProxy} pdfPage + * @property {AnnotationStorage} [annotationStorage] - Storage for annotation * data in forms. + */ + + /** + * @param {CreateXfaLayerBuilderParameters} * @returns {XfaLayerBuilder} */ - createXfaLayerBuilder(pageDiv, pdfPage, annotationStorage = null) { + createXfaLayerBuilder({ + pageDiv, + pdfPage, + annotationStorage = this.pdfDocument?.annotationStorage, + }) { return new XfaLayerBuilder({ pageDiv, pdfPage, - annotationStorage: - annotationStorage || this.pdfDocument?.annotationStorage, + annotationStorage, linkService: this.linkService, }); } /** - * @param {PDFPageProxy} pdfPage + * @typedef {Object} CreateStructTreeLayerBuilderParameters + * @property {PDFPageProxy} pdfPage + */ + + /** + * @param {CreateStructTreeLayerBuilderParameters} * @returns {StructTreeLayerBuilder} */ - createStructTreeLayerBuilder(pdfPage) { + createStructTreeLayerBuilder({ pdfPage }) { return new StructTreeLayerBuilder({ pdfPage, }); diff --git a/web/default_factory.js b/web/default_factory.js index b09f4598a..1908154a9 100644 --- a/web/default_factory.js +++ b/web/default_factory.js @@ -44,23 +44,29 @@ import { XfaLayerBuilder } from "./xfa_layer_builder.js"; */ class DefaultAnnotationLayerFactory { /** - * @param {HTMLDivElement} pageDiv - * @param {PDFPageProxy} pdfPage - * @param {AnnotationStorage} [annotationStorage] - * @param {string} [imageResourcesPath] - Path for image resources, mainly + * @typedef {Object} CreateAnnotationLayerBuilderParameters + * @property {HTMLDivElement} pageDiv + * @property {PDFPageProxy} pdfPage + * @property {AnnotationStorage} [annotationStorage] - Storage for annotation + * data in forms. + * @property {string} [imageResourcesPath] - Path for image resources, mainly * for annotation icons. Include trailing slash. - * @param {boolean} renderForms - * @param {IL10n} l10n - * @param {boolean} [enableScripting] - * @param {Promise} [hasJSActionsPromise] - * @param {Object} [mouseState] - * @param {Promise> | null>} + * @property {boolean} renderForms + * @property {IL10n} l10n + * @property {boolean} [enableScripting] + * @property {Promise} [hasJSActionsPromise] + * @property {Object} [mouseState] + * @property {Promise> | null>} * [fieldObjectsPromise] - * @param {Map} [annotationCanvasMap] - Map some + * @property {Map} [annotationCanvasMap] - Map some * annotation ids with canvases used to render them. + */ + + /** + * @param {CreateAnnotationLayerBuilderParameters} * @returns {AnnotationLayerBuilder} */ - createAnnotationLayerBuilder( + createAnnotationLayerBuilder({ pageDiv, pdfPage, annotationStorage = null, @@ -71,8 +77,8 @@ class DefaultAnnotationLayerFactory { hasJSActionsPromise = null, mouseState = null, fieldObjectsPromise = null, - annotationCanvasMap = null - ) { + annotationCanvasMap = null, + }) { return new AnnotationLayerBuilder({ pageDiv, pdfPage, @@ -95,19 +101,28 @@ class DefaultAnnotationLayerFactory { */ class DefaultAnnotationEditorLayerFactory { /** - * @param {HTMLDivElement} pageDiv - * @param {PDFPageProxy} pdfPage - * @param {IL10n} l10n - * @param {AnnotationStorage} [annotationStorage] + * @typedef {Object} CreateAnnotationEditorLayerBuilderParameters + * @property {AnnotationEditorUIManager} [uiManager] + * @property {HTMLDivElement} pageDiv + * @property {PDFPageProxy} pdfPage + * @property {IL10n} l10n + * @property {AnnotationStorage} [annotationStorage] - Storage for annotation + * data in forms. + */ + + /** + * @param {CreateAnnotationEditorLayerBuilderParameters} * @returns {AnnotationEditorLayerBuilder} */ - createAnnotationEditorLayerBuilder( + createAnnotationEditorLayerBuilder({ + uiManager = null, pageDiv, pdfPage, l10n, - annotationStorage = null - ) { + annotationStorage = null, + }) { return new AnnotationEditorLayerBuilder({ + uiManager, pageDiv, pdfPage, l10n, @@ -121,10 +136,15 @@ class DefaultAnnotationEditorLayerFactory { */ class DefaultStructTreeLayerFactory { /** - * @param {PDFPageProxy} pdfPage + * @typedef {Object} CreateStructTreeLayerBuilderParameters + * @property {PDFPageProxy} pdfPage + */ + + /** + * @param {CreateStructTreeLayerBuilderParameters} * @returns {StructTreeLayerBuilder} */ - createStructTreeLayerBuilder(pdfPage) { + createStructTreeLayerBuilder({ pdfPage }) { return new StructTreeLayerBuilder({ pdfPage, }); @@ -136,22 +156,27 @@ class DefaultStructTreeLayerFactory { */ class DefaultTextLayerFactory { /** - * @param {HTMLDivElement} textLayerDiv - * @param {number} pageIndex - * @param {PageViewport} viewport - * @param {boolean} enhanceTextSelection - * @param {EventBus} eventBus - * @param {TextHighlighter} highlighter + * @typedef {Object} CreateTextLayerBuilderParameters + * @property {HTMLDivElement} textLayerDiv + * @property {number} pageIndex + * @property {PageViewport} viewport + * @property {boolean} [enhanceTextSelection] + * @property {EventBus} eventBus + * @property {TextHighlighter} highlighter + */ + + /** + * @param {CreateTextLayerBuilderParameters} * @returns {TextLayerBuilder} */ - createTextLayerBuilder( + createTextLayerBuilder({ textLayerDiv, pageIndex, viewport, enhanceTextSelection = false, eventBus, - highlighter - ) { + highlighter, + }) { return new TextLayerBuilder({ textLayerDiv, pageIndex, @@ -168,23 +193,23 @@ class DefaultTextLayerFactory { */ class DefaultXfaLayerFactory { /** - * @param {HTMLDivElement} pageDiv - * @param {PDFPageProxy} pdfPage - * @param {AnnotationStorage} [annotationStorage] - * @param {Object} [xfaHtml] + * @typedef {Object} CreateXfaLayerBuilderParameters + * @property {HTMLDivElement} pageDiv + * @property {PDFPageProxy} pdfPage + * @property {AnnotationStorage} [annotationStorage] - Storage for annotation + * data in forms. */ - createXfaLayerBuilder( - pageDiv, - pdfPage, - annotationStorage = null, - xfaHtml = null - ) { + + /** + * @param {CreateXfaLayerBuilderParameters} + * @returns {XfaLayerBuilder} + */ + createXfaLayerBuilder({ pageDiv, pdfPage, annotationStorage = null }) { return new XfaLayerBuilder({ pageDiv, pdfPage, annotationStorage, linkService: new SimpleLinkService(), - xfaHtml, }); } } diff --git a/web/interfaces.js b/web/interfaces.js index c2c01013c..c9536c84a 100644 --- a/web/interfaces.js +++ b/web/interfaces.js @@ -155,22 +155,27 @@ class IRenderableView { */ class IPDFTextLayerFactory { /** - * @param {HTMLDivElement} textLayerDiv - * @param {number} pageIndex - * @param {PageViewport} viewport - * @param {boolean} enhanceTextSelection - * @param {EventBus} eventBus - * @param {TextHighlighter} highlighter + * @typedef {Object} CreateTextLayerBuilderParameters + * @property {HTMLDivElement} textLayerDiv + * @property {number} pageIndex + * @property {PageViewport} viewport + * @property {boolean} [enhanceTextSelection] + * @property {EventBus} eventBus + * @property {TextHighlighter} highlighter + */ + + /** + * @param {CreateTextLayerBuilderParameters} * @returns {TextLayerBuilder} */ - createTextLayerBuilder( + createTextLayerBuilder({ textLayerDiv, pageIndex, viewport, enhanceTextSelection = false, eventBus, - highlighter - ) {} + highlighter, + }) {} } /** @@ -178,24 +183,29 @@ class IPDFTextLayerFactory { */ class IPDFAnnotationLayerFactory { /** - * @param {HTMLDivElement} pageDiv - * @param {PDFPageProxy} pdfPage - * @param {AnnotationStorage} [annotationStorage] - Storage for annotation + * @typedef {Object} CreateAnnotationLayerBuilderParameters + * @property {HTMLDivElement} pageDiv + * @property {PDFPageProxy} pdfPage + * @property {AnnotationStorage} [annotationStorage] - Storage for annotation * data in forms. - * @param {string} [imageResourcesPath] - Path for image resources, mainly + * @property {string} [imageResourcesPath] - Path for image resources, mainly * for annotation icons. Include trailing slash. - * @param {boolean} renderForms - * @param {IL10n} l10n - * @param {boolean} [enableScripting] - * @param {Promise} [hasJSActionsPromise] - * @param {Object} [mouseState] - * @param {Promise> | null>} + * @property {boolean} renderForms + * @property {IL10n} l10n + * @property {boolean} [enableScripting] + * @property {Promise} [hasJSActionsPromise] + * @property {Object} [mouseState] + * @property {Promise> | null>} * [fieldObjectsPromise] - * @param {Map} [annotationCanvasMap] - Map some + * @property {Map} [annotationCanvasMap] - Map some * annotation ids with canvases used to render them. + */ + + /** + * @param {CreateAnnotationLayerBuilderParameters} * @returns {AnnotationLayerBuilder} */ - createAnnotationLayerBuilder( + createAnnotationLayerBuilder({ pageDiv, pdfPage, annotationStorage = null, @@ -206,8 +216,8 @@ class IPDFAnnotationLayerFactory { hasJSActionsPromise = null, mouseState = null, fieldObjectsPromise = null, - annotationCanvasMap = null - ) {} + annotationCanvasMap = null, + }) {} } /** @@ -215,19 +225,26 @@ class IPDFAnnotationLayerFactory { */ class IPDFAnnotationEditorLayerFactory { /** - * @param {HTMLDivElement} pageDiv - * @param {PDFPageProxy} pdfPage - * @param {IL10n} l10n - * @param {AnnotationStorage} [annotationStorage] - Storage for annotation + * @typedef {Object} CreateAnnotationEditorLayerBuilderParameters + * @property {AnnotationEditorUIManager} [uiManager] + * @property {HTMLDivElement} pageDiv + * @property {PDFPageProxy} pdfPage + * @property {IL10n} l10n + * @property {AnnotationStorage} [annotationStorage] - Storage for annotation * data in forms. + */ + + /** + * @param {CreateAnnotationEditorLayerBuilderParameters} * @returns {AnnotationEditorLayerBuilder} */ - createAnnotationEditorLayerBuilder( + createAnnotationEditorLayerBuilder({ + uiManager = null, pageDiv, pdfPage, - l10n = undefined, - annotationStorage = null - ) {} + l10n, + annotationStorage = null, + }) {} } /** @@ -235,18 +252,18 @@ class IPDFAnnotationEditorLayerFactory { */ class IPDFXfaLayerFactory { /** - * @param {HTMLDivElement} pageDiv - * @param {PDFPageProxy} pdfPage - * @param {AnnotationStorage} [annotationStorage] - * @param {Object} [xfaHtml] + * @typedef {Object} CreateXfaLayerBuilderParameters + * @property {HTMLDivElement} pageDiv + * @property {PDFPageProxy} pdfPage + * @property {AnnotationStorage} [annotationStorage] - Storage for annotation + * data in forms. + */ + + /** + * @param {CreateXfaLayerBuilderParameters} * @returns {XfaLayerBuilder} */ - createXfaLayerBuilder( - pageDiv, - pdfPage, - annotationStorage = null, - xfaHtml = null - ) {} + createXfaLayerBuilder({ pageDiv, pdfPage, annotationStorage = null }) {} } /** @@ -254,10 +271,15 @@ class IPDFXfaLayerFactory { */ class IPDFStructTreeLayerFactory { /** - * @param {PDFPageProxy} pdfPage + * @typedef {Object} CreateStructTreeLayerBuilderParameters + * @property {PDFPageProxy} pdfPage + */ + + /** + * @param {CreateStructTreeLayerBuilderParameters} * @returns {StructTreeLayerBuilder} */ - createStructTreeLayerBuilder(pdfPage) {} + createStructTreeLayerBuilder({ pdfPage }) {} } /** diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index bc77234fc..388391762 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -133,10 +133,10 @@ class PDFPageView { this.annotationEditorLayerFactory = options.annotationEditorLayerFactory; this.xfaLayerFactory = options.xfaLayerFactory; this.textHighlighter = - options.textHighlighterFactory?.createTextHighlighter( - this.id - 1, - this.eventBus - ); + options.textHighlighterFactory?.createTextHighlighter({ + pageIndex: this.id - 1, + eventBus: this.eventBus, + }); this.structTreeLayerFactory = options.structTreeLayerFactory; if ( typeof PDFJSDev === "undefined" || @@ -657,14 +657,15 @@ class PDFPageView { div.append(textLayerDiv); } - textLayer = this.textLayerFactory.createTextLayerBuilder( + textLayer = this.textLayerFactory.createTextLayerBuilder({ textLayerDiv, - this.id - 1, - this.viewport, - this.textLayerMode === TextLayerMode.ENABLE_ENHANCE, - this.eventBus, - this.textHighlighter - ); + pageIndex: this.id - 1, + viewport: this.viewport, + enhanceTextSelection: + this.textLayerMode === TextLayerMode.ENABLE_ENHANCE, + eventBus: this.eventBus, + highlighter: this.textHighlighter, + }); } this.textLayer = textLayer; @@ -674,19 +675,14 @@ class PDFPageView { ) { this._annotationCanvasMap ||= new Map(); this.annotationLayer ||= - this.annotationLayerFactory.createAnnotationLayerBuilder( - div, + this.annotationLayerFactory.createAnnotationLayerBuilder({ + pageDiv: div, pdfPage, - /* annotationStorage = */ null, - this.imageResourcesPath, - this.#annotationMode === AnnotationMode.ENABLE_FORMS, - this.l10n, - /* enableScripting = */ null, - /* hasJSActionsPromise = */ null, - /* mouseState = */ null, - /* fieldObjectsPromise = */ null, - /* annotationCanvasMap */ this._annotationCanvasMap - ); + imageResourcesPath: this.imageResourcesPath, + renderForms: this.#annotationMode === AnnotationMode.ENABLE_FORMS, + l10n: this.l10n, + annotationCanvasMap: this._annotationCanvasMap, + }); } if (this.xfaLayer?.div) { @@ -769,10 +765,11 @@ class PDFPageView { if (this.annotationEditorLayerFactory) { this.annotationEditorLayer ||= this.annotationEditorLayerFactory.createAnnotationEditorLayerBuilder( - div, - pdfPage, - this.l10n, - /* annotationStorage = */ null + { + pageDiv: div, + pdfPage, + l10n: this.l10n, + } ); this._renderAnnotationEditorLayer(); } @@ -787,11 +784,10 @@ class PDFPageView { if (this.xfaLayerFactory) { if (!this.xfaLayer) { - this.xfaLayer = this.xfaLayerFactory.createXfaLayerBuilder( - div, + this.xfaLayer = this.xfaLayerFactory.createXfaLayerBuilder({ + pageDiv: div, pdfPage, - /* annotationStorage = */ null - ); + }); } this._renderXfaLayer(); } @@ -825,7 +821,7 @@ class PDFPageView { }; this.eventBus._on("textlayerrendered", this._onTextLayerRendered); this.structTreeLayer = - this.structTreeLayerFactory.createStructTreeLayerBuilder(pdfPage); + this.structTreeLayerFactory.createStructTreeLayerBuilder({ pdfPage }); } div.setAttribute("data-loaded", true);