Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update validation tracking to show invalid without spamming with input errors #699

Merged
merged 2 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading