Skip to content

Commit

Permalink
Merge pull request #699 from NeurodataWithoutBorders/track-validation…
Browse files Browse the repository at this point in the history
…-without-display

Update validation tracking to show invalid without spamming with input errors
  • Loading branch information
CodyCBakerPhD authored Mar 25, 2024
2 parents 6998959 + d909551 commit de93c84
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 23 deletions.
39 changes: 21 additions & 18 deletions src/renderer/src/stories/JSONSchemaForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ export class JSONSchemaForm extends LitElement {

this.groups = props.groups ?? []; // NOTE: We assume properties only belong to one conditional requirement group

// NOTE: Documentation for validateEmptyValues
this.validateEmptyValues = props.validateEmptyValues === undefined ? true : props.validateEmptyValues; // false = validate when not empty, true = always validate, null = never validate

if (props.onInvalid) this.onInvalid = props.onInvalid;
Expand Down Expand Up @@ -934,7 +935,13 @@ export class JSONSchemaForm extends LitElement {
if (isUndefined) {
// Throw at least a basic warning if a non-linked property is required and missing
if (!hasLinks && isRequired) {
if (this.validateEmptyValues) {
if (this.validateEmptyValues === null) {
warnings.push({
message: `${schema.title ?? header(name)} is a suggested property.`,
type: "warning",
missing: true,
});
} else {
const rowName = pathToValidate.slice(-1)[0];
const isRow = typeof rowName === "number";

Expand All @@ -949,12 +956,6 @@ export class JSONSchemaForm extends LitElement {
type: "error",
missing: true,
});
} else if (this.validateEmptyValues === null) {
warnings.push({
message: `${schema.title ?? header(name)} is a suggested property.`,
type: "warning",
missing: true,
});
}
}
}
Expand Down Expand Up @@ -995,7 +996,7 @@ export class JSONSchemaForm extends LitElement {
this.#nWarnings += updatedWarnings.length;
this.checkStatus();

// Show aggregated errors and warnings (if any)
// Show aggregated errors and warnings (if allowed)
updatedWarnings.forEach((info) => (onWarning ? "" : this.#addMessage(localPath, info, "warnings")));
info.forEach((info) => (onInfo ? onInfo(info) : this.#addMessage(localPath, info, "info")));

Expand All @@ -1017,16 +1018,18 @@ export class JSONSchemaForm extends LitElement {

return true;
} else {
// Add new invalid classes and errors
input.classList.add("invalid");

// Only add the conditional class for linked elements
await this.#applyToLinkedProperties(
(name, element) => element.classList.add("required", "conditional"),
[...path, name]
);
if (this.validateEmptyValues) {
// Add new invalid classes and errors
input.classList.add("invalid");

// Only add the conditional class for linked elements
await this.#applyToLinkedProperties(
(name, element) => element.classList.add("required", "conditional"),
[...path, name]
);

updatedErrors.forEach((info) => (onError ? "" : this.#addMessage(localPath, info, "errors")));
updatedErrors.forEach((info) => (onError ? "" : this.#addMessage(localPath, info, "errors")));
}
// element.title = errors.map((info) => info.message).join("\n"); // Set all errors to show on hover

return false;
Expand Down Expand Up @@ -1423,7 +1426,7 @@ export class JSONSchemaForm extends LitElement {

// // Delete extraneous results
// this.#deleteExtraneousResults(this.results, this.schema);

this.#requirements = {};
this.#registerRequirements(this.schema, this.required);

return html`
Expand Down
3 changes: 2 additions & 1 deletion src/renderer/src/stories/JSONSchemaInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,8 @@ export class JSONSchemaInput extends LitElement {
attributeChangedCallback(key, _, latest) {
super.attributeChangedCallback(...arguments);

const formSchema = this.form.schema;
const formSchema = this.form?.schema;
if (!formSchema) return;

if (key === "required") {
const name = this.path.slice(-1)[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { header } from "../../../forms/utils";

import autocompleteIcon from "../../../assets/inspect.svg?raw";

const propOrder = ["path", "subject_id", "session_id"];

export async function autocompleteFormatString(path) {
let notification;

Expand Down Expand Up @@ -50,10 +52,8 @@ export async function autocompleteFormatString(path) {
const content = document.createElement("div");
Object.assign(content.style, {
padding: "25px",
paddingBottom: "0px",
});

const propOrder = ["path", "subject_id", "session_id"];
const form = new JSONSchemaForm({
validateEmptyValues: false,
schema: {
Expand Down Expand Up @@ -331,6 +331,13 @@ export class GuidedPathExpansionPage extends Page {
finalStructure[key] = entry;
}

if (Object.keys(finalStructure).length === 0) {
const message =
"Please configure at least one interface. <br/><small>Otherwise, revisit <b>Pipeline Workflow</b> to update your configuration.</small>";
this.#notification = this.notify(message, "error");
throw message;
}

const results = await run(`locate`, finalStructure, { title: "Locating Data" }).catch((error) => {
this.notify(error.message, "error");
throw error;
Expand Down Expand Up @@ -438,7 +445,7 @@ export class GuidedPathExpansionPage extends Page {
const form = (this.form = new JSONSchemaForm({
...structureState,
onThrow,
validateEmptyValues: false,
validateEmptyValues: null,

controls,

Expand Down
1 change: 0 additions & 1 deletion src/renderer/src/stories/pages/uploads/UploadsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ export async function createDandiset(results = {}) {
const content = document.createElement("div");
Object.assign(content.style, {
padding: "25px",
paddingBottom: "0px",
});

const updateNIHInput = (state) => {
Expand Down

0 comments on commit de93c84

Please sign in to comment.