From 6659fbf2a3a6579297c84334758617039d9ce87f Mon Sep 17 00:00:00 2001 From: Garrett Date: Tue, 31 Oct 2023 16:35:57 -0700 Subject: [PATCH 1/8] Prep JSONSchemaForm, but don't implement --- src/renderer/src/stories/JSONSchemaForm.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/renderer/src/stories/JSONSchemaForm.js b/src/renderer/src/stories/JSONSchemaForm.js index 1e80b594a..622d53145 100644 --- a/src/renderer/src/stories/JSONSchemaForm.js +++ b/src/renderer/src/stories/JSONSchemaForm.js @@ -496,6 +496,8 @@ export class JSONSchemaForm extends LitElement { } }; + // willValidateWhenEmpty = (k) => (Array.isArray(this.validateEmptyValues) && this.validateEmptyValues.includes(k)) || this.validateEmptyValues; + #validateRequirements = async (resolved = this.resolved, requirements = this.#requirements, parentPath) => { let invalid = []; From 5bceb523279533a3982a05678598029d5cb2b2a9 Mon Sep 17 00:00:00 2001 From: Garrett Date: Tue, 31 Oct 2023 16:46:45 -0700 Subject: [PATCH 2/8] Fix array copy-paste (only works when empty) --- src/renderer/src/stories/Table.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/stories/Table.js b/src/renderer/src/stories/Table.js index 9984a75bf..d3edcfdd7 100644 --- a/src/renderer/src/stories/Table.js +++ b/src/renderer/src/stories/Table.js @@ -423,6 +423,7 @@ export class Table extends LitElement { // Update data on passed object else { + const globalValue = this.globals[header]; if (value == undefined || value === "") { @@ -432,7 +433,15 @@ export class Table extends LitElement { this.onOverride(header, value, rowName); } target[rowName][header] = undefined; - } else target[rowName][header] = value === globalValue ? undefined : value; + } else { + + // Correct for expected arrays (copy-paste issue) + if (entries[header]?.type === "array") { + if (value && !Array.isArray(value)) value = value.split(",").map((v) => v.trim()); + } + + target[rowName][header] = value === globalValue ? undefined : value; + } } validated++; From 7774ce2379d7585b278e86c46431379ece74f1c2 Mon Sep 17 00:00:00 2001 From: Garrett Date: Tue, 31 Oct 2023 16:50:48 -0700 Subject: [PATCH 3/8] Selectively validate certain empty values on tables --- src/renderer/src/stories/Table.js | 68 ++++++++++++------- .../pages/guided-mode/setup/GuidedSubjects.js | 27 +++++--- 2 files changed, 59 insertions(+), 36 deletions(-) diff --git a/src/renderer/src/stories/Table.js b/src/renderer/src/stories/Table.js index d3edcfdd7..0cd2320b5 100644 --- a/src/renderer/src/stories/Table.js +++ b/src/renderer/src/stories/Table.js @@ -1,5 +1,4 @@ import { LitElement, html } from "lit"; -import { notify } from "../dependencies/globals"; import { Handsontable, css } from "./hot"; import { header } from "./forms/utils"; import { errorHue, warningHue } from "./globals"; @@ -77,6 +76,7 @@ export class Table extends LitElement { onOverride, validateEmptyCells, onStatusChange, + onThrow, contextMenu, } = {}) { super(); @@ -87,6 +87,7 @@ export class Table extends LitElement { this.validateEmptyCells = validateEmptyCells ?? true; this.contextMenu = contextMenu ?? {}; + if (onThrow) this.onThrow = onThrow; if (onUpdate) this.onUpdate = onUpdate; if (onOverride) this.onOverride = onOverride; if (validateOnChange) this.validateOnChange = validateOnChange; @@ -138,6 +139,13 @@ export class Table extends LitElement { validate = () => { let message; + const nUnresolved = Object.keys(this.unresolved).length; + if (nUnresolved) + message = `${nUnresolved} row${nUnresolved > 1 ? "s are" : " is"} missing a ${ + this.keyColumn ? `${header(this.keyColumn)} ` : "n " + }entry`; + + if (!message) { const errors = this.querySelectorAll("[error]"); const len = errors.length; @@ -145,12 +153,6 @@ export class Table extends LitElement { else if (len) message = `${len} errors exist on this table.`; } - const nUnresolved = Object.keys(this.unresolved).length; - if (nUnresolved) - message = `${nUnresolved} row${nUnresolved > 1 ? "s are" : " is"} missing a ${ - this.keyColumn ? `${header(this.keyColumn)} ` : "n " - }entry`; - if (message) throw new Error(message); }; @@ -158,6 +160,7 @@ export class Table extends LitElement { onStatusChange = () => {}; onUpdate = () => {}; onOverride = () => {}; + onThrow = () => {} isRequired = (col) => { return this.schema?.required?.includes(col); @@ -166,6 +169,8 @@ export class Table extends LitElement { updated() { const div = (this.shadowRoot ?? this).querySelector("div"); + const unresolved = (this.unresolved = {}); + const entries = { ...this.schema.properties }; // Add existing additional properties to the entries variable if necessary @@ -266,25 +271,31 @@ export class Table extends LitElement { const isRequired = this.isRequired(k); const validator = async function (value, callback) { - if (!value) { - if (!ogThis.validateEmptyCells) { - ogThis.#handleValidationResult( - [], // Clear errors - this.row, - this.col - ); - callback(true); // Allow empty value - return true; - } - if (isRequired) { + const validateEmptyCells = ogThis.validateEmptyCells; + const willValidate = validateEmptyCells === true || (Array.isArray(validateEmptyCells) && validateEmptyCells.includes(k)) + + + // Clear empty values if not validated + if (!value && !willValidate) { + ogThis.#handleValidationResult( + [], // Clear errors + this.row, + this.col + ); + callback(true); // Allow empty value + return true; + } + + if (value && k === ogThis.keyColumn && unresolved[this.row]) { + if (value in ogThis.data) { ogThis.#handleValidationResult( - [{ message: `${k} is a required property.`, type: "error" }], + [{ message: `${header(k)} already exists`, type: "error" }], this.row, this.col ); callback(false); - return true; + return false; } } @@ -292,6 +303,16 @@ export class Table extends LitElement { callback(false); return true; } + + if (!value && isRequired) { + ogThis.#handleValidationResult( + [{ message: `${header(k)} is a required property.`, type: "error" }], + this.row, + this.col + ); + callback(false); + return true; + } }; if (info.validator) { @@ -316,7 +337,7 @@ export class Table extends LitElement { const rel = TH.querySelector(".relative"); const isRequired = this.isRequired(col); - if (isRequired) rel.setAttribute("data-required", this.validateEmptyCells ? true : undefined); + if (isRequired) rel.setAttribute("data-required", this.validateEmptyCells ? (Array.isArray(this.validateEmptyCells) ?this.validateEmptyCells.includes(col) : true) : undefined); if (desc) { let span = rel.querySelector(".info"); @@ -384,8 +405,6 @@ export class Table extends LitElement { const menu = div.ownerDocument.querySelector(".htContextMenu"); if (menu) this.#root.appendChild(menu); // Move to style root - const unresolved = (this.unresolved = {}); - let validated = 0; const initialCellsToUpdate = data.reduce((acc, v) => acc + v.length, 0); @@ -411,7 +430,6 @@ export class Table extends LitElement { // Transfer data to object if (header === this.keyColumn) { - console.log(value, rowName); if (value && value !== rowName) { const old = target[rowName] ?? {}; this.data[value] = old; @@ -455,7 +473,7 @@ export class Table extends LitElement { // If only one row, do not allow deletion table.addHook("beforeRemoveRow", (index, amount) => { if (nRows - amount < 1) { - notify("You must have at least one row", "error"); + this.onThrow("You must have at least one row", "error") return false; } }); diff --git a/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js b/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js index 1dedc3117..ab1e36f5f 100644 --- a/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js +++ b/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js @@ -45,15 +45,6 @@ export class GuidedSubjectsPage extends Page { if (!this.localState[key]) delete globalSubjects[key]; } - const noSessions = Object.keys(this.localState).filter((sub) => !this.localState[sub].sessions?.length); - if (noSessions.length) { - const error = `${noSessions.length} subject${ - noSessions.length > 1 ? "s are" : " is" - } missing Sessions entries`; - this.notify(error, "error"); - throw new Error(error); - } - this.info.globalState.subjects = merge(this.localState, globalSubjects); // Merge the local and global states const { results, subjects } = this.info.globalState; @@ -122,10 +113,14 @@ export class GuidedSubjectsPage extends Page { data: subjects, globals: this.info.globalState.project.Subject, keyColumn: "subject_id", - validateEmptyCells: false, + validateEmptyCells: [ + 'subject_id', + 'sessions' + ], contextMenu: { ignore: ["row_below"], }, + onThrow: (message, type) => this.notify(message, type), onOverride: (name) => { this.notify(`${header(name)} has been overriden with a global value.`, "warning", 3000); }, @@ -133,7 +128,17 @@ export class GuidedSubjectsPage extends Page { this.unsavedUpdates = true; }, validateOnChange: (key, parent, v) => { - if (key === "sessions") return true; + if (key === "sessions") { + if (v?.length) return true + else { + return [ + { + message: "Sessions must have at least one entry", + type: "error", + }, + ] + } + } else { delete parent.sessions; // Delete dessions from parent copy return validateOnChange(key, parent, ["Subject"], v); From 207aeac98caceb74763195f971ecfbac2d1fc6fa Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 31 Oct 2023 23:51:48 +0000 Subject: [PATCH 4/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/renderer/src/stories/Table.js | 23 +++++++++++-------- .../pages/guided-mode/setup/GuidedSubjects.js | 12 ++++------ 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/renderer/src/stories/Table.js b/src/renderer/src/stories/Table.js index 0cd2320b5..eea8d0130 100644 --- a/src/renderer/src/stories/Table.js +++ b/src/renderer/src/stories/Table.js @@ -145,7 +145,6 @@ export class Table extends LitElement { this.keyColumn ? `${header(this.keyColumn)} ` : "n " }entry`; - if (!message) { const errors = this.querySelectorAll("[error]"); const len = errors.length; @@ -160,7 +159,7 @@ export class Table extends LitElement { onStatusChange = () => {}; onUpdate = () => {}; onOverride = () => {}; - onThrow = () => {} + onThrow = () => {}; isRequired = (col) => { return this.schema?.required?.includes(col); @@ -271,11 +270,11 @@ export class Table extends LitElement { const isRequired = this.isRequired(k); const validator = async function (value, callback) { - const validateEmptyCells = ogThis.validateEmptyCells; - const willValidate = validateEmptyCells === true || (Array.isArray(validateEmptyCells) && validateEmptyCells.includes(k)) + const willValidate = + validateEmptyCells === true || + (Array.isArray(validateEmptyCells) && validateEmptyCells.includes(k)); - // Clear empty values if not validated if (!value && !willValidate) { ogThis.#handleValidationResult( @@ -337,7 +336,15 @@ export class Table extends LitElement { const rel = TH.querySelector(".relative"); const isRequired = this.isRequired(col); - if (isRequired) rel.setAttribute("data-required", this.validateEmptyCells ? (Array.isArray(this.validateEmptyCells) ?this.validateEmptyCells.includes(col) : true) : undefined); + if (isRequired) + rel.setAttribute( + "data-required", + this.validateEmptyCells + ? Array.isArray(this.validateEmptyCells) + ? this.validateEmptyCells.includes(col) + : true + : undefined + ); if (desc) { let span = rel.querySelector(".info"); @@ -441,7 +448,6 @@ export class Table extends LitElement { // Update data on passed object else { - const globalValue = this.globals[header]; if (value == undefined || value === "") { @@ -452,7 +458,6 @@ export class Table extends LitElement { } target[rowName][header] = undefined; } else { - // Correct for expected arrays (copy-paste issue) if (entries[header]?.type === "array") { if (value && !Array.isArray(value)) value = value.split(",").map((v) => v.trim()); @@ -473,7 +478,7 @@ export class Table extends LitElement { // If only one row, do not allow deletion table.addHook("beforeRemoveRow", (index, amount) => { if (nRows - amount < 1) { - this.onThrow("You must have at least one row", "error") + this.onThrow("You must have at least one row", "error"); return false; } }); diff --git a/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js b/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js index ab1e36f5f..59f39860f 100644 --- a/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js +++ b/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js @@ -113,10 +113,7 @@ export class GuidedSubjectsPage extends Page { data: subjects, globals: this.info.globalState.project.Subject, keyColumn: "subject_id", - validateEmptyCells: [ - 'subject_id', - 'sessions' - ], + validateEmptyCells: ["subject_id", "sessions"], contextMenu: { ignore: ["row_below"], }, @@ -129,17 +126,16 @@ export class GuidedSubjectsPage extends Page { }, validateOnChange: (key, parent, v) => { if (key === "sessions") { - if (v?.length) return true + if (v?.length) return true; else { return [ { message: "Sessions must have at least one entry", type: "error", }, - ] + ]; } - } - else { + } else { delete parent.sessions; // Delete dessions from parent copy return validateOnChange(key, parent, ["Subject"], v); } From 1fd04f2cc2b60bd1d5e0caeb097d491af9844151 Mon Sep 17 00:00:00 2001 From: Garrett Date: Wed, 1 Nov 2023 10:21:24 -0700 Subject: [PATCH 5/8] Fix callback thrown detection --- src/renderer/src/stories/Table.js | 50 +++++++++++++------ .../pages/guided-mode/setup/GuidedSubjects.js | 2 +- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/renderer/src/stories/Table.js b/src/renderer/src/stories/Table.js index eea8d0130..2bd83d446 100644 --- a/src/renderer/src/stories/Table.js +++ b/src/renderer/src/stories/Table.js @@ -283,9 +283,10 @@ export class Table extends LitElement { this.col ); callback(true); // Allow empty value - return true; + return; } + if (value && k === ogThis.keyColumn && unresolved[this.row]) { if (value in ogThis.data) { ogThis.#handleValidationResult( @@ -294,13 +295,13 @@ export class Table extends LitElement { this.col ); callback(false); - return false; + return; } } if (!(await runThisValidator(value, this.row, this.col))) { callback(false); - return true; + return; } if (!value && isRequired) { @@ -310,20 +311,35 @@ export class Table extends LitElement { this.col ); callback(false); - return true; + return; } }; if (info.validator) { const og = info.validator; info.validator = async function (value, callback) { - const called = await validator.call(this, value, callback); - if (!called) og(value, callback); + let wasCalled = false; + + const newCallback = (valid) => { + wasCalled = true; + callback(valid); + }; + + await validator.call(this, value, newCallback); + if (!wasCalled) og(value, callback); }; } else info.validator = async function (value, callback) { - const called = await validator.call(this, value, callback); - if (!called) callback(true); // Default to true if not called earlier + + let wasCalled = false; + + const newCallback = (valid) => { + wasCalled = true; + callback(valid); + }; + + await validator.call(this, value, newCallback); + if (!wasCalled) callback(true); // Default to true if not called earlier }; return info; @@ -435,14 +451,16 @@ export class Table extends LitElement { const isUserUpdate = initialCellsToUpdate <= validated; - // Transfer data to object - if (header === this.keyColumn) { - if (value && value !== rowName) { - const old = target[rowName] ?? {}; - this.data[value] = old; - delete target[rowName]; - delete unresolved[row]; - rowHeaders[row] = value; + // Transfer data to object (if valid) + if (isValid){ + if (header === this.keyColumn) { + if (value && value !== rowName) { + const old = target[rowName] ?? {}; + this.data[value] = old; + delete target[rowName]; + delete unresolved[row]; + rowHeaders[row] = value; + } } } diff --git a/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js b/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js index 59f39860f..259a41df6 100644 --- a/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js +++ b/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js @@ -136,7 +136,7 @@ export class GuidedSubjectsPage extends Page { ]; } } else { - delete parent.sessions; // Delete dessions from parent copy + delete parent.sessions; // Delete sessions from parent copy return validateOnChange(key, parent, ["Subject"], v); } }, From a3b530cc3100ba5e67ba803ccb9dbf9b1f7b31da Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 1 Nov 2023 17:21:43 +0000 Subject: [PATCH 6/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/renderer/src/stories/Table.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/stories/Table.js b/src/renderer/src/stories/Table.js index 2bd83d446..a10c8e7ea 100644 --- a/src/renderer/src/stories/Table.js +++ b/src/renderer/src/stories/Table.js @@ -286,7 +286,6 @@ export class Table extends LitElement { return; } - if (value && k === ogThis.keyColumn && unresolved[this.row]) { if (value in ogThis.data) { ogThis.#handleValidationResult( @@ -324,13 +323,12 @@ export class Table extends LitElement { wasCalled = true; callback(valid); }; - + await validator.call(this, value, newCallback); if (!wasCalled) og(value, callback); }; } else info.validator = async function (value, callback) { - let wasCalled = false; const newCallback = (valid) => { @@ -452,7 +450,7 @@ export class Table extends LitElement { const isUserUpdate = initialCellsToUpdate <= validated; // Transfer data to object (if valid) - if (isValid){ + if (isValid) { if (header === this.keyColumn) { if (value && value !== rowName) { const old = target[rowName] ?? {}; From 3601483640e6c2eb6e3525132291d9d12f71be89 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 1 Nov 2023 20:01:47 +0000 Subject: [PATCH 7/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/renderer/src/stories/JSONSchemaInput.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/renderer/src/stories/JSONSchemaInput.js b/src/renderer/src/stories/JSONSchemaInput.js index 753aeef26..f99643d45 100644 --- a/src/renderer/src/stories/JSONSchemaInput.js +++ b/src/renderer/src/stories/JSONSchemaInput.js @@ -110,7 +110,6 @@ export class JSONSchemaInput extends LitElement { // onValidate = () => {} updateData(value, forceValidate = false) { - if (this.value === value && !forceValidate) { const el = this.getElement(); if (el.type === "checkbox") el.checked = value; @@ -130,7 +129,6 @@ export class JSONSchemaInput extends LitElement { this.#triggerValidation(name, path); this.#updateData(fullPath, value); - return true; } From 473fdefa7980230a02f1ddf82b336384aaa12911 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 2 Nov 2023 19:58:29 +0000 Subject: [PATCH 8/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/renderer/src/stories/Table.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/renderer/src/stories/Table.js b/src/renderer/src/stories/Table.js index ae660d95e..e88abb036 100644 --- a/src/renderer/src/stories/Table.js +++ b/src/renderer/src/stories/Table.js @@ -270,7 +270,6 @@ export class Table extends LitElement { const isRequired = this.isRequired(k); const validator = async function (value, callback) { - const validateEmptyCells = ogThis.validateEmptyCells; const willValidate = validateEmptyCells === true || @@ -465,7 +464,6 @@ export class Table extends LitElement { delete unresolved[row]; rowHeaders[row] = value; } - } // Update data on passed object