From f76c7d74d076b53d2f17ef4d234d4fa894bbf611 Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Tue, 27 Sep 2022 20:17:59 -0400 Subject: [PATCH] Cleanup and refactoring (#2239) * Remove unused ignoreSelectors * Remove unused isMouseButtonPressed * Update getWritingMode to use the immediate element if possible * Move static functions to DocumentUtil * Fix documentation --- ext/js/dom/document-util.js | 40 ++++++++++++++----- ext/js/dom/dom-data-binder.js | 34 ++-------------- ext/js/dom/text-source-range.js | 9 +++-- .../settings/generic-setting-controller.js | 3 +- .../settings/keyboard-shortcuts-controller.js | 4 +- .../settings/nested-popups-controller.js | 4 +- 6 files changed, 45 insertions(+), 49 deletions(-) diff --git a/ext/js/dom/document-util.js b/ext/js/dom/document-util.js index ed0a98e7..3934c1e2 100644 --- a/ext/js/dom/document-util.js +++ b/ext/js/dom/document-util.js @@ -328,16 +328,6 @@ class DocumentUtil { return false; } - static isMouseButtonPressed(mouseEvent, button) { - const mouseEventButton = mouseEvent.button; - switch (button) { - case 'primary': return mouseEventButton === 0; - case 'secondary': return mouseEventButton === 2; - case 'auxiliary': return mouseEventButton === 1; - default: return false; - } - } - /** * Gets an array of the active modifier keys. * @param {KeyboardEvent|MouseEvent|TouchEvent} event The event to check. @@ -563,6 +553,26 @@ class DocumentUtil { } } + /** + * Converts a value from an element to a number. + * @param {string} value A string representation of a number. + * @param {object} constraints An object which might contain `min`, `max`, and `step` fields which are used to constrain the value. + * @returns {number} The parsed and constrained number. + */ + static convertElementValueToNumber(value, constraints) { + value = parseFloat(value); + if (!Number.isFinite(value)) { value = 0; } + + let {min, max, step} = constraints; + min = this._convertToNumberOrNull(min); + max = this._convertToNumberOrNull(max); + step = this._convertToNumberOrNull(step); + if (typeof min === 'number') { value = Math.max(value, min); } + if (typeof max === 'number') { value = Math.min(value, max); } + if (typeof step === 'number' && step !== 0) { value = Math.round(value / step) * step; } + return value; + } + // Private static _getActiveButtons(event, array) { @@ -905,6 +915,16 @@ class DocumentUtil { static _isElementUserSelectAll(element) { return getComputedStyle(element).userSelect === 'all'; } + + static _convertToNumberOrNull(value) { + if (typeof value !== 'number') { + if (typeof value !== 'string' || value.length === 0) { + return null; + } + value = parseFloat(value); + } + return !Number.isNaN(value) ? value : null; + } } // eslint-disable-next-line no-underscore-dangle DocumentUtil._transparentColorPattern = /rgba\s*\([^)]*,\s*0(?:\.0+)?\s*\)/; diff --git a/ext/js/dom/dom-data-binder.js b/ext/js/dom/dom-data-binder.js index 185dc439..5835cf8c 100644 --- a/ext/js/dom/dom-data-binder.js +++ b/ext/js/dom/dom-data-binder.js @@ -16,14 +16,14 @@ */ /* global + * DocumentUtil * SelectorObserver * TaskAccumulator */ class DOMDataBinder { - constructor({selector, ignoreSelectors=[], createElementMetadata, compareElementMetadata, getValues, setValues, onError=null}) { + constructor({selector, createElementMetadata, compareElementMetadata, getValues, setValues, onError=null}) { this._selector = selector; - this._ignoreSelectors = ignoreSelectors; this._createElementMetadata = createElementMetadata; this._compareElementMetadata = compareElementMetadata; this._getValues = getValues; @@ -33,7 +33,7 @@ class DOMDataBinder { this._assignTasks = new TaskAccumulator(this._onBulkAssign.bind(this)); this._selectorObserver = new SelectorObserver({ selector, - ignoreSelector: (ignoreSelectors.length > 0 ? ignoreSelectors.join(',') : null), + ignoreSelector: null, onAdded: this._createObserver.bind(this), onRemoved: this._removeObserver.bind(this), onChildrenUpdated: this._onObserverChildrenUpdated.bind(this), @@ -186,7 +186,7 @@ class DOMDataBinder { case 'text': return `${element.value}`; case 'number': - return DOMDataBinder.convertToNumber(element.value, element); + return DocumentUtil.convertElementValueToNumber(element.value, element); case 'textarea': case 'select': return element.value; @@ -210,30 +210,4 @@ class DOMDataBinder { return null; } } - - // Utilities - - static convertToNumber(value, constraints) { - value = parseFloat(value); - if (!Number.isFinite(value)) { value = 0; } - - let {min, max, step} = constraints; - min = DOMDataBinder.convertToNumberOrNull(min); - max = DOMDataBinder.convertToNumberOrNull(max); - step = DOMDataBinder.convertToNumberOrNull(step); - if (typeof min === 'number') { value = Math.max(value, min); } - if (typeof max === 'number') { value = Math.min(value, max); } - if (typeof step === 'number' && step !== 0) { value = Math.round(value / step) * step; } - return value; - } - - static convertToNumberOrNull(value) { - if (typeof value !== 'number') { - if (typeof value !== 'string' || value.length === 0) { - return null; - } - value = parseFloat(value); - } - return !Number.isNaN(value) ? value : null; - } } diff --git a/ext/js/dom/text-source-range.js b/ext/js/dom/text-source-range.js index a0225748..bbc22599 100644 --- a/ext/js/dom/text-source-range.js +++ b/ext/js/dom/text-source-range.js @@ -124,7 +124,7 @@ class TextSourceRange { * @param {boolean} fromEnd Whether to move the offset from the current end position (if `true`) or the start position (if `false`). * @param {boolean} layoutAwareScan Whether or not HTML layout information should be used to generate * the string content when scanning. - * @returns {number} The actual number of characters (not codepoints) that were read. + * @returns {number} The actual number of codepoints that were read. */ setEndOffset(length, fromEnd, layoutAwareScan) { let node; @@ -147,7 +147,7 @@ class TextSourceRange { * @param {number} length The maximum number of codepoints to move by. * @param {boolean} layoutAwareScan Whether or not HTML layout information should be used to generate * the string content when scanning. - * @returns {number} The actual number of characters (not codepoints) that were read. + * @returns {number} The actual number of codepoints that were read. */ setStartOffset(length, layoutAwareScan) { const state = new DOMTextScanner(this._range.startContainer, this._range.startOffset, !layoutAwareScan, layoutAwareScan).seek(-length); @@ -172,8 +172,9 @@ class TextSourceRange { * @returns {string} The rects. */ getWritingMode() { - const node = this._isImposterDisconnected() ? this._imposterSourceElement : this._range.startContainer; - return DocumentUtil.getElementWritingMode(node !== null ? node.parentElement : null); + let node = this._isImposterDisconnected() ? this._imposterSourceElement : this._range.startContainer; + if (node !== null && node.nodeType !== Node.ELEMENT_NODE) { node = node.parentElement; } + return DocumentUtil.getElementWritingMode(node); } /** diff --git a/ext/js/pages/settings/generic-setting-controller.js b/ext/js/pages/settings/generic-setting-controller.js index 58f472a2..68cb1155 100644 --- a/ext/js/pages/settings/generic-setting-controller.js +++ b/ext/js/pages/settings/generic-setting-controller.js @@ -17,6 +17,7 @@ /* globals * DOMDataBinder + * DocumentUtil */ class GenericSettingController { @@ -218,7 +219,7 @@ class GenericSettingController { _toNumber(value, data) { let {constraints} = data; if (!isObject(constraints)) { constraints = {}; } - return DOMDataBinder.convertToNumber(value, constraints); + return DocumentUtil.convertElementValueToNumber(value, constraints); } _toBoolean(value) { diff --git a/ext/js/pages/settings/keyboard-shortcuts-controller.js b/ext/js/pages/settings/keyboard-shortcuts-controller.js index 69acf275..5a3b94a6 100644 --- a/ext/js/pages/settings/keyboard-shortcuts-controller.js +++ b/ext/js/pages/settings/keyboard-shortcuts-controller.js @@ -16,7 +16,7 @@ */ /* global - * DOMDataBinder + * DocumentUtil * KeyboardMouseInputField * ObjectPropertyAccessor */ @@ -322,7 +322,7 @@ class KeyboardShortcutHotkeyEntry { let value = this._getArgumentInputValue(node); switch (template) { case 'hotkey-argument-move-offset': - value = `${DOMDataBinder.convertToNumber(value, node)}`; + value = `${DocumentUtil.convertElementValueToNumber(value, node)}`; break; } this._setArgument(value); diff --git a/ext/js/pages/settings/nested-popups-controller.js b/ext/js/pages/settings/nested-popups-controller.js index 8b6f02c5..cbee4e0e 100644 --- a/ext/js/pages/settings/nested-popups-controller.js +++ b/ext/js/pages/settings/nested-popups-controller.js @@ -16,7 +16,7 @@ */ /* global - * DOMDataBinder + * DocumentUtil */ class NestedPopupsController { @@ -52,7 +52,7 @@ class NestedPopupsController { _onNestedPopupsCountChange(e) { const node = e.currentTarget; - const value = Math.max(1, DOMDataBinder.convertToNumber(node.value, node)); + const value = Math.max(1, DocumentUtil.convertElementValueToNumber(node.value, node)); this._setPopupNestingMaxDepth(value); }