Improve settings transforms (#1187)

4 files changed, 144 insertions, 85 deletions

diff --git a/ext/bg/js/settings/generic-setting-controller.js b/ext/bg/js/settings/generic-setting-controller.js
index 132dc042..04cad67d 100644
--- a/ext/bg/js/settings/generic-setting-controller.js
+++ b/ext/bg/js/settings/generic-setting-controller.js
@@ -31,8 +31,7 @@ class GenericSettingController {
             setValues: this._setValues.bind(this)
         });
         this._transforms = new Map([
-            ['setDocumentAttribute', this._setDocumentAttribute.bind(this)],
-            ['setRelativeAttribute', this._setRelativeAttribute.bind(this)],
+            ['setAttribute', this._setAttribute.bind(this)],
             ['setVisibility', this._setVisibility.bind(this)],
             ['splitTags', this._splitTags.bind(this)],
             ['joinTags', this._joinTags.bind(this)],
@@ -57,12 +56,19 @@ class GenericSettingController {
     }
 
     _createElementMetadata(element) {
-        const {dataset: {setting: path, scope, transform, transformPre, transformPost}} = element;
+        const {dataset: {setting: path, scope, transform: transformRaw}} = element;
+        let transforms;
+        if (typeof transformRaw === 'string') {
+            transforms = JSON.parse(transformRaw);
+            if (!Array.isArray(transforms)) { transforms = [transforms]; }
+        } else {
+            transforms = [];
+        }
         return {
             path,
             scope,
-            transformPre: typeof transformPre === 'string' ? transformPre : transform,
-            transformPost: typeof transformPost === 'string' ? transformPost : transform
+            transforms,
+            transformRaw
         };
     }
 
@@ -70,8 +76,7 @@ class GenericSettingController {
         return (
             metadata1.path === metadata2.path &&
             metadata1.scope === metadata2.scope &&
-            metadata1.transformPre === metadata2.transformPre &&
-            metadata1.transformPost === metadata2.transformPost
+            metadata1.transformRaw === metadata2.transformRaw
         );
     }
 
@@ -91,13 +96,13 @@ class GenericSettingController {
     async _setValues(targets) {
         const defaultScope = this._defaultScope;
         const settingsTargets = [];
-        for (const {metadata, value, element} of targets) {
-            const {path, scope, transformPre} = metadata;
+        for (const {metadata: {path, scope, transforms}, value, element} of targets) {
+            const transformedValue = this._applyTransforms(value, transforms, 'pre', element);
             const target = {
                 path,
                 scope: scope || defaultScope,
                 action: 'set',
-                value: this._transform(value, transformPre, metadata, element)
+                value: transformedValue
             };
             settingsTargets.push(target);
         }
@@ -108,64 +113,50 @@ class GenericSettingController {
         return values.map((value, i) => {
             const error = value.error;
             if (error) { return jsonToError(error); }
-            const {metadata, element} = targets[i];
-            const result = this._transform(value.result, metadata.transformPost, metadata, element);
+            const {metadata: {transforms}, element} = targets[i];
+            const result = this._applyTransforms(value.result, transforms, 'post', element);
             return {result};
         });
     }
 
-    _transform(value, transform, metadata, element) {
-        if (typeof transform === 'string') {
-            const transformFunction = this._transforms.get(transform);
-            if (typeof transformFunction !== 'undefined') {
-                value = transformFunction(value, metadata, element);
-            }
+    _applyTransforms(value, transforms, step, element) {
+        for (const transform of transforms) {
+            const transformStep = transform.step;
+            if (typeof transformStep !== 'undefined' && transformStep !== step) { continue; }
+
+            const transformFunction = this._transforms.get(transform.type);
+            if (typeof transformFunction === 'undefined') { continue; }
+
+            value = transformFunction(value, transform, element);
         }
         return value;
     }
 
     _getAncestor(node, ancestorDistance) {
-        if (typeof ancestorDistance === 'string') {
-            const ii = Number.parseInt(ancestorDistance, 10);
-            if (Number.isFinite(ii)) {
-                if (ii < 0) {
-                    node = document.documentElement;
-                } else {
-                    for (let i = 0; i < ii && node !== null; ++i) {
-                        node = node.parentNode;
-                    }
-                }
-            }
+        if (ancestorDistance < 0) {
+            return document.documentElement;
+        }
+        for (let i = 0; i < ancestorDistance && node !== null; ++i) {
+            node = node.parentNode;
         }
         return node;
     }
 
-    _getElementRelativeToAncestor(node, ancestorDistance, relativeSelector) {
-        const relativeElement = this._getAncestor(node, ancestorDistance);
-        if (relativeElement === null) { return null; }
+    _getRelativeElement(node, ancestorDistance, selector) {
+        const selectorRoot = (
+            typeof ancestorDistance === 'number' ?
+            this._getAncestor(node, ancestorDistance) :
+            document
+        );
+        if (selectorRoot === null) { return null; }
 
         return (
-            typeof relativeSelector === 'string' ?
-            relativeElement.querySelector(relativeSelector) :
-            relativeElement
+            typeof selector === 'string' ?
+            selectorRoot.querySelector(selector) :
+            (selectorRoot === document ? document.documentElement : selectorRoot)
         );
     }
 
-    _getConditionalResult(value, conditionString) {
-        let op = '!!';
-        let rhsOperand = null;
-        try {
-            if (typeof conditionString === 'string') {
-                const {op: op2, value: value2} = JSON.parse(conditionString);
-                op = (typeof op2 === 'string' ? op2 : '===');
-                rhsOperand = value2;
-            }
-        } catch (e) {
-            // NOP
-        }
-        return this._evaluateSimpleOperation(op, value, rhsOperand);
-    }
-
     _evaluateSimpleOperation(operation, lhs, rhs) {
         switch (operation) {
             case '!': return !lhs;
@@ -182,25 +173,20 @@ class GenericSettingController {
 
     // Transforms
 
-    _setDocumentAttribute(value, metadata, element) {
-        document.documentElement.setAttribute(element.dataset.documentAttribute, `${value}`);
-        return value;
-    }
-
-    _setRelativeAttribute(value, metadata, element) {
-        const {ancestorDistance, relativeSelector, relativeAttribute} = element.dataset;
-        const relativeElement = this._getElementRelativeToAncestor(element, ancestorDistance, relativeSelector);
+    _setAttribute(value, data, element) {
+        const {ancestorDistance, selector, attribute} = data;
+        const relativeElement = this._getRelativeElement(element, ancestorDistance, selector);
         if (relativeElement !== null) {
-            relativeElement.setAttribute(relativeAttribute, `${value}`);
+            relativeElement.setAttribute(attribute, `${value}`);
         }
         return value;
     }
 
-    _setVisibility(value, metadata, element) {
-        const {ancestorDistance, relativeSelector, visbilityCondition} = element.dataset;
-        const relativeElement = this._getElementRelativeToAncestor(element, ancestorDistance, relativeSelector);
+    _setVisibility(value, data, element) {
+        const {ancestorDistance, selector, condition} = data;
+        const relativeElement = this._getRelativeElement(element, ancestorDistance, selector);
         if (relativeElement !== null) {
-            relativeElement.hidden = !this._getConditionalResult(value, visbilityCondition);
+            relativeElement.hidden = !this._evaluateSimpleOperation(condition.op, value, condition.value);
         }
         return value;
     }
@@ -213,8 +199,10 @@ class GenericSettingController {
         return value.join(' ');
     }
 
-    _toNumber(value, metadata, element) {
-        return DOMDataBinder.convertToNumber(value, element.dataset);
+    _toNumber(value, data) {
+        let {constraints} = data;
+        if (!isObject(constraints)) { constraints = {}; }
+        return DOMDataBinder.convertToNumber(value, constraints);
     }
 
     _toString(value) {
diff --git a/ext/bg/settings.html b/ext/bg/settings.html
index c6070baf..ec048a3b 100644
--- a/ext/bg/settings.html
+++ b/ext/bg/settings.html
@@ -151,7 +151,13 @@
                 </div>
 
                 <div class="checkbox">
-                    <label><input type="checkbox" id="show-advanced-options" data-setting="general.showAdvanced" data-transform-pre="setDocumentAttribute" data-transform-post="setDocumentAttribute" data-document-attribute="data-options-general-show-advanced"> Show advanced options</label>
+                    <label><input type="checkbox" id="show-advanced-options" data-setting="general.showAdvanced"
+                        data-transform='{
+                            "type": "setAttribute",
+                            "selector": ":root",
+                            "attribute": "data-options-general-show-advanced"
+                        }'
+                    > Show advanced options</label>
                 </div>
 
                 <div class="checkbox options-advanced ignore-form-changes" data-hide-for-browser="firefox-mobile">
@@ -195,7 +201,13 @@
                 </div>
 
                 <div class="checkbox options-advanced">
-                    <label><input type="checkbox" id="show-debug-info" data-setting="general.debugInfo" data-transform-pre="setDocumentAttribute" data-transform-post="setDocumentAttribute" data-document-attribute="data-options-general-debug-info"> Show debug information</label>
+                    <label><input type="checkbox" id="show-debug-info" data-setting="general.debugInfo"
+                        data-transform='{
+                            "type": "setAttribute",
+                            "selector": ":root",
+                            "attribute": "data-options-general-debug-info"
+                        }'
+                    > Show debug information</label>
                 </div>
 
                 <div class="checkbox options-advanced">
@@ -204,7 +216,13 @@
 
                 <div class="form-group">
                     <label for="result-output-mode">Result grouping</label>
-                    <select class="form-control" id="result-output-mode" data-setting="general.resultOutputMode" data-transform-pre="setDocumentAttribute" data-transform-post="setDocumentAttribute" data-document-attribute="data-options-general-result-output-mode">
+                    <select class="form-control" id="result-output-mode" data-setting="general.resultOutputMode"
+                        data-transform='{
+                            "type": "setAttribute",
+                            "selector": ":root",
+                            "attribute": "data-options-general-result-output-mode"
+                        }'
+                    >
                         <option value="group">Group results by term-reading pairs</option>
                         <option value="merge">Group results by main dictionary entry</option>
                         <option value="split">Split definitions to their own results</option>
@@ -498,10 +516,11 @@
                                     <label class="scan-input-toggle scan-input-type"><input type="checkbox" class="scan-input-settings-checkbox" data-property="types.touch"><span>Touch</span></label>
                                     <label class="scan-input-toggle scan-input-type"><input type="checkbox" class="scan-input-settings-checkbox" data-property="types.pen"><span>Pen</span></label>
                                     <label class="scan-input-toggle scan-input-type"><input type="checkbox" class="scan-input-settings-checkbox" data-property="options.showAdvanced"
-                                        data-transform-pre="setRelativeAttribute"
-                                        data-transform-post="setRelativeAttribute"
-                                        data-ancestor-distance="7"
-                                        data-relative-attribute="data-show-advanced"
+                                        data-transform='{
+                                            "type": "setAttribute",
+                                            "ancestorDistance": 7,
+                                            "attribute": "data-show-advanced"
+                                        }'
                                     ><span>Advanced</span></label>
                                 </div></div>
                             </div></div>
@@ -899,7 +918,13 @@
                 </p>
 
                 <div class="checkbox">
-                    <label><input type="checkbox" id="anki-enable" data-setting="anki.enable" data-transform="setDocumentAttribute" data-document-attribute="data-options-anki-enable"> Enable Anki integration</label>
+                    <label><input type="checkbox" id="anki-enable" data-setting="anki.enable"
+                        data-transform='{
+                            "type": "setAttribute",
+                            "selector": ":root",
+                            "attribute": "data-options-anki-enable"
+                        }'
+                    > Enable Anki integration</label>
                 </div>
 
                 <div id="anki-general">
@@ -925,7 +950,12 @@
 
                     <div class="form-group">
                         <label for="card-tags">Card tags <span class="label-light">(comma or space separated)</span></label>
-                        <input type="text" id="card-tags" class="form-control" data-setting="anki.tags" data-transform-pre="splitTags" data-transform-post="joinTags">
+                        <input type="text" id="card-tags" class="form-control" data-setting="anki.tags"
+                            data-transform='[
+                                {"type": "splitTags", "step": "pre"},
+                                {"type": "joinTags", "step": "post"}
+                            ]'
+                        >
                     </div>
 
                     <div class="form-group options-advanced">
diff --git a/ext/bg/settings2.html b/ext/bg/settings2.html
index 41cd3946..be6e01e1 100644
--- a/ext/bg/settings2.html
+++ b/ext/bg/settings2.html
@@ -39,7 +39,13 @@
         </div>
         <div class="sidebar-bottom">
             <label class="outline-item"><span class="outline-item-left">
-                <label class="toggle"><input id="advanced-checkbox" type="checkbox" data-setting="general.showAdvanced" data-transform="setDocumentAttribute" data-document-attribute="data-advanced"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
+                <label class="toggle"><input id="advanced-checkbox" type="checkbox" data-setting="general.showAdvanced"
+                    data-transform='{
+                        "type": "setAttribute",
+                        "selector": ":root",
+                        "attribute": "data-advanced"
+                    }'
+                ><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
             </span><span class="outline-item-label">Advanced</span></label>
             <a href="/bg/info.html" class="outline-item"><span class="outline-item-left"><span class="outline-item-icon icon" data-icon="question-mark"></span></span><span class="outline-item-label">About Yomichan</span></a>
         </div>
@@ -216,7 +222,13 @@
                     </div>
                 </div>
                 <div class="settings-item-right">
-                    <select data-setting="general.resultOutputMode" data-transform="setVisibility" data-ancestor-distance="-1" data-relative-selector="#main-dictionary-container" data-visbility-condition='{"op":"===","value":"merge"}'>
+                    <select data-setting="general.resultOutputMode"
+                        data-transform='{
+                            "type": "setVisibility",
+                            "selector": "#main-dictionary-container",
+                            "condition": {"op": "===", "value": "merge"}
+                        }'
+                    >
                         <option value="split">No grouping</option>
                         <option value="group">Group term-reading pairs</option>
                         <option value="merge">Group related terms</option>
@@ -375,7 +387,13 @@
                     <div class="settings-item-description">When no text or definitions are found, the popup will automatically hide.</div>
                 </div>
                 <div class="settings-item-right">
-                    <label class="toggle"><input type="checkbox" data-setting="scanning.autoHideResults" data-transform="setVisibility" data-ancestor-distance="-1" data-relative-selector="#auto-hide-search-popup-options" data-visbility-condition='{"op":"===","value":true}'><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
+                    <label class="toggle"><input type="checkbox" data-setting="scanning.autoHideResults"
+                        data-transform='{
+                            "type": "setVisibility",
+                            "selector": "#auto-hide-search-popup-options",
+                            "condition": {"op": "===", "value": true}
+                        }'
+                    ><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
                 </div>
             </div>
             <div class="settings-item-children settings-item-children-group" id="auto-hide-search-popup-options" hidden>
@@ -737,7 +755,12 @@
                     <div class="settings-item-description">Control the scaling factor of the popup.</div>
                 </div>
                 <div class="settings-item-right">
-                    <select data-setting="general.popupScalingFactor" data-transform-pre="toNumber" data-transform-post="toString" data-min="0.25">
+                    <select data-setting="general.popupScalingFactor"
+                        data-transform='[
+                            {"type": "toNumber", "step": "pre", "constraints": {"min": 0.25}},
+                            {"type": "toString", "step": "post"}
+                        ]'
+                    >
                         <option value="0.25">25%</option>
                         <option value="0.33">33%</option>
                         <option value="0.5">50%</option>
@@ -1253,7 +1276,12 @@
                 <div class="settings-item-description">List of space or comma separated tags to add to the card.</div>
             </div>
             <div class="settings-item-right">
-                <input type="text" spellcheck="false" autocomplete="off" data-setting="anki.tags" data-transform-pre="splitTags" data-transform-post="joinTags">
+                <input type="text" spellcheck="false" autocomplete="off" data-setting="anki.tags"
+                    data-transform='[
+                        {"type": "splitTags", "step": "pre"},
+                        {"type": "joinTags", "step": "post"}
+                    ]'
+                >
             </div>
         </div></div>
         <div class="settings-item advanced-only">
@@ -1263,7 +1291,13 @@
                     <div class="settings-item-description">When a card is detected as a duplicate, the add buttons will be disabled.</div>
                 </div>
                 <div class="settings-item-right">
-                    <label class="toggle"><input type="checkbox" data-setting="anki.checkForDuplicates" data-transform="setVisibility" data-ancestor-distance="-1" data-relative-selector="#anki-card-duplicate-options" data-visbility-condition='{"op":"===","value":true}'><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
+                    <label class="toggle"><input type="checkbox" data-setting="anki.checkForDuplicates"
+                        data-transform='{
+                            "type": "setVisibility",
+                            "selector": "#anki-card-duplicate-options",
+                            "condition": {"op": "===", "value": true}
+                        }'
+                    ><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
                 </div>
             </div>
             <div class="settings-item-children settings-item-children-group" id="anki-card-duplicate-options" hidden>
@@ -1316,10 +1350,11 @@
                     <div class="settings-item-group-item">
                         <div class="settings-item-group-item-label">Format</div>
                         <select class="short-width short-height" data-setting="anki.screenshot.format"
-                            data-transform="setVisibility"
-                            data-ancestor-distance="-1"
-                            data-relative-selector="#anki-screenshot-quality"
-                            data-visbility-condition='{"op":"===","value":"jpeg"}'
+                            data-transform='{
+                                "type": "setVisibility",
+                                "selector": "#anki-screenshot-quality",
+                                "condition": {"op": "===", "value": "jpeg"}
+                            }'
                         >
                             <option value="png">PNG</option>
                             <option value="jpeg">JPEG</option>
diff --git a/ext/bg/welcome.html b/ext/bg/welcome.html
index 86c8ee83..2c769e2b 100644
--- a/ext/bg/welcome.html
+++ b/ext/bg/welcome.html
@@ -132,7 +132,13 @@
                     <div class="settings-item-description">When no text or definitions are found, the popup will automatically hide.</div>
                 </div>
                 <div class="settings-item-right">
-                    <label class="toggle"><input type="checkbox" data-setting="scanning.autoHideResults" data-transform="setVisibility" data-ancestor-distance="-1" data-relative-selector="#auto-hide-search-popup-options" data-visbility-condition='{"op":"===","value":true}'><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
+                    <label class="toggle"><input type="checkbox" data-setting="scanning.autoHideResults"
+                        data-transform='{
+                            "type": "setVisibility",
+                            "selector": "#auto-hide-search-popup-options",
+                            "condition": {"op": "===", "value": true}
+                        }'
+                    ><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
                 </div>
             </div>
             <div class="settings-item-children settings-item-children-group" id="auto-hide-search-popup-options" hidden>